From 91393c642a33b6bc34e21bfd2465bdc645264c32 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 3 Sep 2017 22:34:17 +0200 Subject: [PATCH] give a proper constructor/destructor to instr::Container --- src/instr/instr_paje_containers.cpp | 132 ++++++++++++-------------- src/instr/instr_private.h | 5 +- src/msg/instr_msg_process.cpp | 6 +- src/msg/msg_vm.cpp | 7 +- src/plugins/vm/s4u_VirtualMachine.cpp | 2 +- src/smpi/internals/instr_smpi.cpp | 4 +- src/surf/instr_routing.cpp | 10 +- 7 files changed, 80 insertions(+), 86 deletions(-) diff --git a/src/instr/instr_paje_containers.cpp b/src/instr/instr_paje_containers.cpp index 9dea3fde79..24138376f5 100644 --- a/src/instr/instr_paje_containers.cpp +++ b/src/instr/instr_paje_containers.cpp @@ -38,7 +38,7 @@ void PJ_container_set_root (container_t root) rootContainer = root; } -container_t PJ_container_new(const char* name, simgrid::instr::e_container_types kind, container_t father) +simgrid::instr::Container::Container(const char* name, simgrid::instr::e_container_types kind, Container* father) { if (name == nullptr){ THROWF (tracing_error, 0, "can't create a container with a nullptr name"); @@ -49,56 +49,55 @@ container_t PJ_container_new(const char* name, simgrid::instr::e_container_types snprintf (id_str, INSTR_DEFAULT_STR_SIZE, "%lld", container_id); container_id++; - container_t newContainer = xbt_new0(simgrid::instr::Container, 1); - newContainer->name_ = xbt_strdup(name); // name of the container - newContainer->id_ = xbt_strdup(id_str); // id (or alias) of the container - newContainer->father_ = father; + name_ = xbt_strdup(name); // name of the container + id_ = xbt_strdup(id_str); // id (or alias) of the container + father_ = father; sg_host_t sg_host = sg_host_by_name(name); //Search for network_element_t switch (kind){ case simgrid::instr::INSTR_HOST: - newContainer->netpoint_ = sg_host->pimpl_netpoint; - xbt_assert(newContainer->netpoint_, "Element '%s' not found", name); + this->netpoint_ = sg_host->pimpl_netpoint; + xbt_assert(this->netpoint_, "Element '%s' not found", name); break; case simgrid::instr::INSTR_ROUTER: - newContainer->netpoint_ = simgrid::s4u::Engine::getInstance()->getNetpointByNameOrNull(name); - xbt_assert(newContainer->netpoint_, "Element '%s' not found", name); + this->netpoint_ = simgrid::s4u::Engine::getInstance()->getNetpointByNameOrNull(name); + xbt_assert(this->netpoint_, "Element '%s' not found", name); break; case simgrid::instr::INSTR_AS: - newContainer->netpoint_ = simgrid::s4u::Engine::getInstance()->getNetpointByNameOrNull(name); - xbt_assert(newContainer->netpoint_, "Element '%s' not found", name); + this->netpoint_ = simgrid::s4u::Engine::getInstance()->getNetpointByNameOrNull(name); + xbt_assert(this->netpoint_, "Element '%s' not found", name); break; default: - newContainer->netpoint_ = nullptr; + this->netpoint_ = nullptr; break; } // level depends on level of father - if (newContainer->father_) { - newContainer->level_ = newContainer->father_->level_ + 1; + if (this->father_) { + this->level_ = this->father_->level_ + 1; XBT_DEBUG("new container %s, child of %s", name, father->name_); }else{ - newContainer->level_ = 0; + this->level_ = 0; } // type definition (method depends on kind of this new container) - newContainer->kind_ = kind; - if (newContainer->kind_ == simgrid::instr::INSTR_AS) { + this->kind_ = kind; + if (this->kind_ == simgrid::instr::INSTR_AS) { //if this container is of an AS, its type name depends on its level char as_typename[INSTR_DEFAULT_STR_SIZE]; - snprintf(as_typename, INSTR_DEFAULT_STR_SIZE, "L%d", newContainer->level_); - if (newContainer->father_) { - newContainer->type_ = simgrid::instr::Type::getOrNull(as_typename, newContainer->father_->type_); - if (newContainer->type_ == nullptr) { - newContainer->type_ = simgrid::instr::Type::containerNew(as_typename, newContainer->father_->type_); + snprintf(as_typename, INSTR_DEFAULT_STR_SIZE, "L%d", this->level_); + if (this->father_) { + this->type_ = simgrid::instr::Type::getOrNull(as_typename, this->father_->type_); + if (this->type_ == nullptr) { + this->type_ = simgrid::instr::Type::containerNew(as_typename, this->father_->type_); } }else{ - newContainer->type_ = simgrid::instr::Type::containerNew("0", nullptr); + this->type_ = simgrid::instr::Type::containerNew("0", nullptr); } }else{ //otherwise, the name is its kind char typeNameBuff[INSTR_DEFAULT_STR_SIZE]; - switch (newContainer->kind_) { + switch (this->kind_) { case simgrid::instr::INSTR_HOST: snprintf (typeNameBuff, INSTR_DEFAULT_STR_SIZE, "HOST"); break; @@ -124,33 +123,56 @@ container_t PJ_container_new(const char* name, simgrid::instr::e_container_types THROWF (tracing_error, 0, "new container kind is unknown."); break; } - simgrid::instr::Type* type = simgrid::instr::Type::getOrNull(typeNameBuff, newContainer->father_->type_); + simgrid::instr::Type* type = simgrid::instr::Type::getOrNull(typeNameBuff, this->father_->type_); if (type == nullptr){ - newContainer->type_ = simgrid::instr::Type::containerNew(typeNameBuff, newContainer->father_->type_); + this->type_ = simgrid::instr::Type::containerNew(typeNameBuff, this->father_->type_); }else{ - newContainer->type_ = type; + this->type_ = type; } } - newContainer->children_ = xbt_dict_new_homogeneous(nullptr); - if (newContainer->father_) { - xbt_dict_set(newContainer->father_->children_, newContainer->name_, newContainer, nullptr); - LogContainerCreation(newContainer); + this->children_ = xbt_dict_new_homogeneous(nullptr); + if (this->father_) { + xbt_dict_set(this->father_->children_, this->name_, this, nullptr); + LogContainerCreation(this); } //register all kinds by name - if (xbt_dict_get_or_null(allContainers, newContainer->name_) != nullptr) { - THROWF(tracing_error, 1, "container %s already present in allContainers data structure", newContainer->name_); + if (xbt_dict_get_or_null(allContainers, this->name_) != nullptr) { + THROWF(tracing_error, 1, "container %s already present in allContainers data structure", this->name_); } - xbt_dict_set(allContainers, newContainer->name_, newContainer, nullptr); - XBT_DEBUG("Add container name '%s'", newContainer->name_); + xbt_dict_set(allContainers, this->name_, this, nullptr); + XBT_DEBUG("Add container name '%s'", this->name_); //register NODE types for triva configuration - if (newContainer->kind_ == simgrid::instr::INSTR_HOST || newContainer->kind_ == simgrid::instr::INSTR_LINK || - newContainer->kind_ == simgrid::instr::INSTR_ROUTER) { - trivaNodeTypes.insert(newContainer->type_->name_); + if (this->kind_ == simgrid::instr::INSTR_HOST || this->kind_ == simgrid::instr::INSTR_LINK || + this->kind_ == simgrid::instr::INSTR_ROUTER) { + trivaNodeTypes.insert(this->type_->name_); } - return newContainer; +} +simgrid::instr::Container::~Container() +{ + XBT_DEBUG("destroy container %s", name_); + + // obligation to dump previous events because they might + // reference the container that is about to be destroyed + TRACE_last_timestamp_to_dump = surf_get_clock(); + TRACE_paje_dump_buffer(1); + + // trace my destruction + if (not TRACE_disable_destroy() && this != PJ_container_get_root()) { + // do not trace the container destruction if user requests + // or if the container is root + LogContainerDestruction(this); + } + + // remove it from allContainers data structure + xbt_dict_remove(allContainers, name_); + + // free + xbt_free(name_); + xbt_free(id_); + xbt_dict_free(&children_); } container_t PJ_container_get (const char *name) @@ -185,36 +207,6 @@ void PJ_container_remove_from_parent (container_t child) } } -void PJ_container_free (container_t container) -{ - if (container == nullptr){ - THROWF (tracing_error, 0, "trying to free a nullptr container"); - } - XBT_DEBUG("destroy container %s", container->name_); - - //obligation to dump previous events because they might - //reference the container that is about to be destroyed - TRACE_last_timestamp_to_dump = surf_get_clock(); - TRACE_paje_dump_buffer(1); - - //trace my destruction - if (not TRACE_disable_destroy() && container != PJ_container_get_root()) { - //do not trace the container destruction if user requests - //or if the container is root - LogContainerDestruction(container); - } - - //remove it from allContainers data structure - xbt_dict_remove(allContainers, container->name_); - - //free - xbt_free(container->name_); - xbt_free(container->id_); - xbt_dict_free(&container->children_); - xbt_free (container); - container = nullptr; -} - static void recursiveDestroyContainer (container_t container) { if (container == nullptr){ @@ -227,7 +219,7 @@ static void recursiveDestroyContainer (container_t container) xbt_dict_foreach (container->children_, cursor, child_name, child) { recursiveDestroyContainer (child); } - PJ_container_free (container); + delete container; } void PJ_container_free_all () diff --git a/src/instr/instr_private.h b/src/instr/instr_private.h index 3b3e272b7f..6ed519ef7d 100644 --- a/src/instr/instr_private.h +++ b/src/instr/instr_private.h @@ -109,6 +109,9 @@ typedef enum { class Container { public: + Container(const char* name, simgrid::instr::e_container_types kind, Container* father); + virtual ~Container(); + sg_netpoint_t netpoint_; char* name_; /* Unique name of this container */ char* id_; /* Unique id of this container */ @@ -325,12 +328,10 @@ extern XBT_PRIVATE std::set trivaEdgeTypes; XBT_PRIVATE long long int instr_new_paje_id (); XBT_PRIVATE void PJ_container_alloc (); XBT_PRIVATE void PJ_container_release (); -XBT_PUBLIC(container_t) PJ_container_new(const char* name, simgrid::instr::e_container_types kind, container_t father); XBT_PUBLIC(container_t) PJ_container_get (const char *name); XBT_PUBLIC(container_t) PJ_container_get_or_null (const char *name); XBT_PUBLIC(container_t) PJ_container_get_root (); XBT_PUBLIC(void) PJ_container_set_root (container_t root); -XBT_PUBLIC(void) PJ_container_free (container_t container); XBT_PUBLIC(void) PJ_container_free_all (void); XBT_PUBLIC(void) PJ_container_remove_from_parent (container_t container); diff --git a/src/msg/instr_msg_process.cpp b/src/msg/instr_msg_process.cpp index 9b5fc3b618..af391fb2d6 100644 --- a/src/msg/instr_msg_process.cpp +++ b/src/msg/instr_msg_process.cpp @@ -60,8 +60,8 @@ void TRACE_msg_process_create (const char *process_name, int process_pid, msg_ho char str[INSTR_DEFAULT_STR_SIZE]; container_t host_container = PJ_container_get(host->getCname()); - PJ_container_new(instr_process_id_2(process_name, process_pid, str, len), simgrid::instr::INSTR_MSG_PROCESS, - host_container); + new simgrid::instr::Container(instr_process_id_2(process_name, process_pid, str, len), + simgrid::instr::INSTR_MSG_PROCESS, host_container); } } @@ -74,7 +74,7 @@ void TRACE_msg_process_destroy (const char *process_name, int process_pid) container_t process = PJ_container_get_or_null(instr_process_id_2(process_name, process_pid, str, len)); if (process) { PJ_container_remove_from_parent (process); - PJ_container_free (process); + delete process; } } } diff --git a/src/msg/msg_vm.cpp b/src/msg/msg_vm.cpp index 4c82558cee..be46babecb 100644 --- a/src/msg/msg_vm.cpp +++ b/src/msg/msg_vm.cpp @@ -173,7 +173,7 @@ void MSG_vm_destroy(msg_vm_t vm) if (TRACE_msg_vm_is_enabled()) { container_t container = PJ_container_get(vm->getCname()); PJ_container_remove_from_parent(container); - PJ_container_free(container); + delete container; } } @@ -302,10 +302,11 @@ static int migration_rx_fun(int argc, char *argv[]) // destroy existing container of this vm container_t existing_container = PJ_container_get(vm->getCname()); PJ_container_remove_from_parent(existing_container); - PJ_container_free(existing_container); + delete existing_container; // create new container on the new_host location - PJ_container_new(vm->getCname(), simgrid::instr::INSTR_MSG_VM, PJ_container_get(ms->dst_pm->getCname())); + new simgrid::instr::Container(vm->getCname(), simgrid::instr::INSTR_MSG_VM, + PJ_container_get(ms->dst_pm->getCname())); // end link msg = PJ_container_get(vm->getCname()); diff --git a/src/plugins/vm/s4u_VirtualMachine.cpp b/src/plugins/vm/s4u_VirtualMachine.cpp index fe298d79df..14c29fec3d 100644 --- a/src/plugins/vm/s4u_VirtualMachine.cpp +++ b/src/plugins/vm/s4u_VirtualMachine.cpp @@ -33,7 +33,7 @@ VirtualMachine::VirtualMachine(const char* name, s4u::Host* pm, int coreAmount) if (TRACE_msg_vm_is_enabled()) { container_t host_container = PJ_container_get(pm->getCname()); - PJ_container_new(name, simgrid::instr::INSTR_MSG_VM, host_container); + new simgrid::instr::Container(name, simgrid::instr::INSTR_MSG_VM, host_container); } } diff --git a/src/smpi/internals/instr_smpi.cpp b/src/smpi/internals/instr_smpi.cpp index ba34ebd9bd..288a8e3267 100644 --- a/src/smpi/internals/instr_smpi.cpp +++ b/src/smpi/internals/instr_smpi.cpp @@ -199,7 +199,7 @@ void TRACE_smpi_init(int rank) #if HAVE_PAPI container_t container = #endif - PJ_container_new(str, simgrid::instr::INSTR_SMPI, father); + new simgrid::instr::Container(str, simgrid::instr::INSTR_SMPI, father); #if HAVE_PAPI papi_counter_t counters = smpi_process()->papi_counters(); @@ -223,7 +223,7 @@ void TRACE_smpi_finalize(int rank) char str[INSTR_DEFAULT_STR_SIZE]; container_t container = PJ_container_get(smpi_container(rank, str, INSTR_DEFAULT_STR_SIZE)); PJ_container_remove_from_parent (container); - PJ_container_free (container); + delete container; } void TRACE_smpi_collective_in(int rank, const char *operation, instr_extra_data extra) diff --git a/src/surf/instr_routing.cpp b/src/surf/instr_routing.cpp index 7f31041958..1106ad91da 100644 --- a/src/surf/instr_routing.cpp +++ b/src/surf/instr_routing.cpp @@ -167,7 +167,7 @@ static void sg_instr_AS_begin(simgrid::s4u::NetZone& netzone) if (PJ_container_get_root() == nullptr){ PJ_container_alloc (); - container_t root = PJ_container_new(id, simgrid::instr::INSTR_AS, nullptr); + container_t root = new simgrid::instr::Container(id, simgrid::instr::INSTR_AS, nullptr); PJ_container_set_root (root); if (TRACE_smpi_is_enabled()) { @@ -188,7 +188,7 @@ static void sg_instr_AS_begin(simgrid::s4u::NetZone& netzone) if (TRACE_needs_platform()){ container_t father = currentContainer.back(); - container_t container = PJ_container_new(id, simgrid::instr::INSTR_AS, father); + container_t container = new simgrid::instr::Container(id, simgrid::instr::INSTR_AS, father); currentContainer.push_back(container); } } @@ -209,7 +209,7 @@ static void instr_routing_parse_start_link(simgrid::s4u::Link& link) double bandwidth_value = link.bandwidth(); double latency_value = link.latency(); - container_t container = PJ_container_new(link.name(), simgrid::instr::INSTR_LINK, father); + container_t container = new simgrid::instr::Container(link.name(), simgrid::instr::INSTR_LINK, father); if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_link())) { simgrid::instr::Type* bandwidth = simgrid::instr::Type::getOrNull("bandwidth", container->type_); @@ -234,7 +234,7 @@ 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 = PJ_container_new(host.getCname(), simgrid::instr::INSTR_HOST, father); + container_t container = new simgrid::instr::Container(host.getCname(), simgrid::instr::INSTR_HOST, father); if ((TRACE_categorized() || TRACE_uncategorized() || TRACE_platform()) && (not TRACE_disable_speed())) { simgrid::instr::Type* speed = simgrid::instr::Type::getOrNull("power", container->type_); @@ -298,7 +298,7 @@ static void sg_instr_new_router(simgrid::kernel::routing::NetPoint * netpoint) return; if (TRACE_is_enabled() && TRACE_needs_platform()) { container_t father = currentContainer.back(); - PJ_container_new(netpoint->cname(), simgrid::instr::INSTR_ROUTER, father); + new simgrid::instr::Container(netpoint->cname(), simgrid::instr::INSTR_ROUTER, father); } } -- 2.20.1