From fea8b2e865eef9f33d094134cd0c54c8f8cb016f Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 10 Jan 2021 13:20:01 +0100 Subject: [PATCH] Unify on_start/on_completion signals of Activities --- ChangeLog | 1 + docs/source/Platform_Examples.rst | 3 +++ include/simgrid/s4u/Comm.hpp | 6 +++--- include/simgrid/s4u/Exec.hpp | 4 ++-- include/simgrid/s4u/Io.hpp | 5 ++++- src/instr/instr_platform.cpp | 17 +++++++++-------- src/kernel/activity/IoImpl.cpp | 9 --------- src/kernel/activity/IoImpl.hpp | 3 --- src/plugins/dirty_page_tracking.cpp | 4 ++-- src/plugins/host_dvfs.cpp | 4 ++-- src/plugins/host_energy.cpp | 2 +- src/plugins/host_load.cpp | 4 ++-- src/plugins/vm/VirtualMachineImpl.cpp | 4 ++-- src/s4u/s4u_Comm.cpp | 16 ++++++++-------- src/s4u/s4u_Exec.cpp | 8 ++++---- src/s4u/s4u_Io.cpp | 5 +++++ 16 files changed, 48 insertions(+), 47 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6b37281604..9897130eb6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -13,6 +13,7 @@ S4U: - Functions Mailbox::get() and Mailbox::get_async() are now templated with the type of the pointee. Untyped functions are deprecated. Use Mailbox::get() or Mailbox::get_async() if you really want to play with void*. +- Unify the interface of Activity::on_{start/activity} ---------------------------------------------------------------------------- diff --git a/docs/source/Platform_Examples.rst b/docs/source/Platform_Examples.rst index 09b34a1e1d..36592f7b36 100644 --- a/docs/source/Platform_Examples.rst +++ b/docs/source/Platform_Examples.rst @@ -153,6 +153,9 @@ Here is the meaning of this example: ``2 ; 4,4 ; 1,2 ; 1,2`` :language: xml :lines: 1-3,10- +.. todo: + + Model some other platforms, for example from https://link.springer.com/article/10.1007/s11227-019-03142-8 Dragonfly Cluster ----------------- diff --git a/include/simgrid/s4u/Comm.hpp b/include/simgrid/s4u/Comm.hpp index cc16d68d3c..d89f061c89 100644 --- a/include/simgrid/s4u/Comm.hpp +++ b/include/simgrid/s4u/Comm.hpp @@ -42,9 +42,9 @@ public: ~Comm() override; - static xbt::signal on_sender_start; - static xbt::signal on_receiver_start; - static xbt::signal on_completion; + static xbt::signal on_sender_start; + static xbt::signal on_receiver_start; + static xbt::signal on_completion; /*! take a vector s4u::CommPtr and return when one of them is finished. * The return value is the rank of the first finished CommPtr. */ diff --git a/include/simgrid/s4u/Exec.hpp b/include/simgrid/s4u/Exec.hpp index b15ed1b7d9..38289a8b0e 100644 --- a/include/simgrid/s4u/Exec.hpp +++ b/include/simgrid/s4u/Exec.hpp @@ -46,8 +46,8 @@ public: Exec& operator=(Exec const&) = delete; #endif - static xbt::signal on_start; - static xbt::signal on_completion; + static xbt::signal on_start; + static xbt::signal on_completion; Exec* start() override; /** @brief On sequential executions, returns the amount of flops that remain to be done; This cannot be used on diff --git a/include/simgrid/s4u/Io.hpp b/include/simgrid/s4u/Io.hpp index e979fb52e5..f51282b326 100644 --- a/include/simgrid/s4u/Io.hpp +++ b/include/simgrid/s4u/Io.hpp @@ -36,9 +36,12 @@ public: #ifndef DOXYGEN friend Disk; // Factory of IOs friend Storage; // Factory of IOs -#endif ~Io() override = default; +#endif + + static xbt::signal on_start; + static xbt::signal on_completion; Io* start() override; Io* wait() override; diff --git a/src/instr/instr_platform.cpp b/src/instr/instr_platform.cpp index 287d5315a6..e9bb5c4472 100644 --- a/src/instr/instr_platform.cpp +++ b/src/instr/instr_platform.cpp @@ -465,30 +465,31 @@ void define_callbacks() }); s4u::Actor::on_wake_up.connect( [](s4u::Actor const& actor) { Container::by_name(instr_pid(actor))->get_state("ACTOR_STATE")->pop_event(); }); - s4u::Exec::on_start.connect([](simgrid::s4u::Actor const& actor, s4u::Exec const&) { + s4u::Exec::on_start.connect([](s4u::Exec const&, simgrid::s4u::Actor const& actor) { Container::by_name(instr_pid(actor))->get_state("ACTOR_STATE")->push_event("execute"); }); - s4u::Exec::on_completion.connect([](s4u::Actor const& actor, s4u::Exec const&) { + s4u::Exec::on_completion.connect([](s4u::Exec const&, s4u::Actor const& actor) { Container::by_name(instr_pid(actor))->get_state("ACTOR_STATE")->pop_event(); }); - s4u::Comm::on_sender_start.connect([](s4u::Actor const& actor) { + s4u::Comm::on_sender_start.connect([](s4u::Comm const&, s4u::Actor const& actor) { Container::by_name(instr_pid(actor))->get_state("ACTOR_STATE")->push_event("send"); }); - s4u::Comm::on_receiver_start.connect([](s4u::Actor const& actor) { + s4u::Comm::on_receiver_start.connect([](s4u::Comm const&, s4u::Actor const& actor) { Container::by_name(instr_pid(actor))->get_state("ACTOR_STATE")->push_event("receive"); }); - s4u::Comm::on_completion.connect( - [](s4u::Actor const& actor) { Container::by_name(instr_pid(actor))->get_state("ACTOR_STATE")->pop_event(); }); + s4u::Comm::on_completion.connect([](s4u::Comm const&, s4u::Actor const& actor) { + Container::by_name(instr_pid(actor))->get_state("ACTOR_STATE")->pop_event(); + }); s4u::Actor::on_host_change.connect(on_actor_host_change); } if (TRACE_smpi_is_enabled() && TRACE_smpi_is_computing()) { - s4u::Exec::on_start.connect([](simgrid::s4u::Actor const& actor, s4u::Exec const& exec) { + s4u::Exec::on_start.connect([](s4u::Exec const& exec, simgrid::s4u::Actor const& actor) { Container::by_name(std::string("rank-") + std::to_string(actor.get_pid())) ->get_state("MPI_STATE") ->push_event("computing", new CpuTIData("compute", exec.get_cost())); }); - s4u::Exec::on_completion.connect([](s4u::Actor const& actor, s4u::Exec const&) { + s4u::Exec::on_completion.connect([](s4u::Exec const&, s4u::Actor const& actor) { Container::by_name(std::string("rank-") + std::to_string(actor.get_pid()))->get_state("MPI_STATE")->pop_event(); }); } diff --git a/src/kernel/activity/IoImpl.cpp b/src/kernel/activity/IoImpl.cpp index 72e4007085..c9a7430cf5 100644 --- a/src/kernel/activity/IoImpl.cpp +++ b/src/kernel/activity/IoImpl.cpp @@ -61,7 +61,6 @@ IoImpl* IoImpl::start() surf_action_->set_activity(this); XBT_DEBUG("Create IO synchro %p %s", this, get_cname()); - IoImpl::on_start(*this); return this; } @@ -86,8 +85,6 @@ void IoImpl::post() timeout_detector_ = nullptr; } - on_completion(*this); - /* Answer all simcalls associated with the synchro */ finish(); } @@ -122,12 +119,6 @@ void IoImpl::finish() } } -/************* - * Callbacks * - *************/ -xbt::signal IoImpl::on_start; -xbt::signal IoImpl::on_completion; - } // namespace activity } // namespace kernel } // namespace simgrid diff --git a/src/kernel/activity/IoImpl.hpp b/src/kernel/activity/IoImpl.hpp index 707372e5ea..df8bf13404 100644 --- a/src/kernel/activity/IoImpl.hpp +++ b/src/kernel/activity/IoImpl.hpp @@ -36,9 +36,6 @@ public: IoImpl* start(); void post() override; void finish() override; - - static xbt::signal on_start; - static xbt::signal on_completion; }; } // namespace activity } // namespace kernel diff --git a/src/plugins/dirty_page_tracking.cpp b/src/plugins/dirty_page_tracking.cpp index db04237068..9bed1db87d 100644 --- a/src/plugins/dirty_page_tracking.cpp +++ b/src/plugins/dirty_page_tracking.cpp @@ -73,7 +73,7 @@ static void on_virtual_machine_creation(simgrid::vm::VirtualMachineImpl& vm) vm.extension_set(new simgrid::vm::DirtyPageTrackingExt()); } -static void on_exec_creation(simgrid::s4u::Actor const&, simgrid::s4u::Exec const& e) +static void on_exec_creation(simgrid::s4u::Exec const& e, simgrid::s4u::Actor const&) { auto exec = static_cast(e.get_impl()); const simgrid::s4u::VirtualMachine* vm = dynamic_cast(exec->get_host()); @@ -87,7 +87,7 @@ static void on_exec_creation(simgrid::s4u::Actor const&, simgrid::s4u::Exec cons } } -static void on_exec_completion(simgrid::s4u::Actor const&, simgrid::s4u::Exec const& e) +static void on_exec_completion(simgrid::s4u::Exec const& e, simgrid::s4u::Actor const&) { auto exec = static_cast(e.get_impl()); const simgrid::s4u::VirtualMachine* vm = dynamic_cast(exec->get_host()); diff --git a/src/plugins/host_dvfs.cpp b/src/plugins/host_dvfs.cpp index 4c203c2d02..edd21d2459 100644 --- a/src/plugins/host_dvfs.cpp +++ b/src/plugins/host_dvfs.cpp @@ -295,11 +295,11 @@ public: task_id = 0; } }); - simgrid::s4u::Exec::on_start.connect([this](simgrid::s4u::Actor const&, simgrid::s4u::Exec const& activity) { + simgrid::s4u::Exec::on_start.connect([this](simgrid::s4u::Exec const& activity, simgrid::s4u::Actor const&) { if (activity.get_host() == get_host()) pre_task(); }); - simgrid::s4u::Exec::on_completion.connect([this](simgrid::s4u::Actor const&, simgrid::s4u::Exec const& activity) { + simgrid::s4u::Exec::on_completion.connect([this](simgrid::s4u::Exec const& activity, simgrid::s4u::Actor const&) { // For more than one host (not yet supported), we can access the host via // simcalls_.front()->issuer->get_iface()->get_host() if (activity.get_host() == get_host() && iteration_running) { diff --git a/src/plugins/host_energy.cpp b/src/plugins/host_energy.cpp index 1fe704348c..06086cea63 100644 --- a/src/plugins/host_energy.cpp +++ b/src/plugins/host_energy.cpp @@ -551,7 +551,7 @@ void sg_host_energy_plugin_init() // that the next trigger would be the 2nd compute, hence ignoring the idle time // during the recv call. By updating at the beginning of a compute, we can // fix that. (If the cpu is not idle, this is not required.) - simgrid::s4u::Exec::on_start.connect([](simgrid::s4u::Actor const&, simgrid::s4u::Exec const& activity) { + simgrid::s4u::Exec::on_start.connect([](simgrid::s4u::Exec const& activity, simgrid::s4u::Actor const&) { if (activity.get_host_number() == 1) { // We only run on one host simgrid::s4u::Host* host = activity.get_host(); const simgrid::s4u::VirtualMachine* vm = dynamic_cast(host); diff --git a/src/plugins/host_load.cpp b/src/plugins/host_load.cpp index c538bf335d..4011e9871b 100644 --- a/src/plugins/host_load.cpp +++ b/src/plugins/host_load.cpp @@ -231,7 +231,7 @@ void sg_host_load_plugin_init() host.extension_set(new HostLoad(&host)); }); - simgrid::s4u::Exec::on_start.connect([](simgrid::s4u::Actor const&, simgrid::s4u::Exec const& activity) { + simgrid::s4u::Exec::on_start.connect([](simgrid::s4u::Exec const& activity, simgrid::s4u::Actor const&) { if (activity.get_host_number() == 1) { // We only run on one host simgrid::s4u::Host* host = activity.get_host(); const simgrid::s4u::VirtualMachine* vm = dynamic_cast(host); @@ -247,7 +247,7 @@ void sg_host_load_plugin_init() XBT_WARN("HostLoad plugin currently does not support executions on several hosts"); } }); - simgrid::s4u::Exec::on_completion.connect([](simgrid::s4u::Actor const&, simgrid::s4u::Exec const& activity) { + simgrid::s4u::Exec::on_completion.connect([](simgrid::s4u::Exec const& activity, simgrid::s4u::Actor const&) { if (activity.get_host_number() == 1) { // We only run on one host simgrid::s4u::Host* host = activity.get_host(); const simgrid::s4u::VirtualMachine* vm = dynamic_cast(host); diff --git a/src/plugins/vm/VirtualMachineImpl.cpp b/src/plugins/vm/VirtualMachineImpl.cpp index b95cc55909..640d91d6a6 100644 --- a/src/plugins/vm/VirtualMachineImpl.cpp +++ b/src/plugins/vm/VirtualMachineImpl.cpp @@ -55,7 +55,7 @@ static void host_state_change(s4u::Host const& host) } } -static void add_active_exec(s4u::Actor const&, s4u::Exec const& task) +static void add_active_exec(s4u::Exec const& task, s4u::Actor const&) { const s4u::VirtualMachine* vm = dynamic_cast(task.get_host()); if (vm != nullptr) { @@ -65,7 +65,7 @@ static void add_active_exec(s4u::Actor const&, s4u::Exec const& task) } } -static void remove_active_exec(s4u::Actor const&, s4u::Exec const& task) +static void remove_active_exec(s4u::Exec const& task, s4u::Actor const&) { const s4u::VirtualMachine* vm = dynamic_cast(task.get_host()); if (vm != nullptr) { diff --git a/src/s4u/s4u_Comm.cpp b/src/s4u/s4u_Comm.cpp index 8b37c10376..7f77ab4238 100644 --- a/src/s4u/s4u_Comm.cpp +++ b/src/s4u/s4u_Comm.cpp @@ -16,9 +16,9 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(s4u_comm, s4u_activity, "S4U asynchronous commun namespace simgrid { namespace s4u { -xbt::signal Comm::on_sender_start; -xbt::signal Comm::on_receiver_start; -xbt::signal Comm::on_completion; +xbt::signal Comm::on_sender_start; +xbt::signal Comm::on_receiver_start; +xbt::signal Comm::on_completion; Comm::~Comm() { @@ -119,12 +119,12 @@ Comm* Comm::start() "You cannot use %s() once your communication started (not implemented)", __FUNCTION__); if (src_buff_ != nullptr) { // Sender side - on_sender_start(*Actor::self()); + on_sender_start(*this, *Actor::self()); pimpl_ = simcall_comm_isend(sender_, mailbox_->get_impl(), remains_, rate_, src_buff_, src_buff_size_, match_fun_, clean_fun_, copy_data_function_, get_user_data(), detached_); } else if (dst_buff_ != nullptr) { // Receiver side xbt_assert(not detached_, "Receive cannot be detached"); - on_receiver_start(*Actor::self()); + on_receiver_start(*this, *Actor::self()); pimpl_ = simcall_comm_irecv(receiver_, mailbox_->get_impl(), dst_buff_, &dst_buff_size_, match_fun_, copy_data_function_, get_user_data(), rate_); @@ -160,12 +160,12 @@ Comm* Comm::wait_for(double timeout) case State::INITED: case State::STARTING: // It's not started yet. Do it in one simcall if (src_buff_ != nullptr) { - on_sender_start(*Actor::self()); + on_sender_start(*this, *Actor::self()); simcall_comm_send(sender_, mailbox_->get_impl(), remains_, rate_, src_buff_, src_buff_size_, match_fun_, copy_data_function_, get_user_data(), timeout); } else { // Receiver - on_receiver_start(*Actor::self()); + on_receiver_start(*this, *Actor::self()); simcall_comm_recv(receiver_, mailbox_->get_impl(), dst_buff_, &dst_buff_size_, match_fun_, copy_data_function_, get_user_data(), timeout, rate_); } @@ -185,7 +185,7 @@ Comm* Comm::wait_for(double timeout) default: THROW_IMPOSSIBLE; } - on_completion(*Actor::self()); + on_completion(*this, *Actor::self()); return this; } diff --git a/src/s4u/s4u_Exec.cpp b/src/s4u/s4u_Exec.cpp index 67a5caf95c..704506ae16 100644 --- a/src/s4u/s4u_Exec.cpp +++ b/src/s4u/s4u_Exec.cpp @@ -14,8 +14,8 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(s4u_exec, s4u_activity, "S4U asynchronous execut namespace simgrid { namespace s4u { -xbt::signal Exec::on_start; -xbt::signal Exec::on_completion; +xbt::signal Exec::on_start; +xbt::signal Exec::on_completion; Exec::Exec() { @@ -35,7 +35,7 @@ Exec* Exec::wait_for(double timeout) kernel::actor::ActorImpl* issuer = Actor::self()->get_impl(); kernel::actor::simcall_blocking([this, issuer, timeout] { this->get_impl()->wait_for(issuer, timeout); }); state_ = State::FINISHED; - on_completion(*Actor::self(), *this); + on_completion(*this, *Actor::self()); this->release_dependencies(); return this; } @@ -185,7 +185,7 @@ Exec* Exec::start() pimpl_->suspend(); state_ = State::STARTED; - on_start(*Actor::self(), *this); + on_start(*this, *Actor::self()); return this; } diff --git a/src/s4u/s4u_Io.cpp b/src/s4u/s4u_Io.cpp index 4831daf91f..acd9a0df74 100644 --- a/src/s4u/s4u_Io.cpp +++ b/src/s4u/s4u_Io.cpp @@ -12,6 +12,8 @@ namespace simgrid { namespace s4u { +xbt::signal Io::on_start; +xbt::signal Io::on_completion; Io::Io(sg_disk_t disk, sg_size_t size, OpType type) : disk_(disk), size_(size), type_(type) { @@ -49,6 +51,7 @@ Io* Io::start() pimpl_->suspend(); state_ = State::STARTED; + on_start(*this, *Actor::self()); return this; } @@ -73,6 +76,8 @@ Io* Io::wait_for(double timeout) kernel::actor::simcall_blocking([this, issuer, timeout] { this->get_impl()->wait_for(issuer, timeout); }); state_ = State::FINISHED; this->release_dependencies(); + + on_completion(*this, *Actor::self()); return this; } -- 2.20.1