From 2c7a878ee75ee12e2576483c31b34f9d9275fed1 Mon Sep 17 00:00:00 2001 From: Frederic Suter Date: Tue, 17 Oct 2017 17:50:34 +0200 Subject: [PATCH 1/1] further simplifications --- src/instr/instr_paje_trace.cpp | 6 ++-- src/instr/instr_private.hpp | 4 ++- src/instr/instr_resource_utilization.cpp | 41 ++++++++---------------- src/surf/instr_routing.cpp | 38 +++++++++------------- 4 files changed, 36 insertions(+), 53 deletions(-) diff --git a/src/instr/instr_paje_trace.cpp b/src/instr/instr_paje_trace.cpp index 1a5371cb85..1d7f079ce2 100644 --- a/src/instr/instr_paje_trace.cpp +++ b/src/instr/instr_paje_trace.cpp @@ -454,10 +454,10 @@ void simgrid::instr::PushStateEvent::print() char* process_id = nullptr; // FIXME: dirty extract "rank-" from the name, as we want the bare process id here - if (strstr(container->name_.c_str(), "rank-") == nullptr) - process_id = xbt_strdup(container->name_.c_str()); + if (container->getName().find("rank-") != 0) + process_id = xbt_strdup(container->getCname()); else - process_id = xbt_strdup(container->name_.c_str() + 5); + process_id = xbt_strdup(container->getCname() + 5); FILE* trace_file = tracing_files.at(container); diff --git a/src/instr/instr_private.hpp b/src/instr/instr_private.hpp index bb29ad296e..f7d6bedf6d 100644 --- a/src/instr/instr_private.hpp +++ b/src/instr/instr_private.hpp @@ -128,6 +128,7 @@ enum e_container_types { class Container { e_container_types kind_; /* This container is of what kind */ + std::string name_; /* Unique name of this container */ int level_ = 0; /* Level in the hierarchy, root level is 0 */ sg_netpoint_t netpoint_ = nullptr; @@ -135,13 +136,14 @@ public: Container(std::string name, simgrid::instr::e_container_types kind, Container* father); virtual ~Container(); - std::string name_; /* Unique name of this container */ std::string id_; /* Unique id of this container */ Type* type_; /* Type of this container */ Container* father_; std::map children_; static Container* byNameOrNull(std::string name); static Container* byName(std::string name); + std::string getName() { return name_; } + const char* getCname() { return name_.c_str(); } void removeFromParent(); void logCreation(); void logDestruction(); diff --git a/src/instr/instr_resource_utilization.cpp b/src/instr/instr_resource_utilization.cpp index 86d76d885e..127e278139 100644 --- a/src/instr/instr_resource_utilization.cpp +++ b/src/instr/instr_resource_utilization.cpp @@ -13,30 +13,22 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY (instr_resource, instr, "tracing (un)-categorize //to check if variables were previously set to 0, otherwise paje won't simulate them static std::unordered_map platform_variables; -//used by all methods -static void __TRACE_surf_check_variable_set_to_zero(double now, const char* variable, std::string resource) +static void instr_event(double now, double delta, simgrid::instr::Type* variable, container_t resource, double value) { - /* To trace resource utilization, we use pajeAddVariable and pajeSubVariable only. - * The Paje simulator needs a pajeSetVariable in the first place so it knows the initial value of all variables for - * subsequent adds/subs. If we don't do so, the first pajeAddVariable is added to a non-determined value within - * the Paje simulator, causing analysis problems. + /* To trace resource utilization, we use AddVariableEvent and SubVariableEvent only. This implies to add a + * SetVariableEvent first to set the initial value of all variables for subsequent adds/subs. If we don't do so, + * the first AddVariableEvent would be added to a non-determined value, hence causing analysis problems. */ // create a key considering the resource and variable - std::string key = resource + variable; + std::string key = resource->getName() + variable->getName(); - // check if key exists: if it doesn't, set the variable to zero and mark this in the dict + // check if key exists: if it doesn't, set the variable to zero and mark this in the global map. if (platform_variables.find(key) == platform_variables.end()) { - container_t container = simgrid::instr::Container::byName(resource); - simgrid::instr::Type* type = container->type_->byName(variable); - new simgrid::instr::SetVariableEvent(now, container, type, 0); + new simgrid::instr::SetVariableEvent(now, resource, variable, 0); platform_variables[key] = std::string(""); } -} -static void instr_event(double now, double delta, simgrid::instr::Type* variable, container_t resource, double value) -{ - __TRACE_surf_check_variable_set_to_zero(now, variable->getCname(), resource->name_); new simgrid::instr::AddVariableEvent(now, resource, variable, value); new simgrid::instr::SubVariableEvent(now + delta, resource, variable, value); } @@ -45,15 +37,13 @@ static void instr_event(double now, double delta, simgrid::instr::Type* variable void TRACE_surf_link_set_utilization(const char *resource, const char *category, double value, double now, double delta) { //only trace link utilization if link is known by tracing mechanism - if (not simgrid::instr::Container::byNameOrNull(resource)) - return; - if (not value) + container_t container = simgrid::instr::Container::byNameOrNull(resource); + if (not container || not value) return; //trace uncategorized link utilization if (TRACE_uncategorized()){ - XBT_DEBUG("UNCAT LINK [%f - %f] %s bandwidth_used %f", now, now+delta, resource, value); - container_t container = simgrid::instr::Container::byName(resource); + XBT_DEBUG("UNCAT LINK [%f - %f] %s bandwidth_used %f", now, now + delta, resource, value); simgrid::instr::Type* type = container->type_->byName("bandwidth_used"); instr_event (now, delta, type, container, value); } @@ -63,10 +53,8 @@ void TRACE_surf_link_set_utilization(const char *resource, const char *category, if (not category) return; //variable of this category starts by 'b', because we have a link here - char category_type[INSTR_DEFAULT_STR_SIZE]; - snprintf (category_type, INSTR_DEFAULT_STR_SIZE, "b%s", category); - XBT_DEBUG("CAT LINK [%f - %f] %s %s %f", now, now+delta, resource, category_type, value); - container_t container = simgrid::instr::Container::byName(resource); + std::string category_type = std::string("b") + category; + XBT_DEBUG("CAT LINK [%f - %f] %s %s %f", now, now + delta, resource, category_type.c_str(), value); simgrid::instr::Type* type = container->type_->byName(category_type); instr_event (now, delta, type, container, value); } @@ -92,9 +80,8 @@ void TRACE_surf_host_set_utilization(const char *resource, const char *category, if (not category) return; //variable of this category starts by 'p', because we have a host here - char category_type[INSTR_DEFAULT_STR_SIZE]; - snprintf (category_type, INSTR_DEFAULT_STR_SIZE, "p%s", category); - XBT_DEBUG("CAT HOST [%f - %f] %s %s %f", now, now+delta, resource, category_type, value); + std::string category_type = std::string("p") + category; + XBT_DEBUG("CAT HOST [%f - %f] %s %s %f", now, now + delta, resource, category_type.c_str(), value); simgrid::instr::Type* type = container->type_->byName(category_type); instr_event (now, delta, type, container, value); } diff --git a/src/surf/instr_routing.cpp b/src/surf/instr_routing.cpp index d15d6557f4..6447bce76b 100644 --- a/src/surf/instr_routing.cpp +++ b/src/surf/instr_routing.cpp @@ -68,7 +68,7 @@ static container_t lowestCommonAncestor (container_t a1, container_t a2) static void linkContainers(container_t src, container_t dst, std::set* filter) { //ignore loopback - if (src->name_ == "__loopback__" || dst->name_ == "__loopback__") { + if (src->getName() == "__loopback__" || dst->getName() == "__loopback__") { XBT_DEBUG (" linkContainers: ignoring loopback link"); return; } @@ -80,14 +80,14 @@ static void linkContainers(container_t src, container_t dst, std::setname_ + dst->name_; - std::string aux2 = dst->name_ + src->name_; + std::string aux1 = src->getName() + dst->getName(); + std::string aux2 = dst->getName() + src->getName(); if (filter->find(aux1) != filter->end()) { - XBT_DEBUG(" linkContainers: already registered %s <-> %s (1)", src->name_.c_str(), dst->name_.c_str()); + XBT_DEBUG(" linkContainers: already registered %s <-> %s (1)", src->getCname(), dst->getCname()); return; } if (filter->find(aux2) != filter->end()) { - XBT_DEBUG(" linkContainers: already registered %s <-> %s (2)", dst->name_.c_str(), src->name_.c_str()); + XBT_DEBUG(" linkContainers: already registered %s <-> %s (2)", dst->getCname(), src->getCname()); return; } @@ -113,7 +113,7 @@ static void linkContainers(container_t src, container_t dst, std::set %s", src->name_.c_str(), dst->name_.c_str()); + XBT_DEBUG(" linkContainers %s <-> %s", src->getCname(), dst->getCname()); } static void recursiveGraphExtraction(simgrid::s4u::NetZone* netzone, container_t container, @@ -189,18 +189,15 @@ static void instr_routing_parse_start_link(simgrid::s4u::Link& link) { if (currentContainer.empty()) // No ongoing parsing. Are you creating the loopback? return; - container_t father = currentContainer.back(); - - double bandwidth_value = link.bandwidth(); - double latency_value = link.latency(); - container_t container = new simgrid::instr::Container(link.getCname(), simgrid::instr::INSTR_LINK, father); + container_t father = currentContainer.back(); + container_t container = new simgrid::instr::Container(link.getName(), simgrid::instr::INSTR_LINK, father); if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_link())) { simgrid::instr::Type* bandwidth = container->type_->getOrCreateVariableType("bandwidth", ""); simgrid::instr::Type* latency = container->type_->getOrCreateVariableType("latency", ""); - new simgrid::instr::SetVariableEvent(0, container, bandwidth, bandwidth_value); - new simgrid::instr::SetVariableEvent(0, container, latency, latency_value); + new simgrid::instr::SetVariableEvent(0, container, bandwidth, link.bandwidth()); + new simgrid::instr::SetVariableEvent(0, container, latency, link.latency()); } if (TRACE_uncategorized()) { container->type_->getOrCreateVariableType("bandwidth_used", "0.5 0.5 0.5"); @@ -210,14 +207,13 @@ static void instr_routing_parse_start_link(simgrid::s4u::Link& link) static void sg_instr_new_host(simgrid::s4u::Host& host) { container_t father = currentContainer.back(); - container_t container = new simgrid::instr::Container(host.getCname(), simgrid::instr::INSTR_HOST, father); + container_t container = new simgrid::instr::Container(host.getName(), simgrid::instr::INSTR_HOST, father); if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_speed())) { simgrid::instr::Type* speed = container->type_->getOrCreateVariableType("power", ""); - - double current_speed_state = host.getSpeed(); - new simgrid::instr::SetVariableEvent(0, container, speed, current_speed_state); + new simgrid::instr::SetVariableEvent(0, container, speed, host.getSpeed()); } + if (TRACE_uncategorized()) container->type_->getOrCreateVariableType("power_used", "0.5 0.5 0.5"); @@ -253,10 +249,8 @@ static void sg_instr_new_router(simgrid::kernel::routing::NetPoint * netpoint) { if (not netpoint->isRouter()) return; - if (TRACE_is_enabled() && TRACE_needs_platform()) { - container_t father = currentContainer.back(); - new simgrid::instr::Container(netpoint->getCname(), simgrid::instr::INSTR_ROUTER, father); - } + if (TRACE_is_enabled() && TRACE_needs_platform()) + new simgrid::instr::Container(netpoint->getCname(), simgrid::instr::INSTR_ROUTER, currentContainer.back()); } static void instr_routing_parse_end_platform () @@ -283,7 +277,7 @@ void instr_routing_define_callbacks () } simgrid::s4u::NetZone::onCreation.connect(sg_instr_AS_begin); simgrid::s4u::NetZone::onSeal.connect(sg_instr_AS_end); - simgrid::kernel::routing::NetPoint::onCreation.connect(&sg_instr_new_router); + simgrid::kernel::routing::NetPoint::onCreation.connect(sg_instr_new_router); } /* * user categories support -- 2.20.1