From: Frederic Suter Date: Thu, 8 Jun 2017 05:34:16 +0000 (+0200) Subject: use pimpl/piface combo for s4u storage X-Git-Tag: v3.16~141 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/274ff3a104711075a837ddc6e677e713b3348354?hp=-c use pimpl/piface combo for s4u storage not perfect but one more towards killing storage_lib (and xbt_lib) --- 274ff3a104711075a837ddc6e677e713b3348354 diff --git a/examples/s4u/io/s4u_io.cpp b/examples/s4u/io/s4u_io.cpp index a62a5ebc54..90126b62a4 100644 --- a/examples/s4u/io/s4u_io.cpp +++ b/examples/s4u/io/s4u_io.cpp @@ -17,7 +17,7 @@ public: for (const auto&kv : mounts) { const char* mountpoint = kv.first.c_str(); - simgrid::s4u::Storage &storage = *kv.second; + simgrid::s4u::Storage storage = *kv.second; // Retrieve disk's information sg_size_t free_size = storage.sizeFree(); @@ -55,7 +55,7 @@ public: write = file->write(100000); // Write 100,000 bytes XBT_INFO("Write %llu bytes on %s", write, filename); - simgrid::s4u::Storage &storage = simgrid::s4u::Storage::byName("Disk4"); + simgrid::s4u::Storage* storage = simgrid::s4u::Storage::byName("Disk4"); // Now rename file from ./tmp/data.txt to ./tmp/simgrid.readme const char *newpath = "/home/tmp/simgrid.readme"; @@ -71,13 +71,13 @@ public: delete file; // Now attach some user data to disk1 - XBT_INFO("Get/set data for storage element: %s",storage.name()); - XBT_INFO(" Uninitialized storage data: '%s'", (char*)storage.userdata()); + XBT_INFO("Get/set data for storage element: %s", storage->name()); + XBT_INFO(" Uninitialized storage data: '%s'", (char*)storage->userdata()); - storage.setUserdata(xbt_strdup("Some user data")); - XBT_INFO(" Set and get data: '%s'", (char*)storage.userdata()); + storage->setUserdata(xbt_strdup("Some user data")); + XBT_INFO(" Set and get data: '%s'", (char*)storage->userdata()); - xbt_free(storage.userdata()); + xbt_free(storage->userdata()); } }; diff --git a/include/simgrid/s4u/Storage.hpp b/include/simgrid/s4u/Storage.hpp index 305bbba87d..85c16ae3ce 100644 --- a/include/simgrid/s4u/Storage.hpp +++ b/include/simgrid/s4u/Storage.hpp @@ -17,17 +17,18 @@ namespace simgrid { namespace s4u { +std::unordered_map* allStorages(); + XBT_PUBLIC_CLASS Storage { friend s4u::Engine; - - Storage(std::string name, smx_storage_t inferior); + friend simgrid::surf::StorageImpl; public: - Storage() = default; - virtual ~Storage(); + explicit Storage(surf::StorageImpl * pimpl) : pimpl_(pimpl) {} + virtual ~Storage() = default; /** Retrieve a Storage by its name. It must exist in the platform file */ - static Storage& byName(const char* name); + static Storage* byName(const char* name); const char* name(); const char* host(); sg_size_t sizeFree(); @@ -38,22 +39,21 @@ public: const char* property(const char* key); void setProperty(const char* key, char* value); std::map* content(); - std::unordered_map* allStorages(); - -protected: - smx_storage_t inferior(); public: void setUserdata(void* data) { userdata_ = data; } void* userdata() { return userdata_; } -private: - static std::unordered_map* storages_; + /* The signals */ + /** @brief Callback signal fired when a new Link is created */ + static simgrid::xbt::signal onCreation; + + /** @brief Callback signal fired when a Link is destroyed */ + static simgrid::xbt::signal onDestruction; - std::string hostname_; +private: std::string name_; - sg_size_t size_ = 0; - smx_storage_t pimpl_ = nullptr; + surf::StorageImpl* const pimpl_ = nullptr; void* userdata_ = nullptr; }; diff --git a/src/s4u/s4u_engine.cpp b/src/s4u/s4u_engine.cpp index d895beb099..9ab100720c 100644 --- a/src/s4u/s4u_engine.cpp +++ b/src/s4u/s4u_engine.cpp @@ -53,7 +53,6 @@ Engine *Engine::instance() { void Engine::shutdown() { delete s4u::Engine::instance_; s4u::Engine::instance_ = nullptr; - delete s4u::Storage::storages_; } double Engine::getClock() diff --git a/src/s4u/s4u_host.cpp b/src/s4u/s4u_host.cpp index c36d9ec715..a1cf532612 100644 --- a/src/s4u/s4u_host.cpp +++ b/src/s4u/s4u_host.cpp @@ -182,7 +182,7 @@ boost::unordered_map const& Host::mountedStorages() { char *mountname; char *storagename; xbt_dict_foreach(dict, cursor, mountname, storagename) { - mounts->insert({mountname, &Storage::byName(storagename)}); + mounts->insert({mountname, Storage::byName(storagename)}); } xbt_dict_free(&dict); } diff --git a/src/s4u/s4u_storage.cpp b/src/s4u/s4u_storage.cpp index d010801b6c..e3e776c928 100644 --- a/src/s4u/s4u_storage.cpp +++ b/src/s4u/s4u_storage.cpp @@ -9,70 +9,53 @@ #include "xbt/lib.h" #include -extern xbt_lib_t storage_lib; - namespace simgrid { namespace s4u { - -std::unordered_map* Storage::storages_ = new std::unordered_map(); - -Storage::Storage(std::string name, smx_storage_t inferior) : - name_(name), pimpl_(inferior) +std::unordered_map* allStorages() { - hostname_ = surf_storage_get_host(pimpl_); - size_ = surf_storage_get_size(pimpl_); - storages_->insert({name, this}); -} + std::unordered_map* map = surf::StorageImpl::storagesMap(); + std::unordered_map* res = new std::unordered_map; + for (auto s : *map) + res->insert({s.first, &(s.second->piface_)}); // Convert each entry into its interface -Storage::~Storage() = default; - -smx_storage_t Storage::inferior() -{ - return pimpl_; + return res; } -Storage& Storage::byName(const char* name) +Storage* Storage::byName(const char* name) { - s4u::Storage* res = nullptr; - try { - res = storages_->at(name); - } catch (std::out_of_range& e) { - smx_storage_t inferior = xbt_lib_get_elm_or_null(storage_lib,name); - if (inferior == nullptr) - xbt_die("Storage %s does not exist. Please only use the storages that are defined in your platform.", name); - - res = new Storage(name,inferior); - } - return *res; + surf::StorageImpl* res = surf::StorageImpl::byName(name); + if (res == nullptr) + return nullptr; + return &res->piface_; } const char* Storage::name() { - return name_.c_str(); + return pimpl_->cname(); } const char* Storage::host() { - return hostname_.c_str(); + return pimpl_->attach_; } sg_size_t Storage::sizeFree() { - return simgrid::simix::kernelImmediate([this] { return surf_storage_resource_priv(pimpl_)->getFreeSize(); }); + return simgrid::simix::kernelImmediate([this] { return pimpl_->getFreeSize(); }); } sg_size_t Storage::sizeUsed() { - return simgrid::simix::kernelImmediate([this] { return surf_storage_resource_priv(pimpl_)->getUsedSize(); }); + return simgrid::simix::kernelImmediate([this] { return pimpl_->getUsedSize(); }); } sg_size_t Storage::size() { - return size_; + return pimpl_->size_; } xbt_dict_t Storage::properties() { - return simcall_storage_get_properties(pimpl_); + return simgrid::simix::kernelImmediate([this] { return pimpl_->getProperties(); }); } const char* Storage::property(const char* key) @@ -87,13 +70,14 @@ void Storage::setProperty(const char* key, char* value) std::map* Storage::content() { - return simgrid::simix::kernelImmediate([this] { return surf_storage_resource_priv(this->pimpl_)->getContent(); }); + return simgrid::simix::kernelImmediate([this] { return pimpl_->getContent(); }); } -std::unordered_map* Storage::allStorages() -{ - return storages_; -} +/************* + * Callbacks * + *************/ +simgrid::xbt::signal Storage::onCreation; +simgrid::xbt::signal Storage::onDestruction; } /* namespace s4u */ } /* namespace simgrid */ diff --git a/src/surf/StorageImpl.cpp b/src/surf/StorageImpl.cpp index 3eb9910a3d..d15ab37d50 100644 --- a/src/surf/StorageImpl.cpp +++ b/src/surf/StorageImpl.cpp @@ -31,6 +31,17 @@ simgrid::xbt::signal storageDestructedCallbacks; simgrid::xbt::signal storageStateChangedCallbacks; // signature: wasOn, isOn simgrid::xbt::signal storageActionStateChangedCallbacks; +/* List of storages */ +std::unordered_map* StorageImpl::storages = + new std::unordered_map(); + +StorageImpl* StorageImpl::byName(const char* name) +{ + if (storages->find(name) == storages->end()) + return nullptr; + return storages->at(name); +} + /********* * Model * *********/ @@ -53,6 +64,7 @@ StorageModel::~StorageModel() StorageImpl::StorageImpl(Model* model, const char* name, lmm_system_t maxminSystem, double bread, double bwrite, const char* type_id, const char* content_name, sg_size_t size, const char* attach) : Resource(model, name, lmm_constraint_new(maxminSystem, this, MAX(bread, bwrite))) + , piface_(this) , size_(size) , usedSize_(0) , typeId_(xbt_strdup(type_id)) @@ -64,6 +76,7 @@ StorageImpl::StorageImpl(Model* model, const char* name, lmm_system_t maxminSyst XBT_DEBUG("Create resource with Bread '%f' Bwrite '%f' and Size '%llu'", bread, bwrite, size); constraintRead_ = lmm_constraint_new(maxminSystem, this, bread); constraintWrite_ = lmm_constraint_new(maxminSystem, this, bwrite); + storages->insert({name, this}); } StorageImpl::~StorageImpl() diff --git a/src/surf/StorageImpl.hpp b/src/surf/StorageImpl.hpp index b02503e94e..9b1af7f869 100644 --- a/src/surf/StorageImpl.hpp +++ b/src/surf/StorageImpl.hpp @@ -7,6 +7,7 @@ #include #include +#include "simgrid/s4u/Storage.hpp" #include "src/surf/PropertyHolder.hpp" #include "surf_interface.hpp" #include @@ -86,7 +87,12 @@ public: StorageImpl(Model* model, const char* name, lmm_system_t maxminSystem, double bread, double bwrite, const char* type_id, const char* content_name, sg_size_t size, const char* attach); - ~StorageImpl(); + ~StorageImpl() override; + +public: + /** @brief Public interface */ + s4u::Storage piface_; + static StorageImpl* byName(const char* name); /** @brief Check if the Storage is used (if an action currently uses its resources) */ bool isUsed() override; @@ -160,7 +166,8 @@ public: virtual sg_size_t getUsedSize(); std::map* parseContent(const char* filename); - + static std::unordered_map* storages; + static std::unordered_map* storagesMap() { return StorageImpl::storages; } std::vector writeActions_; lmm_constraint_t constraintWrite_; /* Constraint for maximum write bandwidth*/ diff --git a/src/surf/storage_n11.cpp b/src/surf/storage_n11.cpp index 5cffd796e2..8228aa1b20 100644 --- a/src/surf/storage_n11.cpp +++ b/src/surf/storage_n11.cpp @@ -158,6 +158,7 @@ StorageN11::StorageN11(StorageModel* model, const char* name, lmm_system_t maxmi : StorageImpl(model, name, maxminSystem, bread, bwrite, type_id, content_name, size, attach) { XBT_DEBUG("Create resource with Bread '%f' Bwrite '%f' and Size '%llu'", bread, bwrite, size); + simgrid::s4u::Storage::onCreation(this->piface_); } StorageAction *StorageN11::open(const char* mount, const char* path) diff --git a/src/surf/storage_n11.hpp b/src/surf/storage_n11.hpp index 00bd08d9ab..72703a1093 100644 --- a/src/surf/storage_n11.hpp +++ b/src/surf/storage_n11.hpp @@ -42,6 +42,7 @@ class StorageN11 : public StorageImpl { public: StorageN11(StorageModel* model, const char* name, lmm_system_t maxminSystem, double bread, double bwrite, const char* type_id, char* content_name, sg_size_t size, char* attach); + virtual ~StorageN11() = default; StorageAction *open(const char* mount, const char* path); StorageAction *close(surf_file_t fd); StorageAction *ls(const char *path); diff --git a/teshsuite/s4u/storage_client_server/storage_client_server.cpp b/teshsuite/s4u/storage_client_server/storage_client_server.cpp index 735879fe9f..b801f7c8d2 100644 --- a/teshsuite/s4u/storage_client_server/storage_client_server.cpp +++ b/teshsuite/s4u/storage_client_server/storage_client_server.cpp @@ -76,34 +76,32 @@ static void display_storage_content(simgrid::s4u::Storage* storage) static void dump_storage_by_name(char* name) { XBT_INFO("*** Dump a storage element ***"); - simgrid::s4u::Storage& storage = simgrid::s4u::Storage::byName(name); - display_storage_content(&storage); + simgrid::s4u::Storage* storage = simgrid::s4u::Storage::byName(name); + display_storage_content(storage); } static void get_set_storage_data(const char* storage_name) { XBT_INFO("*** GET/SET DATA for storage element: %s ***", storage_name); - simgrid::s4u::Storage& storage = simgrid::s4u::Storage::byName(storage_name); + simgrid::s4u::Storage* storage = simgrid::s4u::Storage::byName(storage_name); - char* data = static_cast(storage.userdata()); + char* data = static_cast(storage->userdata()); XBT_INFO("Get data: '%s'", data); - storage.setUserdata(xbt_strdup("Some data")); - data = static_cast(storage.userdata()); + storage->setUserdata(xbt_strdup("Some data")); + data = static_cast(storage->userdata()); XBT_INFO("\tSet and get data: '%s'", data); xbt_free(data); } static void dump_platform_storages() { - std::unordered_map* storages = simgrid::s4u::Storage().allStorages(); + std::unordered_map* storages = simgrid::s4u::allStorages(); for (auto storage : *storages) { XBT_INFO("Storage %s is attached to %s", storage.first.c_str(), storage.second->host()); storage.second->setProperty("other usage", xbt_strdup("gpfs")); } - // Expected output in tesh file that's missing for now - //> [ 1.207952] (server@alice) Storage Disk3 is attached to carl - //> [ 1.207952] (server@alice) Storage Disk4 is attached to denise + delete storages; } static void storage_info(simgrid::s4u::Host* host) @@ -116,15 +114,15 @@ static void storage_info(simgrid::s4u::Host* host) xbt_dict_t storage_list = host->mountedStoragesAsDict(); xbt_dict_foreach (storage_list, cursor, mount_name, storage_name) { XBT_INFO("\tStorage name: %s, mount name: %s", storage_name, mount_name); - simgrid::s4u::Storage& storage = simgrid::s4u::Storage::byName(storage_name); + simgrid::s4u::Storage* storage = simgrid::s4u::Storage::byName(storage_name); - sg_size_t free_size = storage.sizeFree(); - sg_size_t used_size = storage.sizeUsed(); + sg_size_t free_size = storage->sizeFree(); + sg_size_t used_size = storage->sizeUsed(); XBT_INFO("\t\tFree size: %llu bytes", free_size); XBT_INFO("\t\tUsed size: %llu bytes", used_size); - display_storage_properties(&storage); + display_storage_properties(storage); dump_storage_by_name(storage_name); } xbt_dict_free(&storage_list); diff --git a/teshsuite/s4u/storage_client_server/storage_client_server.tesh b/teshsuite/s4u/storage_client_server/storage_client_server.tesh index ee942ed592..c183aa2a09 100644 --- a/teshsuite/s4u/storage_client_server/storage_client_server.tesh +++ b/teshsuite/s4u/storage_client_server/storage_client_server.tesh @@ -102,5 +102,7 @@ $ ./storage_client_server$EXEEXT ${srcdir:=.}/../../../examples/platforms/storag > [ 1.207952] (server@alice) \Windows\winhlp32.exe size: 10752 bytes > [ 1.207952] (server@alice) \Windows\write.exe size: 10752 bytes > [ 1.207952] (server@alice) Storage Disk1 is attached to bob +> [ 1.207952] (server@alice) Storage Disk3 is attached to carl > [ 1.207952] (server@alice) Storage Disk2 is attached to alice +> [ 1.207952] (server@alice) Storage Disk4 is attached to denise > [ 1.207952] (maestro@) Simulated time: 1.20795