From e4b0e5563bf3957c69041a31c1ccc6a0274e1792 Mon Sep 17 00:00:00 2001 From: Bruno Donassolo Date: Mon, 3 Jan 2022 19:12:47 +0100 Subject: [PATCH] Mix XML and C++ platforms Fixes https://framagit.org/simgrid/simgrid/-/issues/99 Main changes: - Allows adding resources to a netzone after loading the XML platform. - Disallows adding resources to a sealed netzone. - Move the seal of resources from s4u::Engine:run to EngineImpl::run. - Remove seal from load_platform (to permit adding resources to netzones) --- ChangeLog | 4 ++++ include/simgrid/s4u/Engine.hpp | 1 + src/kernel/EngineImpl.cpp | 11 +++++++++++ src/kernel/EngineImpl.hpp | 1 + src/kernel/routing/NetZoneImpl.cpp | 10 ++++++++++ src/s4u/s4u_Engine.cpp | 16 ++++++++++------ src/surf/sg_platf.cpp | 14 +++++--------- teshsuite/platforms/flatifier.cpp | 1 + .../basic-parsing-test/basic-parsing-test.cpp | 1 + 9 files changed, 44 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index a563d4078e..d6c10fdcaa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -23,6 +23,10 @@ Python: - Thread contexts are used by default with Python bindings. Other kinds of contexts revealed unstable, specially starting with pybind11 v2.8.0. +Fixed bugs (FG#.. -> FramaGit bugs; FG!.. -> FG merge requests) + (FG: issues on Framagit; GF: issues on GForge; GH: issues on GitHub) + - FG#99: Weird segfault when not sealing an host + ---------------------------------------------------------------------------- SimGrid (3.29) October 7. 2021 diff --git a/include/simgrid/s4u/Engine.hpp b/include/simgrid/s4u/Engine.hpp index 31f6a4088c..7b298a4981 100644 --- a/include/simgrid/s4u/Engine.hpp +++ b/include/simgrid/s4u/Engine.hpp @@ -58,6 +58,7 @@ public: static bool has_instance() { return instance_ != nullptr; } void load_platform(const std::string& platf) const; + void seal_platform() const; void register_function(const std::string& name, const std::function& code); void register_function(const std::string& name, const std::function)>& code); diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp index 665e7cbf97..4a38f962ef 100644 --- a/src/kernel/EngineImpl.cpp +++ b/src/kernel/EngineImpl.cpp @@ -347,6 +347,15 @@ void EngineImpl::shutdown() instance_ = nullptr; } +void EngineImpl::seal_platform() const +{ + /* sealing resources before run: links */ + for (auto const& kv : links_) + kv.second->get_iface()->seal(); + /* seal netzone root, recursively seal children netzones, hosts and disks */ + netzone_root_->seal(); +} + void EngineImpl::load_platform(const std::string& platf) { double start = xbt_os_time(); @@ -689,6 +698,8 @@ double EngineImpl::solve(double max_date) const void EngineImpl::run(double max_date) { + seal_platform(); + if (MC_record_replay_is_active()) { mc::replay(MC_record_path()); empty_trash(); diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp index ed0c976bb9..2fcab66f61 100644 --- a/src/kernel/EngineImpl.hpp +++ b/src/kernel/EngineImpl.hpp @@ -94,6 +94,7 @@ public: void initialize(int* argc, char** argv); void load_platform(const std::string& platf); void load_deployment(const std::string& file) const; + void seal_platform() const; void register_function(const std::string& name, const actor::ActorCodeFactory& code); void register_default(const actor::ActorCodeFactory& code); diff --git a/src/kernel/routing/NetZoneImpl.cpp b/src/kernel/routing/NetZoneImpl.cpp index 7f8f8eec84..68768afea6 100644 --- a/src/kernel/routing/NetZoneImpl.cpp +++ b/src/kernel/routing/NetZoneImpl.cpp @@ -151,6 +151,7 @@ s4u::Host* NetZoneImpl::create_host(const std::string& name, const std::vectorget_iface(); res->set_netpoint((new NetPoint(name, NetPoint::Type::Host))->set_englobing_zone(this)); @@ -165,12 +166,19 @@ s4u::Link* NetZoneImpl::create_link(const std::string& name, const std::vectorcreate_link(name, bandwidths)->get_iface(); } s4u::SplitDuplexLink* NetZoneImpl::create_split_duplex_link(const std::string& name, const std::vector& bandwidths) { + xbt_assert( + network_model_, + "Impossible to create link: %s. Invalid network model: nullptr. Have you set the parent of this NetZone: %s?", + name.c_str(), get_cname()); + xbt_assert(not sealed_, "Impossible to create link: %s. NetZone %s already sealed", name.c_str(), get_cname()); + auto* link_up = network_model_->create_link(name + "_UP", bandwidths); auto* link_down = network_model_->create_link(name + "_DOWN", bandwidths); auto link = std::make_unique(name, link_up, link_down); @@ -184,6 +192,7 @@ s4u::Disk* NetZoneImpl::create_disk(const std::string& name, double read_bandwid xbt_assert(disk_model_, "Impossible to create disk: %s. Invalid disk model: nullptr. Have you set the parent of this NetZone: %s?", name.c_str(), get_cname()); + xbt_assert(not sealed_, "Impossible to create disk: %s. NetZone %s already sealed", name.c_str(), get_cname()); auto* l = disk_model_->create_disk(name, read_bandwidth, write_bandwidth); return l->get_iface(); @@ -193,6 +202,7 @@ NetPoint* NetZoneImpl::create_router(const std::string& name) { xbt_assert(nullptr == s4u::Engine::get_instance()->netpoint_by_name_or_null(name), "Refusing to create a router named '%s': this name already describes a node.", name.c_str()); + xbt_assert(not sealed_, "Impossible to create router: %s. NetZone %s already sealed", name.c_str(), get_cname()); return (new NetPoint(name, NetPoint::Type::Router))->set_englobing_zone(this); } diff --git a/src/s4u/s4u_Engine.cpp b/src/s4u/s4u_Engine.cpp index 0d1c8f8943..68e14760a5 100644 --- a/src/s4u/s4u_Engine.cpp +++ b/src/s4u/s4u_Engine.cpp @@ -112,6 +112,16 @@ void Engine::load_platform(const std::string& platf) const pimpl->load_platform(platf); } +/** + * @brief Seals the platform, finishing the creation of its resources. + * + * This method is optional. The seal() is done automatically when you call Engine::run. + */ +void Engine::seal_platform() const +{ + pimpl->seal_platform(); +} + /** Registers the main function of an actor that will be launched from the deployment file */ void Engine::register_function(const std::string& name, const std::function& code) { @@ -321,12 +331,6 @@ void Engine::run() const } void Engine::run_until(double max_date) const { - /* sealing resources before run: links */ - for (auto* link : get_all_links()) - link->seal(); - /* seal netzone root, recursively seal children netzones, hosts and disks */ - get_netzone_root()->seal(); - /* Clean IO before the run */ fflush(stdout); fflush(stderr); diff --git a/src/surf/sg_platf.cpp b/src/surf/sg_platf.cpp index bf52327199..dd91fbfc03 100644 --- a/src/surf/sg_platf.cpp +++ b/src/surf/sg_platf.cpp @@ -223,27 +223,25 @@ static void sg_platf_new_cluster_hierarchical(const simgrid::kernel::routing::Cl } simgrid::s4u::NetZone const* parent = current_routing ? current_routing->get_iface() : nullptr; - simgrid::s4u::NetZone* zone; switch (cluster->topology) { case simgrid::kernel::routing::ClusterTopology::TORUS: - zone = simgrid::s4u::create_torus_zone( - cluster->id, parent, TorusZone::parse_topo_parameters(cluster->topo_parameters), - {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat, cluster->sharing_policy); + simgrid::s4u::create_torus_zone(cluster->id, parent, TorusZone::parse_topo_parameters(cluster->topo_parameters), + {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat, + cluster->sharing_policy); break; case simgrid::kernel::routing::ClusterTopology::DRAGONFLY: - zone = simgrid::s4u::create_dragonfly_zone( + simgrid::s4u::create_dragonfly_zone( cluster->id, parent, DragonflyZone::parse_topo_parameters(cluster->topo_parameters), {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat, cluster->sharing_policy); break; case simgrid::kernel::routing::ClusterTopology::FAT_TREE: - zone = simgrid::s4u::create_fatTree_zone( + simgrid::s4u::create_fatTree_zone( cluster->id, parent, FatTreeZone::parse_topo_parameters(cluster->topo_parameters), {set_host, set_loopback, set_limiter}, cluster->bw, cluster->lat, cluster->sharing_policy); break; default: THROW_IMPOSSIBLE; } - zone->seal(); } /*************************************************************************************************/ @@ -336,7 +334,6 @@ static void sg_platf_new_cluster_flat(simgrid::kernel::routing::ClusterCreationA auto* router = zone->create_router(cluster->router_id); zone->add_route(router, nullptr, nullptr, nullptr, {}); - zone->seal(); simgrid::kernel::routing::on_cluster_creation(*cluster); } @@ -602,7 +599,6 @@ void sg_platf_new_zone_seal() zone_cluster.cabinets.clear(); zone_cluster.backbone.reset(); } - current_routing->seal(); current_routing = current_routing->get_parent(); } diff --git a/teshsuite/platforms/flatifier.cpp b/teshsuite/platforms/flatifier.cpp index 10bc1ae440..2ab089a7f3 100644 --- a/teshsuite/platforms/flatifier.cpp +++ b/teshsuite/platforms/flatifier.cpp @@ -40,6 +40,7 @@ static void create_environment(xbt_os_timer_t parse_time, const std::string& pla { xbt_os_cputimer_start(parse_time); sg4::Engine::get_instance()->load_platform(platformFile); + sg4::Engine::get_instance()->seal_platform(); xbt_os_cputimer_stop(parse_time); } diff --git a/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp b/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp index 442faf39d5..cba0d49aa8 100644 --- a/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp +++ b/teshsuite/s4u/basic-parsing-test/basic-parsing-test.cpp @@ -63,6 +63,7 @@ int main(int argc, char** argv) /* creation of the environment */ e.load_platform(argv[1]); + e.seal_platform(); XBT_INFO("Workstation number: %zu, link number: %zu", e.get_host_count(), e.get_link_count()); std::vector hosts = e.get_all_hosts(); -- 2.20.1