From eadce057a2c8e09df65641e77935bd9ffc905440 Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Wed, 17 Apr 2019 23:21:48 +0200 Subject: [PATCH] Constify get_properties() and and remove simcalls. --- .../platform-properties/s4u-platform-properties.cpp | 4 ++-- include/simgrid/kernel/routing/NetZoneImpl.hpp | 2 +- include/simgrid/s4u/Actor.hpp | 2 +- include/simgrid/s4u/Host.hpp | 2 +- include/simgrid/s4u/Link.hpp | 2 +- include/simgrid/s4u/NetZone.hpp | 4 ++-- include/simgrid/s4u/Storage.hpp | 4 ++-- src/kernel/actor/ActorImpl.cpp | 4 ++-- src/kernel/actor/ActorImpl.hpp | 6 +++--- src/kernel/routing/NetZoneImpl.cpp | 2 +- src/s4u/s4u_Actor.cpp | 4 ++-- src/s4u/s4u_Host.cpp | 8 ++++---- src/s4u/s4u_Link.cpp | 2 +- src/s4u/s4u_Netzone.cpp | 10 ++++++---- src/s4u/s4u_Storage.cpp | 8 ++++---- src/surf/PropertyHolder.cpp | 2 +- src/surf/PropertyHolder.hpp | 5 +---- .../storage_client_server/storage_client_server.cpp | 2 +- .../storage_client_server/storage_client_server.tesh | 6 +++--- teshsuite/simdag/flatifier/flatifier.cpp | 3 +-- 20 files changed, 40 insertions(+), 42 deletions(-) diff --git a/examples/s4u/platform-properties/s4u-platform-properties.cpp b/examples/s4u/platform-properties/s4u-platform-properties.cpp index fe495966e2..e85410a35a 100644 --- a/examples/s4u/platform-properties/s4u-platform-properties.cpp +++ b/examples/s4u/platform-properties/s4u-platform-properties.cpp @@ -14,7 +14,7 @@ XBT_LOG_NEW_DEFAULT_CATEGORY(s4u_test, "Property test"); static void test_host(const std::string& hostname) { simgrid::s4u::Host* thehost = simgrid::s4u::Host::by_name(hostname); - std::unordered_map* props = thehost->get_properties(); + const std::unordered_map* props = thehost->get_properties(); const char* noexist = "Unknown"; const char* exist = "Hdd"; const char* value; @@ -81,7 +81,7 @@ static void bob(std::vector /*args*/) XBT_INFO(" Zone property: author -> %s", root->get_property("author")); /* Get the property list of current bob process */ - std::unordered_map* props = simgrid::s4u::Actor::self()->get_properties(); + const std::unordered_map* props = simgrid::s4u::Actor::self()->get_properties(); const char* noexist = "UnknownProcessProp"; XBT_ATTRIB_UNUSED const char* value; diff --git a/include/simgrid/kernel/routing/NetZoneImpl.hpp b/include/simgrid/kernel/routing/NetZoneImpl.hpp index eaad97d52b..ef9b53c363 100644 --- a/include/simgrid/kernel/routing/NetZoneImpl.hpp +++ b/include/simgrid/kernel/routing/NetZoneImpl.hpp @@ -61,7 +61,7 @@ public: /** @brief Make a host within that NetZone */ simgrid::s4u::Host* create_host(const char* name, const std::vector& speed_per_pstate, int core_count, - std::map* props); + const std::map* props); /** @brief Creates a new route in this NetZone */ virtual void add_bypass_route(NetPoint* src, NetPoint* dst, NetPoint* gw_src, NetPoint* gw_dst, std::vector& link_list, bool symmetrical); diff --git a/include/simgrid/s4u/Actor.hpp b/include/simgrid/s4u/Actor.hpp index 1dc179f78a..8e48a17218 100644 --- a/include/simgrid/s4u/Actor.hpp +++ b/include/simgrid/s4u/Actor.hpp @@ -289,7 +289,7 @@ public: kernel::actor::ActorImpl* get_impl() const { return pimpl_; } /** Retrieve the property value (or nullptr if not set) */ - std::unordered_map* + const std::unordered_map* get_properties() const; // FIXME: do not export the map, but only the keys or something const char* get_property(const std::string& key) const; void set_property(const std::string& key, const std::string& value); diff --git a/include/simgrid/s4u/Host.hpp b/include/simgrid/s4u/Host.hpp index ea0afe0655..f4d75503e3 100644 --- a/include/simgrid/s4u/Host.hpp +++ b/include/simgrid/s4u/Host.hpp @@ -99,7 +99,7 @@ public: const char* get_property(const std::string& key) const; void set_property(const std::string& key, const std::string& value); - std::unordered_map* get_properties(); + const std::unordered_map* get_properties() const; void set_state_profile(kernel::profile::Profile* p); void set_speed_profile(kernel::profile::Profile* p); diff --git a/include/simgrid/s4u/Link.hpp b/include/simgrid/s4u/Link.hpp index 74fb9e9648..85b5e1ed06 100644 --- a/include/simgrid/s4u/Link.hpp +++ b/include/simgrid/s4u/Link.hpp @@ -93,7 +93,7 @@ public: * The profile must contain absolute values */ void set_latency_profile(kernel::profile::Profile* profile); - const char* get_property(const std::string& key); + const char* get_property(const std::string& key) const; void set_property(const std::string& key, const std::string& value); /* The signals */ diff --git a/include/simgrid/s4u/NetZone.hpp b/include/simgrid/s4u/NetZone.hpp index 8004e96991..e41fed0fe3 100644 --- a/include/simgrid/s4u/NetZone.hpp +++ b/include/simgrid/s4u/NetZone.hpp @@ -49,12 +49,12 @@ private: public: /** Get the properties assigned to a netzone */ - std::unordered_map* get_properties(); + const std::unordered_map* get_properties() const; std::vector get_children(); /** Retrieve the property value (or nullptr if not set) */ - const char* get_property(const std::string& key); + const char* get_property(const std::string& key) const; void set_property(const std::string& key, const std::string& value); /* Add content to the netzone, at parsing time. It should be sealed afterward. */ diff --git a/include/simgrid/s4u/Storage.hpp b/include/simgrid/s4u/Storage.hpp index 09c09e2cf2..9bfa5add00 100644 --- a/include/simgrid/s4u/Storage.hpp +++ b/include/simgrid/s4u/Storage.hpp @@ -56,8 +56,8 @@ public: Host* get_host() { return attached_to_; }; void set_host(Host* host) { attached_to_ = host; } - std::unordered_map* get_properties(); - const char* get_property(const std::string& key); + const std::unordered_map* get_properties() const; + const char* get_property(const std::string& key) const; void set_property(const std::string&, const std::string& value); void set_data(void* data) { userdata_ = data; } diff --git a/src/kernel/actor/ActorImpl.cpp b/src/kernel/actor/ActorImpl.cpp index cce43fc715..9a646da55c 100644 --- a/src/kernel/actor/ActorImpl.cpp +++ b/src/kernel/actor/ActorImpl.cpp @@ -70,7 +70,7 @@ ActorImpl::~ActorImpl() = default; */ ActorImplPtr ActorImpl::attach(const std::string& name, void* data, s4u::Host* host, - std::unordered_map* properties) + const std::unordered_map* properties) { // This is mostly a copy/paste from create(), it'd be nice to share some code between those two functions. @@ -472,7 +472,7 @@ ActorImpl* ActorImpl::start(const simix::ActorCode& code) } ActorImplPtr ActorImpl::create(const std::string& name, const simix::ActorCode& code, void* data, s4u::Host* host, - std::unordered_map* properties, ActorImpl* parent_actor) + const std::unordered_map* properties, ActorImpl* parent_actor) { XBT_DEBUG("Start actor %s@'%s'", name.c_str(), host->get_cname()); diff --git a/src/kernel/actor/ActorImpl.hpp b/src/kernel/actor/ActorImpl.hpp index c6324ecc53..a1d027f968 100644 --- a/src/kernel/actor/ActorImpl.hpp +++ b/src/kernel/actor/ActorImpl.hpp @@ -105,9 +105,9 @@ public: ActorImpl* start(const simix::ActorCode& code); static ActorImplPtr create(const std::string& name, const simix::ActorCode& code, void* data, s4u::Host* host, - std::unordered_map* properties, ActorImpl* parent_actor); + const std::unordered_map* properties, ActorImpl* parent_actor); static ActorImplPtr attach(const std::string& name, void* data, s4u::Host* host, - std::unordered_map* properties); + const std::unordered_map* properties); static void detach(); void cleanup(); void exit(); @@ -133,7 +133,7 @@ public: void* data = nullptr; s4u::Host* host = nullptr; double kill_time = 0.0; - std::shared_ptr> properties = nullptr; + std::shared_ptr> properties = nullptr; bool auto_restart = false; bool daemon_ = false; ProcessArg() = default; diff --git a/src/kernel/routing/NetZoneImpl.cpp b/src/kernel/routing/NetZoneImpl.cpp index b87a8dd20e..babab5a07b 100644 --- a/src/kernel/routing/NetZoneImpl.cpp +++ b/src/kernel/routing/NetZoneImpl.cpp @@ -93,7 +93,7 @@ int NetZoneImpl::get_host_count() } simgrid::s4u::Host* NetZoneImpl::create_host(const char* name, const std::vector& speed_per_pstate, - int coreAmount, std::map* props) + int coreAmount, const std::map* props) { simgrid::s4u::Host* res = new simgrid::s4u::Host(name); diff --git a/src/s4u/s4u_Actor.cpp b/src/s4u/s4u_Actor.cpp index 953763c3e1..fe048a57b7 100644 --- a/src/s4u/s4u_Actor.cpp +++ b/src/s4u/s4u_Actor.cpp @@ -232,7 +232,7 @@ void Actor::kill_all() simix::simcall([self] { self->kill_all(); }); } -std::unordered_map* Actor::get_properties() const +const std::unordered_map* Actor::get_properties() const { return pimpl_->get_properties(); } @@ -525,7 +525,7 @@ xbt_dict_t sg_actor_get_properties(sg_actor_t actor) { xbt_assert(actor != nullptr, "Invalid parameter: First argument must not be nullptr"); xbt_dict_t as_dict = xbt_dict_new_homogeneous(xbt_free_f); - std::unordered_map* props = actor->get_properties(); + const std::unordered_map* props = actor->get_properties(); if (props == nullptr) return nullptr; for (auto const& kv : *props) { diff --git a/src/s4u/s4u_Host.cpp b/src/s4u/s4u_Host.cpp index 85ea7a3f0b..2af9129ccd 100644 --- a/src/s4u/s4u_Host.cpp +++ b/src/s4u/s4u_Host.cpp @@ -165,9 +165,9 @@ void Host::route_to(Host* dest, std::vector& links, } /** Get the properties assigned to a host */ -std::unordered_map* Host::get_properties() +const std::unordered_map* Host::get_properties() const { - return simix::simcall([this] { return this->pimpl_->get_properties(); }); + return this->pimpl_->get_properties(); } /** Retrieve the property value (or nullptr if not set) */ @@ -505,7 +505,7 @@ int sg_host_is_off(sg_host_t host) xbt_dict_t sg_host_get_properties(sg_host_t host) { xbt_dict_t as_dict = xbt_dict_new_homogeneous(xbt_free_f); - std::unordered_map* props = host->get_properties(); + const std::unordered_map* props = host->get_properties(); if (props == nullptr) return nullptr; for (auto const& elm : *props) { @@ -584,7 +584,7 @@ void sg_host_dump(sg_host_t host) XBT_INFO("Displaying host %s", host->get_cname()); XBT_INFO(" - speed: %.0f", host->get_speed()); XBT_INFO(" - available speed: %.2f", sg_host_get_available_speed(host)); - std::unordered_map* props = host->get_properties(); + const std::unordered_map* props = host->get_properties(); if (not props->empty()) { XBT_INFO(" - properties:"); diff --git a/src/s4u/s4u_Link.cpp b/src/s4u/s4u_Link.cpp index 456c57d006..866436c721 100644 --- a/src/s4u/s4u_Link.cpp +++ b/src/s4u/s4u_Link.cpp @@ -105,7 +105,7 @@ void Link::set_latency_profile(kernel::profile::Profile* trace) simgrid::simix::simcall([this, trace]() { this->pimpl_->set_latency_profile(trace); }); } -const char* Link::get_property(const std::string& key) +const char* Link::get_property(const std::string& key) const { return this->pimpl_->get_property(key); } diff --git a/src/s4u/s4u_Netzone.cpp b/src/s4u/s4u_Netzone.cpp index a0393554d1..b17dd18962 100644 --- a/src/s4u/s4u_Netzone.cpp +++ b/src/s4u/s4u_Netzone.cpp @@ -27,16 +27,18 @@ NetZone::~NetZone() { } -std::unordered_map* NetZone::get_properties() +const std::unordered_map* NetZone::get_properties() const { - return simix::simcall([this] { return &properties_; }); + return &properties_; } /** Retrieve the property value (or nullptr if not set) */ -const char* NetZone::get_property(const std::string& key) +const char* NetZone::get_property(const std::string& key) const { - return properties_.at(key).c_str(); + auto prop = properties_.find(key); + return prop == properties_.end() ? nullptr : prop->second.c_str(); } + void NetZone::set_property(const std::string& key, const std::string& value) { simix::simcall([this, &key, &value] { properties_[key] = value; }); diff --git a/src/s4u/s4u_Storage.cpp b/src/s4u/s4u_Storage.cpp index 0be7985d0f..8de6680859 100644 --- a/src/s4u/s4u_Storage.cpp +++ b/src/s4u/s4u_Storage.cpp @@ -41,12 +41,12 @@ const char* Storage::get_type() return pimpl_->typeId_.c_str(); } -std::unordered_map* Storage::get_properties() +const std::unordered_map* Storage::get_properties() const { - return simix::simcall([this] { return pimpl_->get_properties(); }); + return pimpl_->get_properties(); } -const char* Storage::get_property(const std::string& key) +const char* Storage::get_property(const std::string& key) const { return this->pimpl_->get_property(key); } @@ -118,7 +118,7 @@ xbt_dict_t sg_storage_get_properties(sg_storage_t storage) { xbt_assert((storage != nullptr), "Invalid parameters (storage is nullptr)"); xbt_dict_t as_dict = xbt_dict_new_homogeneous(xbt_free_f); - std::unordered_map* props = storage->get_properties(); + const std::unordered_map* props = storage->get_properties(); if (props == nullptr) return nullptr; for (auto const& elm : *props) { diff --git a/src/surf/PropertyHolder.cpp b/src/surf/PropertyHolder.cpp index a5bd8024c8..8b002a4c86 100644 --- a/src/surf/PropertyHolder.cpp +++ b/src/surf/PropertyHolder.cpp @@ -26,7 +26,7 @@ void PropertyHolder::set_property(const std::string& key, const std::string& val } /** @brief Return the whole set of properties. Don't mess with it, dude! */ -std::unordered_map* PropertyHolder::get_properties() +const std::unordered_map* PropertyHolder::get_properties() { if (not properties_) properties_.reset(new std::unordered_map); diff --git a/src/surf/PropertyHolder.hpp b/src/surf/PropertyHolder.hpp index a88589bbaf..27ae7a627f 100644 --- a/src/surf/PropertyHolder.hpp +++ b/src/surf/PropertyHolder.hpp @@ -27,10 +27,7 @@ public: const char* get_property(const std::string& key) const; void set_property(const std::string& id, const std::string& value); - /* FIXME: This should not be exposed, as users may do bad things with the map they got (it's not a copy). - * But some user API expose this call so removing it is not so easy. - */ - std::unordered_map* get_properties(); + const std::unordered_map* get_properties(); private: std::unique_ptr> properties_ = nullptr; diff --git a/teshsuite/s4u/storage_client_server/storage_client_server.cpp b/teshsuite/s4u/storage_client_server/storage_client_server.cpp index 7e2e9850e2..f5fb17f9ad 100644 --- a/teshsuite/s4u/storage_client_server/storage_client_server.cpp +++ b/teshsuite/s4u/storage_client_server/storage_client_server.cpp @@ -13,7 +13,7 @@ XBT_LOG_NEW_DEFAULT_CATEGORY(storage, "Messages specific for this simulation"); static void display_storage_properties(simgrid::s4u::Storage* storage) { - std::unordered_map* props = storage->get_properties(); + const std::unordered_map* props = storage->get_properties(); if (not props->empty()) { XBT_INFO("\tProperties of mounted storage: %s", storage->get_cname()); diff --git a/teshsuite/s4u/storage_client_server/storage_client_server.tesh b/teshsuite/s4u/storage_client_server/storage_client_server.tesh index 8cc8816b2b..14fe8b0cc6 100644 --- a/teshsuite/s4u/storage_client_server/storage_client_server.tesh +++ b/teshsuite/s4u/storage_client_server/storage_client_server.tesh @@ -56,9 +56,6 @@ $ ./storage_client_server$EXEEXT ${platfdir}/storage/storage.xml "--log=root.fmt > [ 1.207952] (server@alice) Storage name: Disk2, mount name: c: > [ 1.207952] (server@alice) Free size: 534479367024 bytes > [ 1.207952] (server@alice) Used size: 2391544976 bytes -> [ 1.207952] (client@bob) *** GET/SET DATA for storage element: Disk1 *** -> [ 1.207952] (client@bob) Get data: 'No User Data' -> [ 1.207952] (client@bob) Set and get data: 'Some data' > [ 1.207952] (server@alice) No property attached. > [ 1.207952] (server@alice) *** Dump a storage element *** > [ 1.207952] (server@alice) Print the content of the storage element: Disk2 @@ -102,6 +99,9 @@ $ ./storage_client_server$EXEEXT ${platfdir}/storage/storage.xml "--log=root.fmt > [ 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] (client@bob) *** GET/SET DATA for storage element: Disk1 *** +> [ 1.207952] (client@bob) Get data: 'No User Data' +> [ 1.207952] (client@bob) Set and get data: 'Some data' > [ 1.207952] (server@alice) Storage Disk2 is attached to alice > [ 1.207952] (server@alice) Storage Disk3 is attached to carl > [ 1.207952] (server@alice) Storage Disk4 is attached to denise diff --git a/teshsuite/simdag/flatifier/flatifier.cpp b/teshsuite/simdag/flatifier/flatifier.cpp index 844e307c7a..ce9155bcd2 100644 --- a/teshsuite/simdag/flatifier/flatifier.cpp +++ b/teshsuite/simdag/flatifier/flatifier.cpp @@ -42,7 +42,6 @@ static void create_environment(xbt_os_timer_t parse_time, const char *platformFi static void dump_hosts() { - std::unordered_map* props = nullptr; unsigned int totalHosts = sg_host_count(); sg_host_t* hosts = sg_host_list(); std::sort(hosts, hosts + totalHosts, @@ -50,7 +49,7 @@ static void dump_hosts() for (unsigned int i = 0; i < totalHosts; i++) { std::printf(" get_cname(), sg_host_speed(hosts[i])); - props = hosts[i]->get_properties(); + const std::unordered_map* props = hosts[i]->get_properties(); if (hosts[i]->get_core_count() > 1) { std::printf(" core=\"%d\"", hosts[i]->get_core_count()); } -- 2.20.1