From: Frederic Suter <frederic.suter@cc.in2p3.fr>
Date: Mon, 12 Mar 2018 10:30:37 +0000 (+0100)
Subject: Move the list of storages to the engine
X-Git-Tag: v3.19~80
X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/1ff58d7a8f46e07cdc902f96b99bb47d0a077932

Move the list of storages to the engine

Not completely satisfied about how StorageImpl are freed but this
works and I spent too much time on finding a better solution today.
---

diff --git a/include/simgrid/s4u/Engine.hpp b/include/simgrid/s4u/Engine.hpp
index cd42aec895..c4e7d2b0e9 100644
--- a/include/simgrid/s4u/Engine.hpp
+++ b/include/simgrid/s4u/Engine.hpp
@@ -56,12 +56,17 @@ public:
 
 protected:
   friend s4u::Host;
+  friend s4u::Storage;
   void addHost(std::string name, simgrid::s4u::Host * host);
   void delHost(std::string name);
 
 public:
+  void addStorage(std::string name, simgrid::s4u::Storage * storage);
+  void delStorage(std::string name);
   simgrid::s4u::Host* hostByName(std::string name);
   simgrid::s4u::Host* hostByNameOrNull(std::string name);
+  simgrid::s4u::Storage* storageByName(std::string name);
+  simgrid::s4u::Storage* storageByNameOrNull(std::string name);
 
   size_t getHostCount();
   void getHostList(std::vector<Host*> * whereTo);
@@ -71,6 +76,8 @@ public:
   void getLinkList(std::vector<Link*> * list);
   std::vector<Link*> getAllLinks();
 
+  std::vector<Storage*> getAllStorages();
+
   /** @brief Run the simulation */
   void run();
 
diff --git a/include/simgrid/s4u/Storage.hpp b/include/simgrid/s4u/Storage.hpp
index 571c3b1d92..bc24f9329f 100644
--- a/include/simgrid/s4u/Storage.hpp
+++ b/include/simgrid/s4u/Storage.hpp
@@ -21,7 +21,7 @@ extern template class XBT_PUBLIC() Extendable<simgrid::s4u::Storage>;
 }
 namespace s4u {
 
-XBT_ATTRIB_PUBLIC void getStorageList(std::map<std::string, Storage*>* whereTo);
+void getStorageList(std::map<std::string, Storage*>* whereTo);
 
 XBT_PUBLIC_CLASS Storage : public simgrid::xbt::Extendable<Storage>
 {
@@ -29,7 +29,7 @@ XBT_PUBLIC_CLASS Storage : public simgrid::xbt::Extendable<Storage>
   friend simgrid::surf::StorageImpl;
 
 public:
-  explicit Storage(surf::StorageImpl * pimpl) : pimpl_(pimpl) {}
+  explicit Storage(std::string name, surf::StorageImpl * pimpl);
   virtual ~Storage() = default;
   /** Retrieve a Storage by its name. It must exist in the platform file */
   static Storage* byName(std::string name);
diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp
index df3b8939d0..c8c4895e04 100644
--- a/src/kernel/EngineImpl.cpp
+++ b/src/kernel/EngineImpl.cpp
@@ -7,6 +7,7 @@
 #include "simgrid/s4u/Host.hpp"
 #include "src/kernel/routing/NetPoint.hpp"
 #include "src/kernel/routing/NetZoneImpl.hpp"
+#include "src/surf/StorageImpl.hpp"
 
 #include <algorithm>
 
@@ -36,6 +37,10 @@ EngineImpl::~EngineImpl()
   delete netRoot_;
   for (auto const& kv : netpoints_)
     delete kv.second;
+
+  for (auto const& kv : storages_)
+    if (kv.second)
+      delete kv.second->getImpl();
 }
 }
 }
diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp
index 4e2ef24bd5..cba6843ece 100644
--- a/src/kernel/EngineImpl.hpp
+++ b/src/kernel/EngineImpl.hpp
@@ -20,6 +20,7 @@ public:
 
 private:
   std::map<std::string, simgrid::s4u::Host*> hosts_;
+  std::map<std::string, simgrid::s4u::Storage*> storages_;
   std::unordered_map<std::string, simgrid::kernel::routing::NetPoint*> netpoints_;
   friend simgrid::s4u::Engine;
 };
