From 6ae07464eeacbd09ee8b4ff9630c78d62bfe699e Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Mon, 17 Oct 2016 20:54:54 +0200 Subject: [PATCH] put the HostImpl as s4u::Host->pimpl instead of as an extension --- include/simgrid/forward.h | 1 + include/simgrid/s4u/host.hpp | 1 + src/s4u/s4u_host.cpp | 22 ++++++++++------------ src/simgrid/host.cpp | 4 ++-- src/simix/smx_host.cpp | 8 +++----- src/simix/smx_vm.cpp | 10 +++++----- src/surf/HostImpl.cpp | 12 +++--------- src/surf/HostImpl.hpp | 2 -- src/surf/plugins/energy.cpp | 15 ++++++--------- src/surf/surf_c_bindings.cpp | 4 ++-- src/surf/virtual_machine.cpp | 5 +++-- 11 files changed, 36 insertions(+), 48 deletions(-) diff --git a/include/simgrid/forward.h b/include/simgrid/forward.h index 09a183fa35..054ce67c99 100644 --- a/include/simgrid/forward.h +++ b/include/simgrid/forward.h @@ -32,6 +32,7 @@ namespace simgrid { class Resource; class Cpu; class Link; + class HostImpl; } namespace trace_mgr { class trace; diff --git a/include/simgrid/s4u/host.hpp b/include/simgrid/s4u/host.hpp index 2eeded4cee..ac7174d64c 100644 --- a/include/simgrid/s4u/host.hpp +++ b/include/simgrid/s4u/host.hpp @@ -102,6 +102,7 @@ private: public: // TODO, this could be a unique_ptr + surf::HostImpl* pimpl_ = nullptr; /** DO NOT USE DIRECTLY (@todo: these should be protected, once our code is clean) */ surf::Cpu *pimpl_cpu = nullptr; /** DO NOT USE DIRECTLY (@todo: these should be protected, once our code is clean) */ diff --git a/src/s4u/s4u_host.cpp b/src/s4u/s4u_host.cpp index cbd28f1cfa..56229fed33 100644 --- a/src/s4u/s4u_host.cpp +++ b/src/s4u/s4u_host.cpp @@ -48,6 +48,7 @@ Host::~Host() { xbt_assert(currentlyDestroying_, "Please call h->destroy() instead of manually deleting it."); + delete pimpl_; delete pimpl_cpu; delete pimpl_netcard; delete mounts; @@ -57,7 +58,7 @@ Host::~Host() * * Don't delete directly an Host, call h->destroy() instead. * - * This is cumbersome but there is the simplest solution to ensure that the + * This is cumbersome but this is the simplest solution to ensure that the * onDestruction() callback receives a valid object (because of the destructor * order in a class hierarchy). */ @@ -97,7 +98,7 @@ void Host::turnOn() { if (isOff()) { simgrid::simix::kernelImmediate([&]{ this->extension()->turnOn(); - this->extension()->turnOn(); + this->pimpl_->turnOn(); }); } } @@ -137,20 +138,17 @@ boost::unordered_map const& Host::mountedStorages() { /** Get the properties assigned to a host */ xbt_dict_t Host::properties() { return simgrid::simix::kernelImmediate([&] { - simgrid::surf::HostImpl* surf_host = this->extension(); - return surf_host->getProperties(); + return this->pimpl_->getProperties(); }); } /** Retrieve the property value (or nullptr if not set) */ const char*Host::property(const char*key) { - simgrid::surf::HostImpl* surf_host = this->extension(); - return surf_host->getProperty(key); + return this->pimpl_->getProperty(key); } void Host::setProperty(const char*key, const char *value){ simgrid::simix::kernelImmediate([&] { - simgrid::surf::HostImpl* surf_host = this->extension(); - surf_host->setProperty(key,value); + this->pimpl_->setProperty(key,value); }); } @@ -203,14 +201,14 @@ int Host::pstate() void Host::parameters(vm_params_t params) { simgrid::simix::kernelImmediate([&]() { - this->extension()->getParams(params); + this->pimpl_->getParams(params); }); } void Host::setParameters(vm_params_t params) { simgrid::simix::kernelImmediate([&]() { - this->extension()->setParams(params); + this->pimpl_->setParams(params); }); } @@ -222,7 +220,7 @@ void Host::setParameters(vm_params_t params) xbt_dict_t Host::mountedStoragesAsDict() { return simgrid::simix::kernelImmediate([&] { - return this->extension()->getMountedStorageList(); + return this->pimpl_->getMountedStorageList(); }); } @@ -234,7 +232,7 @@ xbt_dict_t Host::mountedStoragesAsDict() xbt_dynar_t Host::attachedStorages() { return simgrid::simix::kernelImmediate([&] { - return this->extension()->getAttachedStorageList(); + return this->pimpl_->getAttachedStorageList(); }); } diff --git a/src/simgrid/host.cpp b/src/simgrid/host.cpp index fdceaacf89..963302e1e2 100644 --- a/src/simgrid/host.cpp +++ b/src/simgrid/host.cpp @@ -109,11 +109,11 @@ smx_host_priv_t sg_host_simix(sg_host_t host){ // ========= storage related functions ============ xbt_dict_t sg_host_get_mounted_storage_list(sg_host_t host){ - return host->extension()->getMountedStorageList(); + return host->pimpl_->getMountedStorageList(); } xbt_dynar_t sg_host_get_attached_storage_list(sg_host_t host){ - return host->extension()->getAttachedStorageList(); + return host->pimpl_->getAttachedStorageList(); } diff --git a/src/simix/smx_host.cpp b/src/simix/smx_host.cpp index bc21bdd42e..57a50270eb 100644 --- a/src/simix/smx_host.cpp +++ b/src/simix/smx_host.cpp @@ -84,8 +84,7 @@ void SIMIX_host_off(sg_host_t h, smx_actor_t issuer) xbt_assert((host != nullptr), "Invalid parameters"); if (h->isOn()) { - simgrid::surf::HostImpl* surf_host = h->extension(); - surf_host->turnOff(); + h->pimpl_->turnOff(); /* Clean Simulator data */ if (xbt_swag_size(host->process_list) != 0) { @@ -204,10 +203,9 @@ smx_activity_t SIMIX_execution_parallel_start(const char *name, int host_nb, sg_ host_list_cpy[i] = host_list[i]; /* Check that we are not mixing VMs and PMs in the parallel task */ - simgrid::surf::HostImpl *host = host_list[0]->extension(); - bool is_a_vm = (nullptr != dynamic_cast(host)); + bool is_a_vm = (nullptr != dynamic_cast(host_list[0]->pimpl_)); for (int i = 1; i < host_nb; i++) { - bool tmp_is_a_vm = (nullptr != dynamic_cast(host_list[i]->extension())); + bool tmp_is_a_vm = (nullptr != dynamic_cast(host_list[i]->pimpl_)); xbt_assert(is_a_vm == tmp_is_a_vm, "parallel_execute: mixing VMs and PMs is not supported (yet)."); } diff --git a/src/simix/smx_vm.cpp b/src/simix/smx_vm.cpp index e582a31b9e..fec4850959 100644 --- a/src/simix/smx_vm.cpp +++ b/src/simix/smx_vm.cpp @@ -14,7 +14,7 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(simix_vm, simix, "Logging specific to SIMIX Virt static long host_get_ramsize(sg_host_t vm, int *overcommit) { s_vm_params_t params; - vm->extension()->getParams(¶ms); + vm->pimpl_->getParams(¶ms); if (overcommit) *overcommit = params.overcommit; @@ -42,7 +42,7 @@ static int __can_be_started(sg_host_t vm) } long total_ramsize_of_vms = 0; - xbt_dynar_t dyn_vms = pm->extension()->getVms(); + xbt_dynar_t dyn_vms = pm->pimpl_->getVms(); { unsigned int cursor = 0; sg_host_t another_vm; @@ -67,7 +67,7 @@ void SIMIX_vm_start(sg_host_t vm) { if (__can_be_started(vm)) static_cast( - vm->extension() + vm->pimpl_ )->setState(SURF_VM_STATE_RUNNING); else THROWF(vm_error, 0, "The VM %s cannot be started", vm->name().c_str()); @@ -77,7 +77,7 @@ void SIMIX_vm_start(sg_host_t vm) e_surf_vm_state_t SIMIX_vm_get_state(sg_host_t vm) { return static_cast( - vm->extension() + vm->pimpl_ )->getState(); } @@ -289,7 +289,7 @@ void SIMIX_vm_shutdown(sg_host_t vm, smx_actor_t issuer) /* FIXME: we may have to do something at the surf layer, e.g., vcpu action */ static_cast( - vm->extension() + vm->pimpl_ )->setState(SURF_VM_STATE_CREATED); } diff --git a/src/surf/HostImpl.cpp b/src/surf/HostImpl.cpp index 6e97afd76d..877fce9ddf 100644 --- a/src/surf/HostImpl.cpp +++ b/src/surf/HostImpl.cpp @@ -24,8 +24,6 @@ simgrid::surf::HostModel *surf_host_model = nullptr; namespace simgrid { namespace surf { -simgrid::xbt::Extension HostImpl::EXTENSION_ID; - /********* * Model * *********/ @@ -105,8 +103,6 @@ HostImpl::HostImpl(simgrid::surf::HostModel *model, const char *name, xbt_dynar_ , PropertyHolder(nullptr) , storage_(storage), cpu_(cpu) { - if (!EXTENSION_ID.valid()) - EXTENSION_ID = simgrid::s4u::Host::extension_create(); params_.ramsize = 0; } @@ -120,15 +116,13 @@ HostImpl::HostImpl(simgrid::surf::HostModel *model, const char *name, lmm_constr } /** @brief use destroy() instead of this destructor */ -HostImpl::~HostImpl() -{ -} +HostImpl::~HostImpl() = default; void HostImpl::attach(simgrid::s4u::Host* host) { if (piface_ != nullptr) xbt_die("Already attached to host %s", host->name().c_str()); - host->extension_set(this); + host->pimpl_ = this; piface_ = host; } @@ -365,7 +359,7 @@ xbt_dynar_t HostImpl::getVms() xbt_dynar_t dyn = xbt_dynar_new(sizeof(simgrid::surf::VirtualMachine*), nullptr); for (VirtualMachine *ws_vm : VirtualMachine::allVms_) { - if (this == ws_vm->getPm()->extension()) + if (this == ws_vm->getPm()->pimpl_) xbt_dynar_push(dyn, &ws_vm); } diff --git a/src/surf/HostImpl.hpp b/src/surf/HostImpl.hpp index 6734180cdf..a47d5660d0 100644 --- a/src/surf/HostImpl.hpp +++ b/src/surf/HostImpl.hpp @@ -66,8 +66,6 @@ public: class HostImpl : public simgrid::surf::Resource, public simgrid::surf::PropertyHolder { -public: - static simgrid::xbt::Extension EXTENSION_ID; public: /** diff --git a/src/surf/plugins/energy.cpp b/src/surf/plugins/energy.cpp index 48085230b4..32e79f7cf3 100644 --- a/src/surf/plugins/energy.cpp +++ b/src/surf/plugins/energy.cpp @@ -65,7 +65,7 @@ simgrid::xbt::Extension HostEnergy::EXTENSION_ID /* Computes the consumption so far. Called lazily on need. */ void HostEnergy::update() { - simgrid::surf::HostImpl* surf_host = host->extension(); + simgrid::surf::HostImpl* surf_host = host->pimpl_; double start_time = this->last_updated; double finish_time = surf_get_clock(); double cpu_load; @@ -234,8 +234,7 @@ void HostEnergy::initWattsRangeList() /* **************************** events callback *************************** */ static void onCreation(simgrid::s4u::Host& host) { - simgrid::surf::HostImpl* surf_host = host.extension(); - if (dynamic_cast(surf_host)) // Ignore virtual machines + if (dynamic_cast(host.pimpl_)) // Ignore virtual machines return; host.extension_set(new HostEnergy(&host)); } @@ -246,10 +245,10 @@ static void onActionStateChange(simgrid::surf::CpuAction *action, simgrid::surf: sg_host_t sghost = sg_host_by_name(name); if(sghost == nullptr) continue; - simgrid::surf::HostImpl *host = sghost->extension(); + simgrid::surf::HostImpl *host = sghost->pimpl_; simgrid::surf::VirtualMachine *vm = dynamic_cast(host); if (vm) // If it's a VM, take the corresponding PM - host = vm->getPm()->extension(); + host = vm->getPm()->pimpl_; HostEnergy *host_energy = host->piface_->extension(); @@ -259,8 +258,7 @@ static void onActionStateChange(simgrid::surf::CpuAction *action, simgrid::surf: } static void onHostStateChange(simgrid::s4u::Host &host) { - simgrid::surf::HostImpl* surf_host = host.extension(); - if (dynamic_cast(surf_host)) // Ignore virtual machines + if (dynamic_cast(host.pimpl_)) // Ignore virtual machines return; HostEnergy *host_energy = host.extension(); @@ -271,8 +269,7 @@ static void onHostStateChange(simgrid::s4u::Host &host) { static void onHostDestruction(simgrid::s4u::Host& host) { // Ignore virtual machines - simgrid::surf::HostImpl* surf_host = host.extension(); - if (dynamic_cast(surf_host)) + if (dynamic_cast(host.pimpl_)) return; HostEnergy *host_energy = host.extension(); host_energy->update(); diff --git a/src/surf/surf_c_bindings.cpp b/src/surf/surf_c_bindings.cpp index 5ff85aa899..c599164461 100644 --- a/src/surf/surf_c_bindings.cpp +++ b/src/surf/surf_c_bindings.cpp @@ -18,11 +18,11 @@ XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(surf_kernel); *********/ static simgrid::surf::HostImpl *get_casted_host(sg_host_t host){ //FIXME: killme - return host->extension(); + return host->pimpl_; } static simgrid::surf::VirtualMachine *get_casted_vm(sg_host_t host){ - return static_cast(host->extension()); + return static_cast(host->pimpl_); } extern double NOW; diff --git a/src/surf/virtual_machine.cpp b/src/surf/virtual_machine.cpp index 794ec016c5..f28f6aed57 100644 --- a/src/surf/virtual_machine.cpp +++ b/src/surf/virtual_machine.cpp @@ -108,8 +108,9 @@ VirtualMachine::VirtualMachine(HostModel* model, const char* name, simgrid::s4u: { /* Register this VM to the list of all VMs */ allVms_.push_back(this); + piface_ = new simgrid::s4u::Host(name); - piface_->extension_set(this); + piface_->pimpl_ = this; /* Currently, a VM uses the network resource of its physical host. In * host_lib, this network resource object is referred from two different keys. @@ -202,7 +203,7 @@ sg_host_t VirtualMachine::getPm() { /* Update the physical host of the given VM */ void VirtualMachine::migrate(sg_host_t host_dest) { - HostImpl *surfHost_dst = host_dest->extension(); + HostImpl *surfHost_dst = host_dest->pimpl_; const char *vm_name = getName(); const char *pm_name_src = hostPM_->name().c_str(); const char *pm_name_dst = surfHost_dst->getName(); -- 2.20.1