From: SUTER Frederic Date: Tue, 28 Sep 2021 14:29:10 +0000 (+0200) Subject: More consistency in resource creation/destruction X-Git-Tag: v3.29~37 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/386393c4094b45cce01179befdd3d519b320db04?ds=sidebyside More consistency in resource creation/destruction --- diff --git a/include/simgrid/s4u/Host.hpp b/include/simgrid/s4u/Host.hpp index d50d8f9650..ae737ffef2 100644 --- a/include/simgrid/s4u/Host.hpp +++ b/include/simgrid/s4u/Host.hpp @@ -59,13 +59,13 @@ protected: public: /** Called on each newly created host */ static xbt::signal on_creation; - /** Called just before destructing a host */ - static xbt::signal on_destruction; /** Called when the machine is turned on or off (called AFTER the change) */ static xbt::signal on_state_change; /** Called when the speed of the machine is changed (called AFTER the change) * (either because of a pstate switch or because of an external load event coming from the profile) */ static xbt::signal on_speed_change; + /** Called just before destructing a host */ + static xbt::signal on_destruction; virtual void destroy(); #ifndef DOXYGEN diff --git a/include/simgrid/s4u/Link.hpp b/include/simgrid/s4u/Link.hpp index 4eb8cf6616..8e144d917f 100644 --- a/include/simgrid/s4u/Link.hpp +++ b/include/simgrid/s4u/Link.hpp @@ -141,9 +141,6 @@ public: /** @brief Callback signal fired when a new Link is created */ static xbt::signal on_creation; - /** @brief Callback signal fired when a Link is destroyed */ - static xbt::signal on_destruction; - /** @brief Callback signal fired when the state of a Link changes (when it is turned on or off) */ static xbt::signal on_state_change; @@ -156,6 +153,9 @@ public: /** @brief Callback signal fired when a communication changes it state (ready/done/cancel) */ static xbt::signal on_communication_state_change; + + /** @brief Callback signal fired when a Link is destroyed */ + static xbt::signal on_destruction; }; /** diff --git a/include/simgrid/s4u/VirtualMachine.hpp b/include/simgrid/s4u/VirtualMachine.hpp index 76e83805ae..73f03e5cf8 100644 --- a/include/simgrid/s4u/VirtualMachine.hpp +++ b/include/simgrid/s4u/VirtualMachine.hpp @@ -23,7 +23,6 @@ namespace s4u { */ class XBT_PUBLIC VirtualMachine : public s4u::Host { vm::VirtualMachineImpl* const pimpl_vm_; - ~VirtualMachine() override; public: explicit VirtualMachine(const std::string& name, Host* physical_host, int core_amount); @@ -60,6 +59,7 @@ public: VirtualMachine* set_bound(double bound); State get_state() const; + static xbt::signal on_creation; static xbt::signal on_start; static xbt::signal on_started; static xbt::signal on_shutdown; @@ -67,6 +67,7 @@ public: static xbt::signal on_resume; static xbt::signal on_migration_start; static xbt::signal on_migration_end; + static xbt::signal on_destruction; }; } // namespace s4u } // namespace simgrid diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp index 4d5cf7ef3a..4f748093a3 100644 --- a/src/kernel/EngineImpl.cpp +++ b/src/kernel/EngineImpl.cpp @@ -161,9 +161,8 @@ EngineImpl::~EngineImpl() for (auto const& kv : netpoints_) delete kv.second; - for (auto const& kv : links_) - if (kv.second) - kv.second->destroy(); + while (not links_.empty()) + links_.begin()->second->destroy(); for (auto const& kv : mailboxes_) delete kv.second; diff --git a/src/plugins/dirty_page_tracking.cpp b/src/plugins/dirty_page_tracking.cpp index 98cf2a97c1..6b643cf664 100644 --- a/src/plugins/dirty_page_tracking.cpp +++ b/src/plugins/dirty_page_tracking.cpp @@ -67,14 +67,14 @@ double DirtyPageTrackingExt::computed_flops_lookup() } // namespace vm } // namespace simgrid -static void on_virtual_machine_creation(simgrid::vm::VirtualMachineImpl& vm) +static void on_virtual_machine_creation(simgrid::s4u::VirtualMachine& vm) { - vm.extension_set(new simgrid::vm::DirtyPageTrackingExt()); + vm.get_vm_impl()->extension_set(new simgrid::vm::DirtyPageTrackingExt()); } static void on_exec_creation(simgrid::s4u::Exec const& e) { - auto exec = static_cast(e.get_impl()); + auto exec = static_cast(e.get_impl()); const simgrid::s4u::VirtualMachine* vm = dynamic_cast(exec->get_host()); if (vm == nullptr) return; @@ -107,7 +107,7 @@ void sg_vm_dirty_page_tracking_init() if (not simgrid::vm::DirtyPageTrackingExt::EXTENSION_ID.valid()) { simgrid::vm::DirtyPageTrackingExt::EXTENSION_ID = simgrid::vm::VirtualMachineImpl::extension_create(); - simgrid::vm::VirtualMachineImpl::on_creation.connect(&on_virtual_machine_creation); + simgrid::s4u::VirtualMachine::on_creation.connect(&on_virtual_machine_creation); simgrid::s4u::Exec::on_start.connect(&on_exec_creation); simgrid::s4u::Exec::on_completion.connect(&on_exec_completion); } diff --git a/src/plugins/vm/VirtualMachineImpl.cpp b/src/plugins/vm/VirtualMachineImpl.cpp index 0f389ca840..6fe26b7f15 100644 --- a/src/plugins/vm/VirtualMachineImpl.cpp +++ b/src/plugins/vm/VirtualMachineImpl.cpp @@ -40,11 +40,6 @@ namespace simgrid { template class xbt::Extendable; namespace vm { -/************* - * Callbacks * - *************/ -xbt::signal VirtualMachineImpl::on_creation; -xbt::signal VirtualMachineImpl::on_destruction; /********* * Model * @@ -191,13 +186,12 @@ VirtualMachineImpl::VirtualMachineImpl(const std::string& name, s4u::VirtualMach update_action_weight(); XBT_VERB("Create VM(%s)@PM(%s)", name.c_str(), physical_host_->get_cname()); - on_creation(*this); } /** @brief A physical host does not disappear in the current SimGrid code, but a VM may disappear during a simulation */ -VirtualMachineImpl::~VirtualMachineImpl() +void VirtualMachineImpl::destroy() { - on_destruction(*this); + s4u::VirtualMachine::on_destruction(*piface_); /* I was already removed from the allVms set if the VM was destroyed cleanly */ auto iter = find(allVms_.begin(), allVms_.end(), piface_); if (iter != allVms_.end()) @@ -210,7 +204,7 @@ VirtualMachineImpl::~VirtualMachineImpl() void VirtualMachineImpl::suspend(smx_actor_t issuer) { - if (get_state() != s4u::VirtualMachine::State::RUNNING) + if (vm_state_ != s4u::VirtualMachine::State::RUNNING) throw VmFailureException(XBT_THROW_POINT, xbt::string_printf("Cannot suspend VM %s: it is not running.", piface_->get_cname())); if (issuer->get_host() == piface_) @@ -233,7 +227,7 @@ void VirtualMachineImpl::suspend(smx_actor_t issuer) void VirtualMachineImpl::resume() { - if (get_state() != s4u::VirtualMachine::State::SUSPENDED) + if (vm_state_ != s4u::VirtualMachine::State::SUSPENDED) throw VmFailureException(XBT_THROW_POINT, xbt::string_printf("Cannot resume VM %s: it was not suspended", piface_->get_cname())); @@ -258,7 +252,7 @@ void VirtualMachineImpl::resume() */ void VirtualMachineImpl::shutdown(smx_actor_t issuer) { - if (get_state() != s4u::VirtualMachine::State::RUNNING) + if (vm_state_ != s4u::VirtualMachine::State::RUNNING) XBT_VERB("Shutting down the VM %s even if it's not running but in state %s", piface_->get_cname(), s4u::VirtualMachine::to_c_str(get_state())); diff --git a/src/plugins/vm/VirtualMachineImpl.hpp b/src/plugins/vm/VirtualMachineImpl.hpp index 68bd7c3b0c..c77576145f 100644 --- a/src/plugins/vm/VirtualMachineImpl.hpp +++ b/src/plugins/vm/VirtualMachineImpl.hpp @@ -33,20 +33,15 @@ class XBT_PUBLIC VirtualMachineImpl : public surf::HostImpl, public simgrid::xbt #endif public: - /** @brief Callbacks fired after VM creation. Signature: `void(VirtualMachineImpl&)` */ - static xbt::signal on_creation; - /** @brief Callbacks fired after VM destruction. Signature: `void(VirtualMachineImpl const&)` */ - static xbt::signal on_destruction; - static std::deque allVms_; explicit VirtualMachineImpl(const std::string& name, s4u::VirtualMachine* piface, s4u::Host* host, int core_amount, size_t ramsize); - ~VirtualMachineImpl() override; void suspend(kernel::actor::ActorImpl* issuer); void resume(); void shutdown(kernel::actor::ActorImpl* issuer); + void destroy(); /** @brief Change the physical host on which the given VM is running */ void set_physical_host(s4u::Host* dest); diff --git a/src/plugins/vm/s4u_VirtualMachine.cpp b/src/plugins/vm/s4u_VirtualMachine.cpp index 486a75b4cb..ce07172831 100644 --- a/src/plugins/vm/s4u_VirtualMachine.cpp +++ b/src/plugins/vm/s4u_VirtualMachine.cpp @@ -16,6 +16,7 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(s4u_vm, s4u, "S4U virtual machines"); namespace simgrid { namespace s4u { +simgrid::xbt::signal VirtualMachine::on_creation; simgrid::xbt::signal VirtualMachine::on_start; simgrid::xbt::signal VirtualMachine::on_started; simgrid::xbt::signal VirtualMachine::on_shutdown; @@ -23,6 +24,7 @@ simgrid::xbt::signal VirtualMachine::on_suspend; simgrid::xbt::signal VirtualMachine::on_resume; simgrid::xbt::signal VirtualMachine::on_migration_start; simgrid::xbt::signal VirtualMachine::on_migration_end; +simgrid::xbt::signal VirtualMachine::on_destruction; VirtualMachine::VirtualMachine(const std::string& name, s4u::Host* physical_host, int core_amount) : VirtualMachine(name, physical_host, core_amount, 1024) @@ -54,14 +56,7 @@ VirtualMachine::VirtualMachine(const std::string& name, s4u::Host* physical_host set_pstate(physical_host->get_pstate()); seal(); // seal this host -} - -VirtualMachine::~VirtualMachine() -{ - on_destruction(*this); - - /* Don't free these things twice: they are the ones of my physical host */ - set_netpoint(nullptr); + s4u::VirtualMachine::on_creation(*this); } void VirtualMachine::start() @@ -127,7 +122,11 @@ void VirtualMachine::destroy() /* Then, destroy the VM object */ kernel::actor::simcall([this]() { + get_vm_impl()->destroy(); get_impl()->destroy(); + + /* Don't free these things twice: they are the ones of my physical host */ + set_netpoint(nullptr); delete this; }); }; diff --git a/src/s4u/s4u_Disk.cpp b/src/s4u/s4u_Disk.cpp index 7ad08c3a0b..e9d72220dd 100644 --- a/src/s4u/s4u_Disk.cpp +++ b/src/s4u/s4u_Disk.cpp @@ -157,7 +157,7 @@ Disk* Disk::set_factor_cb(const std::function& cb) Disk* Disk::seal() { kernel::actor::simcall([this]{ pimpl_->seal(); }); - Disk::on_creation(*this); + Disk::on_creation(*this); // notify the signal return this; } } // namespace s4u diff --git a/src/s4u/s4u_Link.cpp b/src/s4u/s4u_Link.cpp index 9c6d41a8c9..a9d97532e5 100644 --- a/src/s4u/s4u_Link.cpp +++ b/src/s4u/s4u_Link.cpp @@ -145,6 +145,7 @@ void Link::turn_off() Link* Link::seal() { kernel::actor::simcall([this]() { this->pimpl_->seal(); }); + s4u::Link::on_creation(*this); // notify the signal return this; } diff --git a/src/surf/HostImpl.cpp b/src/surf/HostImpl.cpp index e81edd4ba6..aeac40d43f 100644 --- a/src/surf/HostImpl.cpp +++ b/src/surf/HostImpl.cpp @@ -54,7 +54,7 @@ HostImpl::~HostImpl() actors_at_boot_.clear(); for (auto const& d : disks_) - d->destroy(); + d.second->destroy(); } /** @brief Fire the required callbacks and destroy the object @@ -125,7 +125,7 @@ std::vector HostImpl::get_disks() const { std::vector disks; for (auto const& d : disks_) - disks.push_back(d->get_iface()); + disks.push_back(d.second->get_iface()); return disks; } @@ -138,19 +138,12 @@ s4u::Disk* HostImpl::create_disk(const std::string& name, double read_bandwidth, void HostImpl::add_disk(const s4u::Disk* disk) { - disks_.push_back(disk->get_impl()); + disks_[disk->get_name()] = disk->get_impl(); } -void HostImpl::remove_disk(const std::string& disk_name) +void HostImpl::remove_disk(const std::string& name) { - auto position = disks_.begin(); - for (auto const& d : disks_) { - if (d->get_name() == disk_name) { - disks_.erase(position); - break; - } - position++; - } + disks_.erase(name); } void HostImpl::seal() @@ -163,8 +156,8 @@ void HostImpl::seal() sealed_ = true; /* seal its disks */ - for (auto* disk : disks_) - disk->seal(); + for (auto const& disk : disks_) + disk.second->seal(); } } // namespace surf } // namespace simgrid diff --git a/src/surf/HostImpl.hpp b/src/surf/HostImpl.hpp index 6ac9e5af0f..8b16e7ce03 100644 --- a/src/surf/HostImpl.hpp +++ b/src/surf/HostImpl.hpp @@ -49,7 +49,7 @@ class XBT_PRIVATE HostImpl : public xbt::PropertyHolder { ActorList actor_list_; std::vector actors_at_boot_; s4u::Host piface_; - std::vector disks_; + std::map> disks_; xbt::string name_{"noname"}; bool sealed_ = false; @@ -66,7 +66,7 @@ public: std::vector get_disks() const; s4u::Disk* create_disk(const std::string& name, double read_bandwidth, double write_bandwidth); void add_disk(const s4u::Disk* disk); - void remove_disk(const std::string& disk_name); + void remove_disk(const std::string& name); virtual const s4u::Host* get_iface() const { return &piface_; } virtual s4u::Host* get_iface() { return &piface_; } diff --git a/src/surf/LinkImpl.cpp b/src/surf/LinkImpl.cpp index ecbf1ac3af..d97bc34f65 100644 --- a/src/surf/LinkImpl.cpp +++ b/src/surf/LinkImpl.cpp @@ -34,7 +34,8 @@ LinkImpl::LinkImpl(const std::string& name) : LinkImplIntf(name), piface_(this) */ void LinkImpl::destroy() { - s4u::Link::on_destruction(this->piface_); + s4u::Link::on_destruction(piface_); + s4u::Engine::get_instance()->link_unregister(get_name()); delete this; } @@ -110,7 +111,6 @@ void LinkImpl::seal() xbt_assert(this->get_model(), "Cannot seal Link(%s) without setting the Network model first", this->get_cname()); Resource::seal(); - s4u::Link::on_creation(piface_); } void LinkImpl::on_bandwidth_change() const @@ -144,4 +144,4 @@ void LinkImpl::set_concurrency_limit(int limit) const } // namespace resource } // namespace kernel -} // namespace simgrid \ No newline at end of file +} // namespace simgrid