diff --git a/src/s4u/s4u_engine.cpp b/src/s4u/s4u_engine.cpp
index e49b59db09..542eb68eb9 100644
--- a/src/s4u/s4u_engine.cpp
+++ b/src/s4u/s4u_engine.cpp
@@ -94,6 +94,7 @@ Engine::getHostList(std::vector<Host*>* list)
   for (auto const& kv : pimpl->hosts_)
     list->push_back(kv.second);
 }
+
 /** @brief Returns the list of all hosts found in the platform */
 std::vector<Host*> Engine::getAllHosts()
 {
@@ -102,29 +103,64 @@ std::vector<Host*> Engine::getAllHosts()
     res.push_back(kv.second);
   return res;
 }
+
 void Engine::addHost(std::string name, simgrid::s4u::Host* host)
 {
   pimpl->hosts_[name] = host;
 }
+
 void Engine::delHost(std::string name)
 {
   pimpl->hosts_.erase(name);
 }
+
 simgrid::s4u::Host* Engine::hostByName(std::string name)
 {
   return pimpl->hosts_.at(name); // Will raise a std::out_of_range if the host does not exist
 }
+
 simgrid::s4u::Host* Engine::hostByNameOrNull(std::string name)
 {
   auto host = pimpl->hosts_.find(name);
   return host == pimpl->hosts_.end() ? nullptr : host->second;
 }
 
+/** @brief Returns the list of all storages found in the platform */
+std::vector<Storage*> Engine::getAllStorages()
+{
+  std::vector<Storage*> res;
+  for (auto const& kv : pimpl->storages_)
+    res.push_back(kv.second);
+  return res;
+}
+
+simgrid::s4u::Storage* Engine::storageByName(std::string name)
+{
+  return pimpl->storages_.at(name); // Will raise a std::out_of_range if the host does not exist
+}
+
+simgrid::s4u::Storage* Engine::storageByNameOrNull(std::string name)
+{
+  auto storage = pimpl->storages_.find(name);
+  return storage == pimpl->storages_.end() ? nullptr : storage->second;
+}
+
+void Engine::addStorage(std::string name, simgrid::s4u::Storage* storage)
+{
+  pimpl->storages_[name] = storage;
+}
+
+void Engine::delStorage(std::string name)
+{
+  pimpl->storages_.erase(name);
+}
+
 /** @brief Returns the amount of links in the platform */
 size_t Engine::getLinkCount()
 {
   return simgrid::surf::LinkImpl::linksCount();
 }
+
 /** @brief Fills the passed list with all links found in the platform
  *
  *  @deprecated. Prefer Engine::getAllLinks() */
@@ -133,6 +169,7 @@ Engine::getLinkList(std::vector<Link*>* list)
 {
   simgrid::surf::LinkImpl::linksList(list);
 }
