From 3ed98d31c172001583f905cfbabaab636651169c Mon Sep 17 00:00:00 2001 From: Frederic Suter Date: Mon, 1 Mar 2021 10:24:24 +0100 Subject: [PATCH] Rollback a little in programmatic description of disks - add Required fields (w.r.t. the DTD) to the create_disk method - leave the setters to enable changes of mind or dynamic stuff between create and seal (TODO: add a state and some asserts) - Resources always have a name so add it in the ctor --- examples/cpp/io-disk-raw/s4u-io-disk-raw.cpp | 6 +----- include/simgrid/kernel/resource/Resource.hpp | 3 +-- include/simgrid/s4u/Disk.hpp | 2 +- include/simgrid/s4u/Host.hpp | 2 +- src/kernel/resource/DiskImpl.hpp | 8 ++++++-- src/kernel/resource/Resource.cpp | 6 ------ src/kernel/resource/profile/Profile_test.cpp | 2 +- src/s4u/s4u_Host.cpp | 4 ++-- src/surf/cpu_interface.cpp | 5 +++-- src/surf/disk_s19.cpp | 5 ++--- src/surf/disk_s19.hpp | 4 ++-- src/surf/network_interface.cpp | 4 ++-- src/surf/sg_platf.cpp | 8 +------- 13 files changed, 23 insertions(+), 36 deletions(-) diff --git a/examples/cpp/io-disk-raw/s4u-io-disk-raw.cpp b/examples/cpp/io-disk-raw/s4u-io-disk-raw.cpp index 5498513510..8d05ad6602 100644 --- a/examples/cpp/io-disk-raw/s4u-io-disk-raw.cpp +++ b/examples/cpp/io-disk-raw/s4u-io-disk-raw.cpp @@ -12,11 +12,7 @@ XBT_LOG_NEW_DEFAULT_CATEGORY(disk_test, "Messages specific for this simulation") static void host() { /* -Add an extra disk in a programmatic way */ - simgrid::s4u::Host::current()->create_disk() - ->set_name("Disk3") - ->set_read_bandwidth(9.6e7) - ->set_write_bandwidth(6.4e7) - ->seal(); + simgrid::s4u::Host::current()->create_disk("Disk3", /*read bandwidth*/ 9.6e7, /*write bandwidth*/6.4e7)->seal(); /* - Display information on the disks mounted by the current host */ XBT_INFO("*** Storage info on %s ***", simgrid::s4u::Host::current()->get_cname()); diff --git a/include/simgrid/kernel/resource/Resource.hpp b/include/simgrid/kernel/resource/Resource.hpp index 526237ba40..731207b7ff 100644 --- a/include/simgrid/kernel/resource/Resource.hpp +++ b/include/simgrid/kernel/resource/Resource.hpp @@ -37,7 +37,7 @@ protected: profile::Event* state_event_ = nullptr; public: - Resource() = default; + Resource(const std::string& name) : name_(name){}; virtual ~Resource() = default; /** @brief Get the Model of the current Resource */ @@ -48,7 +48,6 @@ public: const std::string& get_name() const { return name_; } /** @brief Get the name of the current Resource */ const char* get_cname() const { return name_.c_str(); } - Resource* set_name(const std::string& name); /** @brief Get the lmm constraint associated to this Resource if it is part of a LMM component (or null if none) */ lmm::Constraint* get_constraint() const { return constraint_; } diff --git a/include/simgrid/s4u/Disk.hpp b/include/simgrid/s4u/Disk.hpp index 93a8e3bd9d..9d332e4d42 100644 --- a/include/simgrid/s4u/Disk.hpp +++ b/include/simgrid/s4u/Disk.hpp @@ -43,7 +43,7 @@ protected: public: #ifndef DOXYGEN - explicit Disk(kernel::resource::DiskImpl* pimpl) : pimpl_(pimpl) {} + explicit Disk(const std::string& name, kernel::resource::DiskImpl* pimpl) : pimpl_(pimpl), name_(name) {} #endif /** @brief Callback signal fired when a new Disk is created */ diff --git a/include/simgrid/s4u/Host.hpp b/include/simgrid/s4u/Host.hpp index 13e4544f84..6988a6ff9d 100644 --- a/include/simgrid/s4u/Host.hpp +++ b/include/simgrid/s4u/Host.hpp @@ -143,7 +143,7 @@ public: int get_pstate() const; std::vector get_disks() const; - Disk* create_disk(); + Disk* create_disk(const std::string& name, double read_bandwidth, double write_bandwidth); void add_disk(const Disk* disk); void remove_disk(const std::string& disk_name); diff --git a/src/kernel/resource/DiskImpl.hpp b/src/kernel/resource/DiskImpl.hpp index b130a9696f..60032910e5 100644 --- a/src/kernel/resource/DiskImpl.hpp +++ b/src/kernel/resource/DiskImpl.hpp @@ -41,7 +41,7 @@ public: DiskModel& operator=(const DiskModel&) = delete; ~DiskModel() override; - virtual DiskImpl* create_disk() = 0; + virtual DiskImpl* create_disk(const std::string& name, double read_bandwidth, double write_bandwidth) = 0; }; /************ @@ -59,7 +59,11 @@ protected: ~DiskImpl() override = default; // Disallow direct deletion. Call destroy() instead. public: - DiskImpl() : piface_(this){} + DiskImpl(const std::string& name, double read_bandwidth, double write_bandwidth) + : Resource(name), + piface_(name, this), + read_bw_(read_bandwidth), + write_bw_(write_bandwidth){} DiskImpl(const DiskImpl&) = delete; DiskImpl& operator=(const DiskImpl&) = delete; diff --git a/src/kernel/resource/Resource.cpp b/src/kernel/resource/Resource.cpp index 590db095d4..8df367cd98 100644 --- a/src/kernel/resource/Resource.cpp +++ b/src/kernel/resource/Resource.cpp @@ -13,12 +13,6 @@ namespace simgrid { namespace kernel { namespace resource { -Resource* Resource::set_name(const std::string& name) -{ - name_ = name; - return this; -} - Resource* Resource::set_model(Model* model) { model_ = model; diff --git a/src/kernel/resource/profile/Profile_test.cpp b/src/kernel/resource/profile/Profile_test.cpp index 283de0f333..949cbfdc74 100644 --- a/src/kernel/resource/profile/Profile_test.cpp +++ b/src/kernel/resource/profile/Profile_test.cpp @@ -24,7 +24,7 @@ class MockedResource : public simgrid::kernel::resource::Resource { public: static double the_date; - explicit MockedResource() { this->set_name("fake"); } + explicit MockedResource() : Resource("fake"){} void apply_event(simgrid::kernel::profile::Event* event, double value) override { XBT_VERB("t=%.1f: Change value to %lg (idx: %u)", the_date, value, event->idx); diff --git a/src/s4u/s4u_Host.cpp b/src/s4u/s4u_Host.cpp index 83b0a00283..0b4d103787 100644 --- a/src/s4u/s4u_Host.cpp +++ b/src/s4u/s4u_Host.cpp @@ -267,9 +267,9 @@ std::vector Host::get_disks() const return kernel::actor::simcall([this] { return this->pimpl_->get_disks(); }); } -Disk* Host::create_disk() +Disk* Host::create_disk(const std::string& name, double read_bandwidth, double write_bandwidth) { - auto pimpl = surf_disk_model->create_disk(); + auto pimpl = surf_disk_model->create_disk(name, read_bandwidth, write_bandwidth); pimpl->set_host(this); return pimpl->get_iface(); } diff --git a/src/surf/cpu_interface.cpp b/src/surf/cpu_interface.cpp index c8269441ef..3687e16c0b 100644 --- a/src/surf/cpu_interface.cpp +++ b/src/surf/cpu_interface.cpp @@ -58,11 +58,12 @@ Cpu::Cpu(Model* model, s4u::Host* host, const std::vector& speed_per_pst Cpu::Cpu(Model* model, s4u::Host* host, lmm::Constraint* constraint, const std::vector& speed_per_pstate, int core) - : core_count_(core) + : Resource(host->get_cname()) + , core_count_(core) , host_(host) , speed_per_pstate_(speed_per_pstate) { - this->set_name(host->get_cname())->set_model(model)->set_constraint(constraint); + this->set_model(model)->set_constraint(constraint); xbt_assert(core > 0, "Host %s must have at least one core, not 0.", host->get_cname()); diff --git a/src/surf/disk_s19.cpp b/src/surf/disk_s19.cpp index 3d114ccbbb..b2d57ff707 100644 --- a/src/surf/disk_s19.cpp +++ b/src/surf/disk_s19.cpp @@ -31,9 +31,9 @@ DiskS19Model::DiskS19Model() all_existing_models.push_back(this); } -DiskImpl* DiskS19Model::create_disk() +DiskImpl* DiskS19Model::create_disk(const std::string& name, double read_bandwidth, double write_bandwidth) { - return new DiskS19(); + return new DiskS19(name, read_bandwidth, write_bandwidth); } double DiskS19Model::next_occurring_event(double now) @@ -59,7 +59,6 @@ void DiskS19Model::update_actions_state(double /*now*/, double delta) /************ * Resource * ************/ - DiskAction* DiskS19::io_start(sg_size_t size, s4u::Io::OpType type) { return new DiskS19Action(get_model(), static_cast(size), not is_on(), this, type); diff --git a/src/surf/disk_s19.hpp b/src/surf/disk_s19.hpp index 50b2e3551e..feea129d56 100644 --- a/src/surf/disk_s19.hpp +++ b/src/surf/disk_s19.hpp @@ -29,7 +29,7 @@ class XBT_PRIVATE DiskS19Action; class DiskS19Model : public DiskModel { public: DiskS19Model(); - DiskImpl* create_disk() override; + DiskImpl* create_disk(const std::string& name, double read_bandwidth, double write_bandwidth) override; double next_occurring_event(double now) override; void update_actions_state(double now, double delta) override; }; @@ -40,7 +40,7 @@ public: class DiskS19 : public DiskImpl { public: - DiskS19() = default; + explicit DiskS19(const std::string& name, double read_bw, double write_bw): DiskImpl(name, read_bw, write_bw) {} DiskAction* io_start(sg_size_t size, s4u::Io::OpType type) override; DiskAction* read(sg_size_t size) override; DiskAction* write(sg_size_t size) override; diff --git a/src/surf/network_interface.cpp b/src/surf/network_interface.cpp index 15e75ad5ec..496383dd26 100644 --- a/src/surf/network_interface.cpp +++ b/src/surf/network_interface.cpp @@ -74,9 +74,9 @@ double NetworkModel::next_occurring_event_full(double now) ************/ LinkImpl::LinkImpl(NetworkModel* model, const std::string& name, lmm::Constraint* constraint) - : piface_(this) + : Resource(name), piface_(this) { - this->set_name(name)->set_model(model)->set_constraint(constraint); + this->set_model(model)->set_constraint(constraint); if (name != "__loopback__") xbt_assert(not s4u::Link::by_name_or_null(name), "Link '%s' declared several times in the platform.", name.c_str()); diff --git a/src/surf/sg_platf.cpp b/src/surf/sg_platf.cpp index 7781238942..2db602b584 100644 --- a/src/surf/sg_platf.cpp +++ b/src/surf/sg_platf.cpp @@ -324,19 +324,13 @@ void sg_platf_new_cabinet(const simgrid::kernel::routing::CabinetCreationArgs* c simgrid::kernel::resource::DiskImpl* sg_platf_new_disk(const simgrid::kernel::routing::DiskCreationArgs* disk) { - simgrid::kernel::resource::DiskImpl* pimpl = surf_disk_model->create_disk(); + simgrid::kernel::resource::DiskImpl* pimpl = surf_disk_model->create_disk(disk->id, disk->read_bw, disk->write_bw); - // This should be done using s4u::Disk methods and passed to the pimpl - pimpl->set_read_bandwidth(disk->read_bw) - ->set_write_bandwidth(disk->write_bw) - ->set_name(disk->id); if (disk->properties) { pimpl->set_properties(*disk->properties); delete disk->properties; } - pimpl->get_iface()->set_name(disk->id); - // This should be done in the seal() of a Disk creation pimpl->seal(); simgrid::s4u::Disk::on_creation(*pimpl->get_iface()); return pimpl; -- 2.20.1