Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
do not allocate/free properties
authorSUTER Frederic <frederic.suter@cc.in2p3.fr>
Mon, 12 Apr 2021 15:32:48 +0000 (17:32 +0200)
committerSUTER Frederic <frederic.suter@cc.in2p3.fr>
Mon, 12 Apr 2021 15:32:48 +0000 (17:32 +0200)
include/simgrid/s4u/Disk.hpp
src/kernel/actor/ActorImpl.cpp
src/kernel/actor/ActorImpl.hpp
src/s4u/s4u_Actor.cpp
src/s4u/s4u_Disk.cpp
src/surf/HostImpl.cpp
src/surf/sg_platf.cpp
src/surf/xml/platf_private.hpp
src/surf/xml/surfxml_sax_cb.cpp

index 2ef3828..323f40e 100644 (file)
@@ -68,6 +68,7 @@ public:
   const std::unordered_map<std::string, std::string>* get_properties() const;
   const char* get_property(const std::string& key) const;
   Disk* set_property(const std::string&, const std::string& value);
+  Disk* set_properties(const std::unordered_map<std::string, std::string>& properties);
   Disk* set_host(Host* host);
   Host* get_host() const;
 
index 5fb355b..c09c773 100644 (file)
@@ -90,7 +90,7 @@ ActorImpl::~ActorImpl()
  */
 
 ActorImplPtr ActorImpl::attach(const std::string& name, void* data, s4u::Host* host,
-                               const std::unordered_map<std::string, std::string>* properties)
+                               const std::unordered_map<std::string, std::string>& properties)
 {
   // This is mostly a copy/paste from create(), it'd be nice to share some code between those two functions.
 
@@ -111,8 +111,7 @@ ActorImplPtr ActorImpl::attach(const std::string& name, void* data, s4u::Host* h
   actor->context_.reset(simix_global->context_factory->attach(actor));
 
   /* Add properties */
-  if (properties != nullptr)
-    actor->set_properties(*properties);
+  actor->set_properties(properties);
 
   /* Add the actor to it's host actor list */
   host->get_impl()->add_actor(actor);
@@ -366,7 +365,7 @@ s4u::Actor* ActorImpl::restart()
   context::Context::self()->get_actor()->kill(this);
 
   // start the new actor
-  ActorImplPtr actor = ActorImpl::create(arg.name, arg.code, arg.data, arg.host, arg.properties.get(), nullptr);
+  ActorImplPtr actor = ActorImpl::create(arg.name, arg.code, arg.data, arg.host, arg.properties, nullptr);
   *actor->on_exit = std::move(*arg.on_exit);
   actor->set_kill_time(arg.kill_time);
   actor->set_auto_restart(arg.auto_restart);
@@ -509,7 +508,7 @@ ActorImpl* ActorImpl::start(const ActorCode& code)
 }
 
 ActorImplPtr ActorImpl::create(const std::string& name, const ActorCode& code, void* data, s4u::Host* host,
-                               const std::unordered_map<std::string, std::string>* properties,
+                               const std::unordered_map<std::string, std::string>& properties,
                                const ActorImpl* parent_actor)
 {
   XBT_DEBUG("Start actor %s@'%s'", name.c_str(), host->get_cname());
@@ -524,8 +523,7 @@ ActorImplPtr ActorImpl::create(const std::string& name, const ActorCode& code, v
   actor->set_user_data(data);
 
   /* Add properties */
-  if (properties != nullptr)
-    actor->set_properties(*properties);
+  actor->set_properties(properties);
 
   actor->start(code);
 
index b820560..6289efd 100644 (file)
@@ -121,10 +121,10 @@ public:
   ActorImpl* start(const ActorCode& code);
 
   static ActorImplPtr create(const std::string& name, const ActorCode& code, void* data, s4u::Host* host,
-                             const std::unordered_map<std::string, std::string>* properties,
+                             const std::unordered_map<std::string, std::string>& properties,
                              const ActorImpl* parent_actor);
   static ActorImplPtr attach(const std::string& name, void* data, s4u::Host* host,
-                             const std::unordered_map<std::string, std::string>* properties);
+                             const std::unordered_map<std::string, std::string>& properties);
   static void detach();
   void cleanup();
   void exit();
@@ -156,7 +156,7 @@ public:
   void* data                                                               = nullptr;
   s4u::Host* host                                                          = nullptr;
   double kill_time                                                         = 0.0;
-  std::shared_ptr<const std::unordered_map<std::string, std::string>> properties = nullptr;
+  const std::unordered_map<std::string, std::string> properties;
   bool auto_restart                                                        = false;
   bool daemon_                                                             = false;
   /* list of functions executed when the process dies */
@@ -165,7 +165,7 @@ public:
   ProcessArg()                                                             = default;
 
   explicit ProcessArg(const std::string& name, const std::function<void()>& code, void* data, s4u::Host* host,
-                      double kill_time, std::shared_ptr<std::unordered_map<std::string, std::string>> properties,
+                      double kill_time, const std::unordered_map<std::string, std::string>& properties,
                       bool auto_restart)
       : name(name)
       , code(code)
@@ -187,7 +187,6 @@ public:
       , daemon_(actor->is_daemon())
       , on_exit(actor->on_exit)
   {
-    properties.reset(actor->get_properties(), [](decltype(actor->get_properties())) {});
   }
 };
 
index 944b97c..7cb8a48 100644 (file)
@@ -745,7 +745,7 @@ sg_actor_t sg_actor_attach(const char* name, void* data, sg_host_t host, xbt_dic
   /* Let's create the process: SIMIX may decide to start it right now, even before returning the flow control to us */
   smx_actor_t actor = nullptr;
   try {
-    actor = simgrid::kernel::actor::ActorImpl::attach(name, data, host, &props).get();
+    actor = simgrid::kernel::actor::ActorImpl::attach(name, data, host, props).get();
   } catch (simgrid::HostFailureException const&) {
     xbt_die("Could not attach");
   }
index 265b7f5..28e37b1 100644 (file)
@@ -75,6 +75,12 @@ Disk* Disk::set_property(const std::string& key, const std::string& value)
   return this;
 }
 
+Disk* Disk::set_properties(const std::unordered_map<std::string, std::string>& properties)
+{
+  kernel::actor::simcall([this, properties] { this->pimpl_->set_properties(properties); });
+  return this;
+}
+
 IoPtr Disk::io_init(sg_size_t size, Io::OpType type) const
 {
   return Io::init()->set_disk(this)->set_size(size)->set_op_type(type);
index 7b34661..0279685 100644 (file)
@@ -76,8 +76,8 @@ void HostImpl::turn_on() const
 {
   for (auto const& arg : actors_at_boot_) {
     XBT_DEBUG("Booting Actor %s(%s) right now", arg->name.c_str(), arg->host->get_cname());
-    simgrid::kernel::actor::ActorImplPtr actor = simgrid::kernel::actor::ActorImpl::create(
-        arg->name, arg->code, nullptr, arg->host, arg->properties.get(), nullptr);
+    simgrid::kernel::actor::ActorImplPtr actor =
+        simgrid::kernel::actor::ActorImpl::create(arg->name, arg->code, nullptr, arg->host, arg->properties, nullptr);
     if (arg->on_exit)
       *actor->on_exit = *arg->on_exit;
     if (arg->kill_time >= 0)
index 0db180c..613f2a9 100644 (file)
@@ -63,13 +63,11 @@ void sg_platf_exit()
 /** @brief Add a host to the current NetZone */
 void sg_platf_new_host(const simgrid::kernel::routing::HostCreationArgs* args)
 {
-  simgrid::s4u::Host* host =
-      routing_get_current()->create_host(args->id, args->speed_per_pstate)->set_coordinates(args->coord);
+  simgrid::s4u::Host* host = routing_get_current()
+                                 ->create_host(args->id, args->speed_per_pstate)
+                                 ->set_coordinates(args->coord)
+                                 ->set_properties(args->properties);
 
-  if (args->properties) {
-    host->set_properties(*args->properties);
-    delete args->properties;
-  }
   host->get_impl()->set_disks(args->disks);
 
   host->pimpl_cpu->set_core_count(args->core_amount)
@@ -102,11 +100,10 @@ simgrid::kernel::routing::NetPoint* sg_platf_new_router(const std::string& name,
 
 static void sg_platf_new_link(const simgrid::kernel::routing::LinkCreationArgs* args, const std::string& link_name)
 {
-  simgrid::s4u::Link* link = routing_get_current()->create_link(link_name, args->bandwidths);
-  if (args->properties)
-    link->set_properties(*args->properties);
-
-  link->get_impl() // this call to get_impl saves some simcalls but can be removed
+  routing_get_current()
+      ->create_link(link_name, args->bandwidths)
+      ->set_properties(args->properties)
+      ->get_impl() // this call to get_impl saves some simcalls but can be removed
       ->set_sharing_policy(args->policy)
       ->set_state_profile(args->state_trace)
       ->set_latency_profile(args->latency_trace)
@@ -123,7 +120,6 @@ void sg_platf_new_link(const simgrid::kernel::routing::LinkCreationArgs* link)
   } else {
     sg_platf_new_link(link, link->id);
   }
-  delete link->properties;
 }
 
 void sg_platf_new_cluster(simgrid::kernel::routing::ClusterCreationArgs* cluster)
@@ -155,9 +151,8 @@ void sg_platf_new_cluster(simgrid::kernel::routing::ClusterCreationArgs* cluster
   sg_platf_new_Zone_begin(&zone);
   auto* current_zone = static_cast<ClusterZone*>(routing_get_current());
   current_zone->parse_specific_arguments(cluster);
-  if (cluster->properties != nullptr)
-    for (auto const& elm : *cluster->properties)
-      current_zone->get_iface()->set_property(elm.first, elm.second);
+  for (auto const& elm : cluster->properties)
+    current_zone->get_iface()->set_property(elm.first, elm.second);
 
   if (cluster->loopback_bw > 0 || cluster->loopback_lat > 0) {
     current_zone->set_loopback();
@@ -171,12 +166,10 @@ void sg_platf_new_cluster(simgrid::kernel::routing::ClusterCreationArgs* cluster
     std::string host_id = std::string(cluster->prefix) + std::to_string(i) + cluster->suffix;
 
     XBT_DEBUG("<host\tid=\"%s\"\tspeed=\"%f\">", host_id.c_str(), cluster->speeds.front());
-    simgrid::s4u::Host* host =
-        current_zone->create_host(host_id, cluster->speeds)->set_core_count(cluster->core_amount);
-
-    if ((cluster->properties != nullptr) && (not cluster->properties->empty()))
-      host->set_properties(*cluster->properties);
-    host->seal();
+    current_zone->create_host(host_id, cluster->speeds)
+        ->set_core_count(cluster->core_amount)
+        ->set_properties(cluster->properties)
+        ->seal();
 
     XBT_DEBUG("</host>");
 
@@ -222,7 +215,6 @@ void sg_platf_new_cluster(simgrid::kernel::routing::ClusterCreationArgs* cluster
     }
     rankId++;
   }
-  delete cluster->properties;
 
   // Add a router.
   XBT_DEBUG(" ");
@@ -279,13 +271,10 @@ void sg_platf_new_cabinet(const simgrid::kernel::routing::CabinetCreationArgs* c
 
 simgrid::kernel::resource::DiskImpl* sg_platf_new_disk(const simgrid::kernel::routing::DiskCreationArgs* disk)
 {
-  simgrid::kernel::resource::DiskImpl* pimpl =
-      routing_get_current()->create_disk(disk->id, disk->read_bw, disk->write_bw)->get_impl();
-
-  if (disk->properties) {
-    pimpl->set_properties(*disk->properties);
-    delete disk->properties;
-  }
+  simgrid::kernel::resource::DiskImpl* pimpl = routing_get_current()
+                                                   ->create_disk(disk->id, disk->read_bw, disk->write_bw)
+                                                   ->set_properties(disk->properties)
+                                                   ->get_impl();
 
   pimpl->seal();
   simgrid::s4u::Disk::on_creation(*pimpl->get_iface());
@@ -335,20 +324,20 @@ void sg_platf_new_actor(simgrid::kernel::routing::ActorCreationArgs* actor)
 
   std::string actor_name                 = actor->args[0];
   simgrid::kernel::actor::ActorCode code = factory(std::move(actor->args));
-  std::shared_ptr<std::unordered_map<std::string, std::string>> properties(actor->properties);
 
-  auto* arg =
-      new simgrid::kernel::actor::ProcessArg(actor_name, code, nullptr, host, kill_time, properties, auto_restart);
+  auto* arg = new simgrid::kernel::actor::ProcessArg(actor_name, code, nullptr, host, kill_time, actor->properties,
+                                                     auto_restart);
 
   host->get_impl()->add_actor_at_boot(arg);
 
   if (start_time > SIMIX_get_clock()) {
-    arg = new simgrid::kernel::actor::ProcessArg(actor_name, code, nullptr, host, kill_time, properties, auto_restart);
+    arg = new simgrid::kernel::actor::ProcessArg(actor_name, code, nullptr, host, kill_time, actor->properties,
+                                                 auto_restart);
 
     XBT_DEBUG("Process %s@%s will be started at time %f", arg->name.c_str(), arg->host->get_cname(), start_time);
     simgrid::simix::Timer::set(start_time, [arg, auto_restart]() {
       simgrid::kernel::actor::ActorImplPtr new_actor = simgrid::kernel::actor::ActorImpl::create(
-          arg->name.c_str(), arg->code, arg->data, arg->host, arg->properties.get(), nullptr);
+          arg->name.c_str(), arg->code, arg->data, arg->host, arg->properties, nullptr);
       if (arg->kill_time >= 0)
         new_actor->set_kill_time(arg->kill_time);
       if (auto_restart)
@@ -360,8 +349,8 @@ void sg_platf_new_actor(simgrid::kernel::routing::ActorCreationArgs* actor)
 
     try {
       simgrid::kernel::actor::ActorImplPtr new_actor = nullptr;
-      new_actor = simgrid::kernel::actor::ActorImpl::create(arg->name.c_str(), code, nullptr, host,
-                                                            arg->properties.get(), nullptr);
+      new_actor =
+          simgrid::kernel::actor::ActorImpl::create(arg->name.c_str(), code, nullptr, host, arg->properties, nullptr);
       /* The actor creation will fail if the host is currently dead, but that's fine */
       if (arg->kill_time >= 0)
         new_actor->set_kill_time(arg->kill_time);
@@ -457,12 +446,11 @@ simgrid::kernel::routing::NetZoneImpl* sg_platf_new_Zone_begin(const simgrid::ke
   return new_zone;
 }
 
-void sg_platf_new_Zone_set_properties(const std::unordered_map<std::string, std::string>* props)
+void sg_platf_new_Zone_set_properties(const std::unordered_map<std::string, std::string>& props)
 {
   xbt_assert(current_routing, "Cannot set properties of the current Zone: none under construction");
 
-  if (props)
-    current_routing->set_properties(*props);
+  current_routing->set_properties(props);
 }
 
 /**
index c48c9be..7ea69f6 100644 (file)
@@ -41,7 +41,7 @@ public:
   profile::Profile* speed_trace                            = nullptr;
   profile::Profile* state_trace                            = nullptr;
   std::string coord                                        = "";
-  std::unordered_map<std::string, std::string>* properties = nullptr;
+  std::unordered_map<std::string, std::string> properties;
   std::vector<resource::DiskImpl*> disks;
 };
 
@@ -61,7 +61,7 @@ public:
   profile::Profile* latency_trace                          = nullptr;
   profile::Profile* state_trace                            = nullptr;
   s4u::Link::SharingPolicy policy                          = s4u::Link::SharingPolicy::FATPIPE;
-  std::unordered_map<std::string, std::string>* properties = nullptr;
+  std::unordered_map<std::string, std::string> properties;
 };
 
 class PeerCreationArgs {
@@ -104,7 +104,7 @@ public:
   double limiter_link = 0;
   ClusterTopology topology = ClusterTopology::FLAT;
   std::string topo_parameters;
-  std::unordered_map<std::string, std::string>* properties = nullptr;
+  std::unordered_map<std::string, std::string> properties;
   std::string router_id;
   s4u::Link::SharingPolicy sharing_policy    = s4u::Link::SharingPolicy::SPLITDUPLEX;
   s4u::Link::SharingPolicy bb_sharing_policy = s4u::Link::SharingPolicy::SHARED;
@@ -124,7 +124,7 @@ public:
 class DiskCreationArgs {
 public:
   std::string id;
-  std::unordered_map<std::string, std::string>* properties;
+  std::unordered_map<std::string, std::string> properties;
   double read_bw;
   double write_bw;
 };
@@ -149,7 +149,7 @@ public:
 class ActorCreationArgs {
 public:
   std::vector<std::string> args;
-  std::unordered_map<std::string, std::string>* properties = nullptr;
+  std::unordered_map<std::string, std::string> properties;
   const char* host                       = nullptr;
   const char* function                   = nullptr;
   double start_time                      = 0.0;
@@ -175,7 +175,7 @@ void routing_cluster_add_backbone(simgrid::kernel::resource::LinkImpl* bb);
 
 XBT_PUBLIC simgrid::kernel::routing::NetZoneImpl*
 sg_platf_new_Zone_begin(const simgrid::kernel::routing::ZoneCreationArgs* zone); // Begin description of new Zone
-XBT_PUBLIC void sg_platf_new_Zone_set_properties(const std::unordered_map<std::string, std::string>* props);
+XBT_PUBLIC void sg_platf_new_Zone_set_properties(const std::unordered_map<std::string, std::string>& props);
 XBT_PUBLIC void sg_platf_new_Zone_seal();                                          // That Zone is fully described
 
 XBT_PUBLIC void
index afe1337..a2ec81f 100644 (file)
@@ -129,10 +129,10 @@ static void explodesRadical(const std::string& radicals, std::vector<int>* explo
 
 /* make sure these symbols are defined as strong ones in this file so that the linker can resolve them */
 
-std::vector<std::unordered_map<std::string, std::string>*> property_sets;
+std::vector<std::unordered_map<std::string, std::string>> property_sets;
 
 /* The default current property receiver. Setup in the corresponding opening callbacks. */
-std::unordered_map<std::string, std::string>* current_model_property_set = nullptr;
+std::unordered_map<std::string, std::string> current_model_property_set;
 
 FILE *surf_file_to_parse = nullptr;
 
@@ -227,14 +227,14 @@ void ETag_surfxml_platform(){
 }
 
 void STag_surfxml_host(){
-  property_sets.push_back(new std::unordered_map<std::string, std::string>());
+  property_sets.push_back(std::unordered_map<std::string, std::string>());
 }
 
 void STag_surfxml_prop()
 {
-  property_sets.back()->insert({A_surfxml_prop_id, A_surfxml_prop_value});
+  property_sets.back().insert({A_surfxml_prop_id, A_surfxml_prop_value});
   XBT_DEBUG("add prop %s=%s into current property set %p", A_surfxml_prop_id, A_surfxml_prop_value,
-            property_sets.back());
+            &(property_sets.back()));
 }
 
 void ETag_surfxml_host()    {
@@ -268,7 +268,7 @@ void ETag_surfxml_host()    {
 }
 
 void STag_surfxml_disk() {
-  property_sets.push_back(new std::unordered_map<std::string, std::string>());
+  property_sets.push_back(std::unordered_map<std::string, std::string>());
 }
 
 void ETag_surfxml_disk() {
@@ -384,7 +384,7 @@ void ETag_surfxml_cluster(){
 }
 
 void STag_surfxml_cluster(){
-  property_sets.push_back(new std::unordered_map<std::string, std::string>());
+  property_sets.push_back(std::unordered_map<std::string, std::string>());
 }
 
 void STag_surfxml_cabinet(){
@@ -433,7 +433,7 @@ void STag_surfxml_peer(){
 }
 
 void STag_surfxml_link(){
-  property_sets.push_back(new std::unordered_map<std::string, std::string>());
+  property_sets.push_back(std::unordered_map<std::string, std::string>());
 }
 
 void ETag_surfxml_link(){
@@ -698,7 +698,7 @@ void ETag_surfxml_AS()
 
 void STag_surfxml_zone()
 {
-  property_sets.push_back(new std::unordered_map<std::string, std::string>());
+  property_sets.push_back(std::unordered_map<std::string, std::string>());
   simgrid::kernel::routing::ZoneCreationArgs zone;
   zone.id      = A_surfxml_zone_id;
   zone.routing = A_surfxml_zone_routing;
@@ -708,7 +708,6 @@ void STag_surfxml_zone()
 void ETag_surfxml_zone()
 {
   sg_platf_new_Zone_set_properties(property_sets.back());
-  delete property_sets.back();
   property_sets.pop_back();
 
   sg_platf_new_Zone_seal();
@@ -716,7 +715,7 @@ void ETag_surfxml_zone()
 
 void STag_surfxml_config()
 {
-  property_sets.push_back(new std::unordered_map<std::string, std::string>());
+  property_sets.push_back(std::unordered_map<std::string, std::string>());
   XBT_DEBUG("START configuration name = %s",A_surfxml_config_id);
   if (_sg_cfg_init_status == 2) {
     surf_parse_error("All <config> tags must be given before any platform elements (such as <zone>, <host>, <cluster>, "
@@ -731,20 +730,19 @@ void ETag_surfxml_config()
   auto current_property_set = property_sets.back();
 
   std::vector<std::string> keys;
-  for (auto const& kv : *current_property_set) {
+  for (auto const& kv : current_property_set) {
     keys.push_back(kv.first);
   }
   std::sort(keys.begin(), keys.end());
   for (std::string key : keys) {
     if (simgrid::config::is_default(key.c_str())) {
-      std::string cfg = key + ":" + current_property_set->at(key);
+      std::string cfg = key + ":" + current_property_set.at(key);
       simgrid::config::set_parse(cfg);
     } else
       XBT_INFO("The custom configuration '%s' is already defined by user!", key.c_str());
   }
   XBT_DEBUG("End configuration name = %s",A_surfxml_config_id);
 
-  delete current_property_set;
   property_sets.pop_back();
 }
 
@@ -758,7 +756,7 @@ void STag_surfxml_process()
 
 void STag_surfxml_actor()
 {
-  property_sets.push_back(new std::unordered_map<std::string, std::string>());
+  property_sets.push_back(std::unordered_map<std::string, std::string>());
   arguments.assign(1, A_surfxml_actor_function);
 }
 
@@ -805,10 +803,7 @@ void STag_surfxml_argument(){
 }
 
 void STag_surfxml_model___prop(){
-  if (not current_model_property_set)
-    current_model_property_set = new std::unordered_map<std::string, std::string>();
-
-  current_model_property_set->insert({A_surfxml_model___prop_id, A_surfxml_model___prop_value});
+  current_model_property_set.insert({A_surfxml_model___prop_id, A_surfxml_model___prop_value});
 }
 
 void ETag_surfxml_prop(){/* Nothing to do */}