+
 /** @brief Returns the list of all links found in the platform */
 std::vector<Link*> Engine::getAllLinks()
 {
diff --git a/src/s4u/s4u_storage.cpp b/src/s4u/s4u_storage.cpp
index a351ae469e..5ce0f07de2 100644
--- a/src/s4u/s4u_storage.cpp
+++ b/src/s4u/s4u_storage.cpp
@@ -4,6 +4,7 @@
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
 #include "simgrid/msg.h"
+#include "simgrid/s4u/Engine.hpp"
 #include "simgrid/s4u/Host.hpp"
 #include "simgrid/s4u/Storage.hpp"
 #include "simgrid/simix.hpp"
@@ -19,28 +20,32 @@ template class Extendable<simgrid::s4u::Storage>;
 
 namespace s4u {
 
-void getStorageList(std::map<std::string, Storage*>* whereTo)
+void XBT_ATTRIB_DEPRECATED_v322(
+    "simgrid::s4u::getStorageList() is deprecated in favor of Engine::getAllStorages(). Please switch before v3.22")
+    getStorageList(std::map<std::string, Storage*>* whereTo)
 {
-  for (auto const& s : *surf::StorageImpl::storagesMap())
-    whereTo->insert({s.first, &(s.second->piface_)}); // Convert each entry into its interface
+  for (auto const& s : simgrid::s4u::Engine::getInstance()->getAllStorages())
+    whereTo->insert({s->getName(), s});
+}
+
+Storage::Storage(std::string name, surf::StorageImpl* pimpl) : pimpl_(pimpl), name_(name)
+{
+  simgrid::s4u::Engine::getInstance()->addStorage(name, this);
 }
 
 Storage* Storage::byName(std::string name)
 {
-  surf::StorageImpl* res = surf::StorageImpl::byName(name);
-  if (res == nullptr)
-    return nullptr;
-  return &res->piface_;
+  return Engine::getInstance()->storageByNameOrNull(name);
 }
 
 const std::string& Storage::getName() const
 {
-  return pimpl_->getName();
+  return name_;
 }
 
 const char* Storage::getCname() const
 {
-  return pimpl_->getCname();
+  return name_.c_str();
 }
 
 const char* Storage::getType()
@@ -168,12 +173,10 @@ sg_storage_t sg_storage_get_by_name(const char* name)
  */
 xbt_dynar_t sg_storages_as_dynar()
 {
-  std::map<std::string, simgrid::s4u::Storage*>* storage_list = new std::map<std::string, simgrid::s4u::Storage*>;
-  simgrid::s4u::getStorageList(storage_list);
+  std::vector<simgrid::s4u::Storage*> storage_list = simgrid::s4u::Engine::getInstance()->getAllStorages();
   xbt_dynar_t res = xbt_dynar_new(sizeof(sg_storage_t), nullptr);
-  for (auto const& s : *storage_list)
-    xbt_dynar_push(res, &(s.second));
-  delete storage_list;
+  for (auto const& s : storage_list)
+    xbt_dynar_push(res, &s);
   return res;
 }
 
diff --git a/src/surf/StorageImpl.cpp b/src/surf/StorageImpl.cpp
index fcfa53445e..a11e477ddb 100644
--- a/src/surf/StorageImpl.cpp
+++ b/src/surf/StorageImpl.cpp
@@ -4,6 +4,8 @@
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
 #include "StorageImpl.hpp"
+#include "simgrid/s4u/Engine.hpp"
+#include "src/kernel/EngineImpl.hpp"
 #include "src/kernel/lmm/maxmin.hpp"
 #include "surf_private.hpp"
 
@@ -24,17 +26,6 @@ simgrid::xbt::signal<void(StorageImpl*, int, int)> storageStateChangedCallbacks;
 simgrid::xbt::signal<void(StorageAction*, kernel::resource::Action::State, kernel::resource::Action::State)>
     storageActionStateChangedCallbacks;
 
-/* List of storages */
-std::unordered_map<std::string, StorageImpl*>* StorageImpl::storages =
-    new std::unordered_map<std::string, StorageImpl*>();
-
-StorageImpl* StorageImpl::byName(std::string name)
-{
-  if (storages->find(name) == storages->end())
-    return nullptr;
-  return storages->at(name);
-}
-
 /*********
  * Model *
  *********/
@@ -57,7 +48,7 @@ StorageImpl::StorageImpl(kernel::resource::Model* model, std::string name, lmm_s
                          double bwrite, std::string type_id, std::string content_name, sg_size_t size,
                          std::string attach)
     : Resource(model, name.c_str(), maxminSystem->constraint_new(this, std::max(bread, bwrite)))
-    , piface_(this)
+    , piface_(name, this)
     , typeId_(type_id)
     , content_name(content_name)
     , size_(size)
@@ -67,7 +58,6 @@ StorageImpl::StorageImpl(kernel::resource::Model* model, std::string name, lmm_s
   XBT_DEBUG("Create resource with Bread '%f' Bwrite '%f' and Size '%llu'", bread, bwrite, size);
   constraintRead_  = maxminSystem->constraint_new(this, bread);
   constraintWrite_ = maxminSystem->constraint_new(this, bwrite);
-  storages->insert({name, this});
 }
 
 StorageImpl::~StorageImpl()
@@ -75,7 +65,6 @@ StorageImpl::~StorageImpl()
   storageDestructedCallbacks(this);
 }
 
-
 bool StorageImpl::isUsed()
 {
   THROW_UNIMPLEMENTED;
diff --git a/src/surf/StorageImpl.hpp b/src/surf/StorageImpl.hpp
index ff0704a3c8..55e8db6766 100644
--- a/src/surf/StorageImpl.hpp
+++ b/src/surf/StorageImpl.hpp
@@ -73,8 +73,6 @@ public:
 
   virtual StorageImpl* createStorage(std::string id, std::string type_id, std::string content_name,
                                      std::string attach) = 0;
-
-  std::vector<StorageImpl*> p_storageList;
 };
 
 /************
@@ -94,7 +92,6 @@ public:
 
   /** @brief Public interface */
   s4u::Storage piface_;
-  static StorageImpl* byName(std::string name);
 
   /** @brief Check if the Storage is used (if an action currently uses its resources) */
   bool isUsed() override;
@@ -121,8 +118,6 @@ public:
   virtual StorageAction* write(sg_size_t size) = 0;
   virtual std::string getHost() { return attach_; }
 
-  static std::unordered_map<std::string, StorageImpl*>* storagesMap() { return StorageImpl::storages; }
-
   kernel::lmm::Constraint* constraintWrite_; /* Constraint for maximum write bandwidth*/
   kernel::lmm::Constraint* constraintRead_;  /* Constraint for maximum write bandwidth*/
 
@@ -131,7 +126,6 @@ public:
   sg_size_t size_;          // Only used at parsing time then goes to the FileSystemExtension
 
 private:
-  static std::unordered_map<std::string, StorageImpl*>* storages;
   // Name of the host to which this storage is attached. Only used at platform parsing time, then the interface stores
   // the Host directly.
   std::string attach_;
diff --git a/src/surf/sg_platf.cpp b/src/surf/sg_platf.cpp
index c54247675f..da184ba94f 100644
--- a/src/surf/sg_platf.cpp
+++ b/src/surf/sg_platf.cpp
@@ -394,7 +394,7 @@ void sg_platf_new_mount(MountCreationArgs* mount)
 
   if (mount_list.empty())
     XBT_DEBUG("Create a Mount list for %s", A_surfxml_host_id);
-  mount_list.insert({mount->name, simgrid::surf::StorageImpl::byName(mount->storageId.c_str())});
+  mount_list.insert({mount->name, simgrid::s4u::Engine::getInstance()->storageByName(mount->storageId)->getImpl()});
 }
 
 void sg_platf_new_route(RouteCreationArgs* route)
