From 7fd2ace3755084383206d6f66e6d256b9088c618 Mon Sep 17 00:00:00 2001 From: Frederic Suter Date: Wed, 11 Oct 2017 11:01:31 +0200 Subject: [PATCH] further cleanups in instr --- src/instr/instr_paje_trace.cpp | 50 +++++++++++++++---------------- src/instr/instr_paje_types.cpp | 28 +++++++---------- src/instr/instr_paje_values.cpp | 11 ++++--- src/instr/instr_private.hpp | 21 ++++++++----- src/smpi/internals/instr_smpi.cpp | 2 +- src/surf/instr_routing.cpp | 17 ++++++----- 6 files changed, 63 insertions(+), 66 deletions(-) diff --git a/src/instr/instr_paje_trace.cpp b/src/instr/instr_paje_trace.cpp index 23ca2c2d77..eb8b257db3 100644 --- a/src/instr/instr_paje_trace.cpp +++ b/src/instr/instr_paje_trace.cpp @@ -121,22 +121,22 @@ static void print_timestamp(simgrid::instr::PajeEvent* event) } /* internal do the instrumentation module */ -static void insert_into_buffer(simgrid::instr::PajeEvent* tbi) +void simgrid::instr::PajeEvent::insertIntoBuffer() { - if (TRACE_buffer() == 0){ - tbi->print (); - delete tbi; + if (not TRACE_buffer()) { + print(); + delete this; return; } buffer_debug(&buffer); - XBT_DEBUG("%s: insert event_type=%d, timestamp=%f, buffersize=%zu)", __FUNCTION__, (int)tbi->eventType_, - tbi->timestamp_, buffer.size()); + XBT_DEBUG("%s: insert event_type=%d, timestamp=%f, buffersize=%zu)", __FUNCTION__, static_cast(eventType_), + timestamp_, buffer.size()); std::vector::reverse_iterator i; for (i = buffer.rbegin(); i != buffer.rend(); ++i) { simgrid::instr::PajeEvent* e1 = *i; - XBT_DEBUG("compare to %p is of type %d; timestamp:%f", e1, (int)e1->eventType_, e1->timestamp_); - if (e1->timestamp_ <= tbi->timestamp_) + XBT_DEBUG("compare to %p is of type %d; timestamp:%f", e1, static_cast(e1->eventType_), e1->timestamp_); + if (e1->timestamp_ <= timestamp_) break; } if (i == buffer.rend()) @@ -144,9 +144,8 @@ static void insert_into_buffer(simgrid::instr::PajeEvent* tbi) else if (i == buffer.rbegin()) XBT_DEBUG("%s: inserted at end", __FUNCTION__); else - XBT_DEBUG("%s: inserted at pos= %zd from its end", __FUNCTION__, - std::distance(buffer.rbegin(),i)); - buffer.insert(i.base(), tbi); + XBT_DEBUG("%s: inserted at pos= %zd from its end", __FUNCTION__, std::distance(buffer.rbegin(), i)); + buffer.insert(i.base(), this); buffer_debug(&buffer); } @@ -208,7 +207,6 @@ void LogContainerTypeDefinition(simgrid::instr::Type* type) } else { THROW_IMPOSSIBLE; } - //-- } void LogVariableTypeDefinition(simgrid::instr::Type* type) @@ -222,7 +220,7 @@ void LogVariableTypeDefinition(simgrid::instr::Type* type) stream << std::fixed << std::setprecision(TRACE_precision()); stream << simgrid::instr::PAJE_DefineVariableType; stream << " " << type->id_ << " " << type->father_->id_ << " " << type->name_; - if (type->color_) + if (type->isColored()) stream << " \"" << type->color_ << "\""; print_row(); } else if (instr_fmt_type == instr_fmt_TI) { @@ -289,7 +287,7 @@ void simgrid::instr::Value::print() { XBT_DEBUG("%s: event_type=%d", __FUNCTION__, simgrid::instr::PAJE_DefineEntityValue); //print it -if (instr_fmt_type == instr_fmt_paje) { + if (instr_fmt_type == instr_fmt_paje) { stream << std::fixed << std::setprecision(TRACE_precision()); stream << simgrid::instr::PAJE_DefineEntityValue; stream << " " << id_ << " " << father_->id_ << " " << name_; @@ -385,7 +383,7 @@ simgrid::instr::SetVariableEvent::SetVariableEvent(double timestamp, container_t : simgrid::instr::PajeEvent::PajeEvent(container, type, timestamp, PAJE_SetVariable), value(value) { XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::SetVariableEvent::print() @@ -409,7 +407,7 @@ simgrid::instr::AddVariableEvent::AddVariableEvent(double timestamp, container_t : simgrid::instr::PajeEvent::PajeEvent(container, type, timestamp, PAJE_AddVariable), value(value) { XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::AddVariableEvent::print() @@ -432,7 +430,7 @@ simgrid::instr::SubVariableEvent::SubVariableEvent(double timestamp, container_t : simgrid::instr::PajeEvent::PajeEvent(container, type, timestamp, PAJE_SubVariable), value(value) { XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::SubVariableEvent::print() @@ -463,7 +461,7 @@ simgrid::instr::SetStateEvent::SetStateEvent(double timestamp, container_t conta #endif XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::SetStateEvent::print() @@ -502,7 +500,7 @@ simgrid::instr::PushStateEvent::PushStateEvent(double timestamp, container_t con XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } simgrid::instr::PushStateEvent::PushStateEvent(double timestamp, container_t container, Type* type, Value* val) @@ -678,7 +676,7 @@ simgrid::instr::PopStateEvent::PopStateEvent(double timestamp, container_t conta : simgrid::instr::PajeEvent::PajeEvent(container, type, timestamp, PAJE_PopState) { XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::PopStateEvent::print() @@ -701,7 +699,7 @@ simgrid::instr::ResetStateEvent::ResetStateEvent(double timestamp, container_t c : simgrid::instr::PajeEvent::PajeEvent(container, type, timestamp, PAJE_ResetState) { XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); delete[] this; } @@ -722,12 +720,12 @@ void simgrid::instr::ResetStateEvent::print() } simgrid::instr::StartLinkEvent::StartLinkEvent(double timestamp, container_t container, Type* type, - container_t sourceContainer, const char* value, const char* key) + container_t sourceContainer, std::string value, std::string key) : StartLinkEvent(timestamp, container, type, sourceContainer, value, key, -1) {} simgrid::instr::StartLinkEvent::StartLinkEvent(double timestamp, container_t container, Type* type, - container_t sourceContainer, const char* value, const char* key, + container_t sourceContainer, std::string value, std::string key, int size) : simgrid::instr::PajeEvent::PajeEvent(container, type, timestamp, PAJE_StartLink) , sourceContainer_(sourceContainer) @@ -736,7 +734,7 @@ simgrid::instr::StartLinkEvent::StartLinkEvent(double timestamp, container_t con , size_(size) { XBT_DEBUG("%s: event_type=%d, timestamp=%f, value:%s", __FUNCTION__, (int)eventType_, this->timestamp_, this->value_.c_str()); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::StartLinkEvent::print() @@ -768,7 +766,7 @@ simgrid::instr::EndLinkEvent::EndLinkEvent(double timestamp, container_t contain , key(key) { XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::EndLinkEvent::print() @@ -795,7 +793,7 @@ simgrid::instr::NewEvent::NewEvent(double timestamp, container_t container, Type XBT_DEBUG("%s: event_type=%d, timestamp=%f", __FUNCTION__, (int)eventType_, this->timestamp_); - insert_into_buffer (this); + insertIntoBuffer(); } void simgrid::instr::NewEvent::print() diff --git a/src/instr/instr_paje_types.cpp b/src/instr/instr_paje_types.cpp index 1b03324988..86846ff1a3 100644 --- a/src/instr/instr_paje_types.cpp +++ b/src/instr/instr_paje_types.cpp @@ -15,9 +15,9 @@ simgrid::instr::Type* PJ_type_get_root() return rootType; } -simgrid::instr::Type::Type(const char* typeNameBuff, const char* key, const char* color, e_entity_types kind, +simgrid::instr::Type::Type(const char* typeNameBuff, const char* key, std::string color, e_entity_types kind, Type* father) - : kind_(kind), father_(father) + : color_(color), kind_(kind), father_(father) { if (typeNameBuff == nullptr || key == nullptr){ THROWF(tracing_error, 0, "can't create a new type with name or key equal nullptr"); @@ -25,8 +25,6 @@ simgrid::instr::Type::Type(const char* typeNameBuff, const char* key, const char this->name_ = xbt_strdup(typeNameBuff); this->children_ = xbt_dict_new_homogeneous(nullptr); - this->values_ = xbt_dict_new_homogeneous(nullptr); - this->color_ = xbt_strdup(color); this->id_ = bprintf("%lld", instr_new_paje_id()); @@ -38,14 +36,11 @@ simgrid::instr::Type::Type(const char* typeNameBuff, const char* key, const char simgrid::instr::Type::~Type() { - simgrid::instr::Value* val; - char *value_name; xbt_dict_cursor_t cursor = nullptr; - xbt_dict_foreach (values_, cursor, value_name, val) { - XBT_DEBUG("free value %s, child of %s", val->getCname(), val->father_->name_); - delete val; + for (auto elm : values_) { + XBT_DEBUG("free value %s, child of %s", elm.second->getCname(), elm.second->father_->name_); + delete elm.second; } - xbt_dict_free(&values_); simgrid::instr::Type* child; char *child_name; xbt_dict_foreach (children_, cursor, child_name, child) { @@ -54,7 +49,6 @@ simgrid::instr::Type::~Type() xbt_dict_free(&children_); xbt_free(name_); xbt_free(id_); - xbt_free(color_); } simgrid::instr::Type* simgrid::instr::Type::getChild(const char* name) @@ -91,7 +85,7 @@ simgrid::instr::Type* simgrid::instr::Type::containerNew(const char* name, simgr THROWF (tracing_error, 0, "can't create a container type with a nullptr name"); } - simgrid::instr::Type* ret = new simgrid::instr::Type(name, name, nullptr, TYPE_CONTAINER, father); + simgrid::instr::Type* ret = new simgrid::instr::Type(name, name, "", TYPE_CONTAINER, father); if (father == nullptr) { rootType = ret; } else { @@ -107,13 +101,13 @@ simgrid::instr::Type* simgrid::instr::Type::eventNew(const char* name, simgrid:: THROWF (tracing_error, 0, "can't create an event type with a nullptr name"); } - Type* ret = new Type (name, name, nullptr, TYPE_EVENT, father); + Type* ret = new Type(name, name, "", TYPE_EVENT, father); XBT_DEBUG("EventType %s(%s), child of %s(%s)", ret->name_, ret->id_, father->name_, father->id_); LogDefineEventType(ret); return ret; } -simgrid::instr::Type* simgrid::instr::Type::variableNew(const char* name, const char* color, +simgrid::instr::Type* simgrid::instr::Type::variableNew(const char* name, std::string color, simgrid::instr::Type* father) { if (name == nullptr){ @@ -122,7 +116,7 @@ simgrid::instr::Type* simgrid::instr::Type::variableNew(const char* name, const Type* ret = nullptr; - if (not color) { + if (color.empty()) { char white[INSTR_DEFAULT_STR_SIZE] = "1 1 1"; ret = new Type (name, name, white, TYPE_VARIABLE, father); }else{ @@ -141,7 +135,7 @@ simgrid::instr::Type* simgrid::instr::Type::linkNew(const char* name, Type* fath char key[INSTR_DEFAULT_STR_SIZE]; snprintf(key, INSTR_DEFAULT_STR_SIZE, "%s-%s-%s", name, source->id_, dest->id_); - Type* ret = new Type(name, key, nullptr, TYPE_LINK, father); + Type* ret = new Type(name, key, "", TYPE_LINK, father); XBT_DEBUG("LinkType %s(%s), child of %s(%s) %s(%s)->%s(%s)", ret->name_, ret->id_, father->name_, father->id_, source->name_, source->id_, dest->name_, dest->id_); LogLinkTypeDefinition(ret, source, dest); @@ -154,7 +148,7 @@ simgrid::instr::Type* simgrid::instr::Type::stateNew(const char* name, Type* fat THROWF (tracing_error, 0, "can't create a state type with a nullptr name"); } - Type* ret = new Type(name, name, nullptr, TYPE_STATE, father); + Type* ret = new Type(name, name, "", TYPE_STATE, father); XBT_DEBUG("StateType %s(%s), child of %s(%s)", ret->name_, ret->id_, father->name_, father->id_); LogStateTypeDefinition(ret); return ret; diff --git a/src/instr/instr_paje_values.cpp b/src/instr/instr_paje_values.cpp index a31dcd1863..2c66f147ce 100644 --- a/src/instr/instr_paje_values.cpp +++ b/src/instr/instr_paje_values.cpp @@ -18,7 +18,7 @@ simgrid::instr::Value::Value(std::string name, std::string color, simgrid::instr } this->id_ = std::to_string(instr_new_paje_id()); - xbt_dict_set(father->values_, name.c_str(), this, nullptr); + father->values_.insert({name, this}); XBT_DEBUG("new value %s, child of %s", name_.c_str(), father_->name_); print(); }; @@ -29,8 +29,7 @@ simgrid::instr::Value* simgrid::instr::Value::byNameOrCreate(std::string name, s Value* ret = 0; try { ret = Value::byName(name, father); - } - catch(xbt_ex& e) { + } catch (xbt_ex& e) { ret = new Value(name, color, father); } return ret; @@ -44,9 +43,9 @@ simgrid::instr::Value* simgrid::instr::Value::byName(std::string name, Type* fat if (father->kind_ == TYPE_VARIABLE) THROWF(tracing_error, 0, "variables can't have different values (%s)", father->name_); - Value* ret = (Value*)xbt_dict_get_or_null(father->values_, name.c_str()); - if (ret == nullptr) { + auto ret = father->values_.find(name); + if (ret == father->values_.end()) { THROWF(tracing_error, 2, "value with name (%s) not found in father type (%s)", name.c_str(), father->name_); } - return ret; + return ret->second; } diff --git a/src/instr/instr_private.hpp b/src/instr/instr_private.hpp index 09823441ac..32e36b8971 100644 --- a/src/instr/instr_private.hpp +++ b/src/instr/instr_private.hpp @@ -29,6 +29,9 @@ namespace simgrid { namespace instr { + +class Value; + enum e_event_type { PAJE_DefineContainerType, PAJE_DefineVariableType, @@ -58,22 +61,23 @@ class Type { public: char* id_; char* name_; - char* color_; + std::string color_; e_entity_types kind_; Type* father_; xbt_dict_t children_; - xbt_dict_t values_; // valid for all types except variable and container - Type(const char* typeNameBuff, const char* key, const char* color, e_entity_types kind, Type* father); + std::map values_; // valid for all types except variable and container + Type(const char* typeNameBuff, const char* key, std::string color, e_entity_types kind, Type* father); ~Type(); Type* getChild(const char* name); Type* getChildOrNull(const char* name); static Type* containerNew(const char* name, Type* father); static Type* eventNew(const char* name, Type* father); - static Type* variableNew(const char* name, const char* color, Type* father); + static Type* variableNew(const char* name, std::string color, Type* father); static Type* linkNew(const char* name, Type* father, Type* source, Type* dest); static Type* stateNew(const char* name, Type* father); + bool isColored() { return not color_.empty(); } }; //-------------------------------------------------- @@ -138,6 +142,7 @@ public: : container(container), type(type), timestamp_(timestamp), eventType_(eventType){}; virtual void print() = 0; virtual ~PajeEvent(); + void insertIntoBuffer(); }; //-------------------------------------------------- @@ -208,10 +213,10 @@ class StartLinkEvent : public PajeEvent { int size_; public: - StartLinkEvent(double timestamp, Container* container, Type* type, Container* sourceContainer, const char* value, - const char* key); - StartLinkEvent(double timestamp, Container* container, Type* type, Container* sourceContainer, const char* value, - const char* key, int size); + StartLinkEvent(double timestamp, Container* container, Type* type, Container* sourceContainer, std::string value, + std::string key); + StartLinkEvent(double timestamp, Container* container, Type* type, Container* sourceContainer, std::string value, + std::string key, int size); void print() override; }; diff --git a/src/smpi/internals/instr_smpi.cpp b/src/smpi/internals/instr_smpi.cpp index ae223f0160..a83443b5af 100644 --- a/src/smpi/internals/instr_smpi.cpp +++ b/src/smpi/internals/instr_smpi.cpp @@ -208,7 +208,7 @@ void TRACE_smpi_init(int rank) * multiple times but only the last one would be used... */ if (s_type::getOrNull(it.first.c_str(), container->type_) == nullptr) { - Type::variableNew(it.first.c_str(), nullptr, container->type_); + Type::variableNew(it.first.c_str(), "", container->type_); } } #endif diff --git a/src/surf/instr_routing.cpp b/src/surf/instr_routing.cpp index 7bf6a6283a..1bfe237d74 100644 --- a/src/surf/instr_routing.cpp +++ b/src/surf/instr_routing.cpp @@ -209,10 +209,10 @@ static void instr_routing_parse_start_link(simgrid::s4u::Link& link) if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_link())) { simgrid::instr::Type* bandwidth = container->type_->getChildOrNull("bandwidth"); if (bandwidth == nullptr) - bandwidth = simgrid::instr::Type::variableNew("bandwidth", nullptr, container->type_); + bandwidth = simgrid::instr::Type::variableNew("bandwidth", "", container->type_); simgrid::instr::Type* latency = container->type_->getChildOrNull("latency"); if (latency == nullptr) - latency = simgrid::instr::Type::variableNew("latency", nullptr, container->type_); + latency = simgrid::instr::Type::variableNew("latency", "", container->type_); new simgrid::instr::SetVariableEvent(0, container, bandwidth, bandwidth_value); new simgrid::instr::SetVariableEvent(0, container, latency, latency_value); } @@ -231,7 +231,7 @@ static void sg_instr_new_host(simgrid::s4u::Host& host) if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_speed())) { simgrid::instr::Type* speed = container->type_->getChildOrNull("power"); if (speed == nullptr){ - speed = simgrid::instr::Type::variableNew("power", nullptr, container->type_); + speed = simgrid::instr::Type::variableNew("power", "", container->type_); } double current_speed_state = host.getSpeed(); @@ -325,26 +325,27 @@ void instr_routing_define_callbacks () */ static void recursiveNewVariableType(const char* new_typename, const char* color, simgrid::instr::Type* root) { + if (not strcmp(root->name_, "HOST")) { char tnstr[INSTR_DEFAULT_STR_SIZE]; snprintf (tnstr, INSTR_DEFAULT_STR_SIZE, "p%s", new_typename); - simgrid::instr::Type::variableNew(tnstr, color, root); + simgrid::instr::Type::variableNew(tnstr, color == nullptr ? "" : color, root); } if (not strcmp(root->name_, "MSG_VM")) { char tnstr[INSTR_DEFAULT_STR_SIZE]; snprintf (tnstr, INSTR_DEFAULT_STR_SIZE, "p%s", new_typename); - simgrid::instr::Type::variableNew(tnstr, color, root); + simgrid::instr::Type::variableNew(tnstr, color == nullptr ? "" : color, root); } if (not strcmp(root->name_, "LINK")) { char tnstr[INSTR_DEFAULT_STR_SIZE]; snprintf (tnstr, INSTR_DEFAULT_STR_SIZE, "b%s", new_typename); - simgrid::instr::Type::variableNew(tnstr, color, root); + simgrid::instr::Type::variableNew(tnstr, color == nullptr ? "" : color, root); } xbt_dict_cursor_t cursor = nullptr; simgrid::instr::Type* child_type; char *name; xbt_dict_foreach (root->children_, cursor, name, child_type) { - recursiveNewVariableType (new_typename, color, child_type); + recursiveNewVariableType(new_typename, color == nullptr ? "" : color, child_type); } } @@ -357,7 +358,7 @@ static void recursiveNewUserVariableType(const char* father_type, const char* ne simgrid::instr::Type* root) { if (not strcmp(root->name_, father_type)) { - simgrid::instr::Type::variableNew(new_typename, color, root); + simgrid::instr::Type::variableNew(new_typename, color == nullptr ? "" : color, root); } xbt_dict_cursor_t cursor = nullptr; simgrid::instr::Type* child_type; -- 2.20.1