diff --git a/src/surf/storage_n11.cpp b/src/surf/storage_n11.cpp
index 62e7d21192..10fac18ae5 100644
--- a/src/surf/storage_n11.cpp
+++ b/src/surf/storage_n11.cpp
@@ -5,6 +5,7 @@
 
 #include "storage_n11.hpp"
 #include "simgrid/s4u/Engine.hpp"
+#include "simgrid/s4u/Host.hpp"
 #include "src/kernel/lmm/maxmin.hpp"
 #include "src/kernel/routing/NetPoint.hpp"
 #include "xbt/utility.hpp"
@@ -19,13 +20,13 @@ extern std::map<std::string, simgrid::surf::StorageType*> storage_types;
 
 static void check_disk_attachment()
 {
-  for (auto const& s : *simgrid::surf::StorageImpl::storagesMap()) {
-    simgrid::kernel::routing::NetPoint* host_elm = sg_netpoint_by_name_or_null(s.second->getHost().c_str());
+  for (auto const& s : simgrid::s4u::Engine::getInstance()->getAllStorages()) {
+    simgrid::kernel::routing::NetPoint* host_elm = sg_netpoint_by_name_or_null(s->getImpl()->getHost().c_str());
     if (not host_elm)
-      surf_parse_error(std::string("Unable to attach storage ") + s.second->getCname() + ": host " +
-                       s.second->getHost() + " does not exist.");
+      surf_parse_error(std::string("Unable to attach storage ") + s->getCname() + ": host " +
+                       s->getImpl()->getHost().c_str() + " does not exist.");
     else
-      s.second->piface_.attached_to_ = sg_host_by_name(s.second->getHost().c_str());
+      s->attached_to_ = sg_host_by_name(s->getImpl()->getHost().c_str());
   }
 }
 
@@ -65,8 +66,6 @@ StorageImpl* StorageN11Model::createStorage(std::string id, std::string type_id,
   XBT_DEBUG("SURF storage create resource\n\t\tid '%s'\n\t\ttype '%s'\n\t\tBread '%f'\n", id.c_str(), type_id.c_str(),
             Bread);
 
-  p_storageList.push_back(storage);
-
   return storage;
 }
 
diff --git a/src/surf/surf_interface.cpp b/src/surf/surf_interface.cpp
index f45907ef1a..54f61e8a80 100644
--- a/src/surf/surf_interface.cpp
+++ b/src/surf/surf_interface.cpp
@@ -329,9 +329,6 @@ void surf_exit()
     delete stype->model_properties;
     delete stype;
   }
-  for (auto const& s : *simgrid::surf::StorageImpl::storagesMap())
-    delete s.second;
-  delete simgrid::surf::StorageImpl::storagesMap();
 
   for (auto const& model : *all_existing_models)
     delete model;