Various cleanups in the Host signals

- Add a on_this_*_cb variant to the Host signals
- Rename Activity::on_{suspended,resumed} for symmetry with Actor signals
- Change kernel::activity::CpuAction::on_state_change to s4u::Host::on_exec_state_change

diff --git a/ChangeLog b/ChangeLog
index 20dfd23..e898652 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -24,6 +24,8 @@ S4U:
    and an instance version that is invoked for this specific object only. For example, 
    s4u::Actor::on_suspend_cb() adds a callback that is invoked for the suspend of any actor while
    s4u::Actor::on_this_suspend_cb() adds a callback for this specific actor only.
+ - Activity::on_suspended_cb() is renamed to Activity::on_suspend_cb(), and fired right before the suspend.
+ - Activity::on_resumed_cb() is renamed to Activity::on_resume_cb(), and fired right before the resume.
 
 New S4U plugins:
  - Operation: They are designed to represent workflows, i.e, graphs of repeatable Activities.

diff --git a/docs/source/Plugins.rst b/docs/source/Plugins.rst
index b3678c4..073c904 100644 (file)
--- a/docs/source/Plugins.rst
+++ b/docs/source/Plugins.rst
@@ -86,8 +86,8 @@ Partial list of existing signals in s4u:
 - :cpp:func:`Comm::on_send <simgrid::s4u::Comm::on_send_cb>`
   :cpp:func:`Comm::on_recv <simgrid::s4u::Comm::on_recv_cb>`
   :cpp:func:`Comm::on_completion <simgrid::s4u::Comm::on_completion_cb>`
-- :cpp:func:`CommImpl::on_start <simgrid::s4u::Comm::on_start_cb>`
-  :cpp:func:`CommImpl::on_completion <simgrid::s4u::Comm::on_completion_cb>`
+- :cpp:func:`CommImpl::on_start <simgrid::s4u::CommImpl::on_start_cb>`
+  :cpp:func:`CommImpl::on_completion <simgrid::s4u::CommImpl::on_completion_cb>`
 - :cpp:func:`Disk::on_creation <simgrid::s4u::Disk::on_creation_cb>`
   :cpp:func:`Disk::on_destruction <simgrid::s4u::Disk::on_destruction_cb>`
   :cpp:func:`Disk::on_this_destruction <simgrid::s4u::Disk::on_this_destruction_cb>`
@@ -102,8 +102,11 @@ Partial list of existing signals in s4u:
   :cpp:func:`Exec::on_completion <simgrid::s4u::Exec::on_completion_cb>`
 - :cpp:func:`Host::on_creation <simgrid::s4u::Host::on_creation_cb>`
   :cpp:func:`Host::on_destruction <simgrid::s4u::Host::on_destruction_cb>`
+  :cpp:func:`Host::on_this_destruction <simgrid::s4u::Host::on_this_destruction_cb>`
   :cpp:func:`Host::on_state_change <simgrid::s4u::Host::on_state_change_cb>`
+  :cpp:func:`Host::on_this_state_change <simgrid::s4u::Host::on_this_state_change_cb>`
   :cpp:func:`Host::on_speed_change <simgrid::s4u::Host::on_speed_change_cb>`
+  :cpp:func:`Host::on_this_speed_change <simgrid::s4u::Host::on_this_speed_change_cb>`
 - :cpp:func:`Io::on_start <simgrid::s4u::Io::on_start_cb>`
   :cpp:func:`Io::on_completion <simgrid::s4u::Io::on_completion_cb>`
 - :cpp:func:`Link::on_creation <simgrid::s4u::Link::on_creation_cb>`

diff --git a/docs/source/app_s4u.rst b/docs/source/app_s4u.rst
index b5b7d66..05ed9cd 100644 (file)
--- a/docs/source/app_s4u.rst
+++ b/docs/source/app_s4u.rst
@@ -1497,8 +1497,11 @@ Signals
 
       .. doxygenfunction:: simgrid::s4u::Host::on_creation_cb
       .. doxygenfunction:: simgrid::s4u::Host::on_destruction_cb
+      .. doxygenfunction:: simgrid::s4u::Host::on_this_destruction_cb
       .. doxygenfunction:: simgrid::s4u::Host::on_speed_change_cb
+      .. doxygenfunction:: simgrid::s4u::Host::on_this_speed_change_cb
       .. doxygenfunction:: simgrid::s4u::Host::on_state_change_cb
+      .. doxygenfunction:: simgrid::s4u::Host::on_this_state_change_cb
 
 .. _API_s4u_Link:
 

diff --git a/include/simgrid/forward.h b/include/simgrid/forward.h
index dae9277..d21a7fa 100644 (file)
--- a/include/simgrid/forward.h
+++ b/include/simgrid/forward.h
@@ -178,6 +178,7 @@ class System;
 }
 namespace resource {
 class Action;
+class CpuAction;
 class CpuImpl;
 class Model;
 class Resource;

diff --git a/include/simgrid/s4u/Activity.hpp b/include/simgrid/s4u/Activity.hpp
index 82a880a..83f929d 100644 (file)
--- a/include/simgrid/s4u/Activity.hpp
+++ b/include/simgrid/s4u/Activity.hpp
@@ -105,8 +105,8 @@ protected:
 private:
   static xbt::signal<void(Activity&)> on_veto;
   static xbt::signal<void(Activity const&)> on_completion;
-  static xbt::signal<void(Activity const&)> on_suspended;
-  static xbt::signal<void(Activity const&)> on_resumed;
+  static xbt::signal<void(Activity const&)> on_suspend;
+  static xbt::signal<void(Activity const&)> on_resume;
 
 public:
   /*! Add a callback fired each time that the activity fails to start because of a veto (e.g., unsolved dependency or no
@@ -115,9 +115,26 @@ public:
   /*! Add a callback fired when the activity completes (either normally, cancelled or failed) */
   static void on_completion_cb(const std::function<void(Activity const&)>& cb) { on_completion.connect(cb); }
   /*! Add a callback fired when the activity is suspended */
-  static void on_suspended_cb(const std::function<void(Activity const&)>& cb) { on_suspended.connect(cb); }
+  static void on_suspend_cb(const std::function<void(Activity const&)>& cb)
+  {
+    on_suspend.connect(cb);
+  }
   /*! Add a callback fired when the activity is resumed after being suspended */
-  static void on_resumed_cb(const std::function<void(Activity const&)>& cb) { on_resumed.connect(cb); }
+  static void on_resume_cb(const std::function<void(Activity const&)>& cb)
+  {
+    on_resume.connect(cb);
+  }
+
+  XBT_ATTRIB_DEPRECATED_v334("Please use on_suspend_cb() instead") static void on_suspended_cb(
+      const std::function<void(Activity const&)>& cb)
+  {
+    on_suspend.connect(cb);
+  }
+  XBT_ATTRIB_DEPRECATED_v334("Please use on_resume_cb() instead") static void on_resumed_cb(
+      const std::function<void(Activity const&)>& cb)
+  {
+    on_resume.connect(cb);
+  }
 
   XBT_ATTRIB_DEPRECATED_v334("All start() are vetoable now. Please use start() ") void vetoable_start()
   {

diff --git a/include/simgrid/s4u/Actor.hpp b/include/simgrid/s4u/Actor.hpp
index da3f82e..e4aca9d 100644 (file)
--- a/include/simgrid/s4u/Actor.hpp
+++ b/include/simgrid/s4u/Actor.hpp
@@ -232,13 +232,13 @@ private:
 public:
   /** Add a callback fired when a new actor has been created **/
   static void on_creation_cb(const std::function<void(Actor&)>& cb) { on_creation.connect(cb); }
-  /** Add a callback fired when any actor is suspended**/
+  /** Add a callback fired when any actor is suspended (right before the suspend) **/
   static void on_suspend_cb(const std::function<void(Actor const&)>& cb) { on_suspend.connect(cb); }
-  /** Add a callback fired when this specific actor is suspended**/
+  /** Add a callback fired when this specific actor is suspended (right before the suspend) **/
   void on_this_suspend_cb(const std::function<void(Actor const&)>& cb) { on_this_suspend.connect(cb); }
-  /** Add a callback fired when any actor is resumed **/
+  /** Add a callback fired when any actor is resumed (right before the resume) **/
   static void on_resume_cb(const std::function<void(Actor const&)>& cb) { on_resume.connect(cb); }
-  /** Add a callback fired when any actor is resumed **/
+  /** Add a callback fired when any actor is resumed (right before the resume) **/
   void on_this_resume_cb(const std::function<void(Actor const&)>& cb) { on_this_resume.connect(cb); }
   /** Add a callback fired when any actor starts sleeping **/
   static void on_sleep_cb(const std::function<void(Actor const&)>& cb) { on_sleep.connect(cb); }

diff --git a/include/simgrid/s4u/Host.hpp b/include/simgrid/s4u/Host.hpp
index a06ef84..dacce60 100644 (file)
--- a/include/simgrid/s4u/Host.hpp
+++ b/include/simgrid/s4u/Host.hpp
@@ -7,6 +7,7 @@
 #define SIMGRID_S4U_HOST_HPP
 
 #include <simgrid/forward.h>
+#include <simgrid/kernel/resource/Action.hpp>
 #include <xbt/Extendable.hpp>
 #include <xbt/signal.hpp>
 
@@ -46,6 +47,9 @@ class XBT_PUBLIC Host : public xbt::Extendable<Host> {
 
   kernel::resource::CpuImpl* pimpl_cpu_      = nullptr;
   kernel::routing::NetPoint* pimpl_netpoint_ = nullptr;
+#ifndef DOXYGEN
+  friend kernel::resource::CpuAction; // signal exec_state_changed
+#endif
 
 public:
   explicit Host(kernel::resource::HostImpl* pimpl) : pimpl_(pimpl) {}
@@ -56,20 +60,47 @@ protected:
 
   static xbt::signal<void(Host&)> on_creation;
   static xbt::signal<void(Host const&)> on_destruction;
+  xbt::signal<void(Host const&)> on_this_destruction;
+  static xbt::signal<void(kernel::resource::CpuAction&, kernel::resource::Action::State)> on_exec_state_change;
 
 public:
   static xbt::signal<void(Host const&)> on_speed_change;
+  xbt::signal<void(Host const&)> on_this_speed_change;
   static xbt::signal<void(Host const&)> on_state_change;
+  xbt::signal<void(Host const&)> on_this_state_change;
+
 #endif
   /** Add a callback fired on each newly created host */
   static void on_creation_cb(const std::function<void(Host&)>& cb) { on_creation.connect(cb); }
-  /** Add a callback fired when the machine is turned on or off (called AFTER the change) */
+  /** Add a callback fired when any machine is turned on or off (called AFTER the change) */
   static void on_state_change_cb(const std::function<void(Host const&)>& cb) { on_state_change.connect(cb); }
-  /** Add a callback fired when the speed of the machine is changed (called AFTER the change)
+  /** Add a callback fired when this specific machine is turned on or off (called AFTER the change) */
+  void on_this_state_change_cb(const std::function<void(Host const&)>& cb)
+  {
+    on_this_state_change.connect(cb);
+  }
+  /** Add a callback fired when the speed of any 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 void on_speed_change_cb(const std::function<void(Host const&)>& cb) { on_speed_change.connect(cb); }
-  /** Add a callback fired just before destructing a host */
+  /** Add a callback fired when the speed of this specific machine is changed (called AFTER the change)
+   * (either because of a pstate switch or because of an external load event coming from the profile) */
+  void on_this_speed_change_cb(const std::function<void(Host const&)>& cb)
+  {
+    on_this_speed_change.connect(cb);
+  }
+  /** Add a callback fired just before destructing any host */
   static void on_destruction_cb(const std::function<void(Host const&)>& cb) { on_destruction.connect(cb); }
+  /** Add a callback fired just before destructing this specific host */
+  void on_this_destruction_cb(const std::function<void(Host const&)>& cb)
+  {
+    on_this_destruction.connect(cb);
+  }
+  /** Add a callback fired when the state of any exec activity changes */
+  static void on_exec_state_change_cb(
+      const std::function<void(kernel::resource::CpuAction&, kernel::resource::Action::State previous)>& cb)
+  {
+    on_exec_state_change.connect(cb);
+  }
 
   virtual void destroy();
 #ifndef DOXYGEN

diff --git a/src/instr/instr_platform.cpp b/src/instr/instr_platform.cpp
index 694a04a..4a92977 100644 (file)
--- a/src/instr/instr_platform.cpp
+++ b/src/instr/instr_platform.cpp
@@ -14,6 +14,7 @@
 #include <xbt/graph.h>
 
 #include "src/instr/instr_private.hpp"
+#include "src/kernel/activity/ExecImpl.hpp"
 #include "src/kernel/resource/CpuImpl.hpp"
 #include "src/kernel/resource/NetworkModel.hpp"
 
@@ -362,12 +363,17 @@ static void on_action_state_change(kernel::resource::Action const& action,
       resource_set_utilization("HOST", "speed_used", cpu->get_cname(), action.get_category(), value,
                                action.get_last_update(), simgrid_get_clock() - action.get_last_update());
 
-    if (const auto* link = dynamic_cast<kernel::resource::StandardLinkImpl*>(resource))
+    else if (const auto* link = dynamic_cast<kernel::resource::StandardLinkImpl*>(resource))
       resource_set_utilization("LINK", "bandwidth_used", link->get_cname(), action.get_category(), value,
                                action.get_last_update(), simgrid_get_clock() - action.get_last_update());
   }
 }
 
+static void on_activity_suspend_resume(s4u::Activity const& activity)
+{
+  on_action_state_change(*activity.get_impl()->model_action_, /*ignored*/ kernel::resource::Action::State::STARTED);
+}
+
 static void on_platform_created()
 {
   currentContainer.clear();
@@ -462,8 +468,10 @@ void define_callbacks()
 
   s4u::NetZone::on_creation_cb(on_netzone_creation);
 
-  kernel::resource::CpuAction::on_state_change.connect(on_action_state_change);
+  s4u::Host::on_exec_state_change_cb(on_action_state_change);
   s4u::Link::on_communication_state_change_cb(on_action_state_change);
+  s4u::Activity::on_suspend_cb(on_activity_suspend_resume);
+  s4u::Activity::on_resume_cb(on_activity_suspend_resume);
 
   if (TRACE_actor_is_enabled()) {
     s4u::Actor::on_creation_cb(on_actor_creation);

diff --git a/src/kernel/activity/ActivityImpl.cpp b/src/kernel/activity/ActivityImpl.cpp
index f425dc8..aaaba06 100644 (file)
--- a/src/kernel/activity/ActivityImpl.cpp
+++ b/src/kernel/activity/ActivityImpl.cpp
@@ -176,11 +176,13 @@ void ActivityImpl::wait_any_for(actor::ActorImpl* issuer, const std::vector<Acti
 
 void ActivityImpl::suspend()
 {
-  if (model_action_ == nullptr)
+  if (model_action_ == nullptr) {
+    XBT_CRITICAL("POUET");
     return;
+  }
   XBT_VERB("This activity is suspended (remain: %f)", model_action_->get_remains());
+  s4u::Activity::on_suspend(*get_iface());
   model_action_->suspend();
-  s4u::Activity::on_suspended(*get_iface());
 }
 
 void ActivityImpl::resume()
@@ -188,8 +190,8 @@ void ActivityImpl::resume()
   if (model_action_ == nullptr)
     return;
   XBT_VERB("This activity is resumed (remain: %f)", model_action_->get_remains());
+  s4u::Activity::on_resume(*get_iface());
   model_action_->resume();
-  s4u::Activity::on_resumed(*get_iface());
 }
 
 void ActivityImpl::cancel()

diff --git a/src/kernel/resource/CpuImpl.cpp b/src/kernel/resource/CpuImpl.cpp
index d597356..313b3fc 100644 (file)
--- a/src/kernel/resource/CpuImpl.cpp
+++ b/src/kernel/resource/CpuImpl.cpp
@@ -98,6 +98,7 @@ double CpuImpl::get_pstate_peak_speed(unsigned long pstate_index) const
 void CpuImpl::on_speed_change()
 {
   s4u::Host::on_speed_change(*piface_);
+  piface_->on_this_speed_change(*piface_);
 }
 
 CpuImpl* CpuImpl::set_core_count(int core_count)
@@ -193,27 +194,11 @@ void CpuAction::update_remains_lazy(double now)
   set_last_value(get_rate());
 }
 
-xbt::signal<void(CpuAction const&, Action::State)> CpuAction::on_state_change;
-
-void CpuAction::suspend()
-{
-  Action::State previous = get_state();
-  on_state_change(*this, previous);
-  Action::suspend();
-}
-
-void CpuAction::resume()
-{
-  Action::State previous = get_state();
-  on_state_change(*this, previous);
-  Action::resume();
-}
-
 void CpuAction::set_state(Action::State state)
 {
   Action::State previous = get_state();
   Action::set_state(state);
-  on_state_change(*this, previous);
+  s4u::Host::on_exec_state_change(*this, previous);
 }
 
 /** @brief returns a list of all CPUs that this action is using */

diff --git a/src/kernel/resource/CpuImpl.hpp b/src/kernel/resource/CpuImpl.hpp
index 6237702..ae35bd5 100644 (file)
--- a/src/kernel/resource/CpuImpl.hpp
+++ b/src/kernel/resource/CpuImpl.hpp
@@ -180,18 +180,10 @@ class XBT_PUBLIC CpuAction : public Action {
 public:
   using Action::Action;
 
-  /** @brief Signal emitted when the action state changes (ready/running/done, etc)
-   *  Signature: `void(CpuAction const& action, simgrid::kernel::resource::Action::State previous)`
-   */
-  static xbt::signal<void(CpuAction const&, Action::State)> on_state_change;
-
   void set_state(Action::State state) override;
 
   void update_remains_lazy(double now) override;
   std::list<CpuImpl*> cpus() const;
-
-  void suspend() override;
-  void resume() override;
 };
 } // namespace simgrid::kernel::resource
 

diff --git a/src/kernel/resource/HostImpl.cpp b/src/kernel/resource/HostImpl.cpp
index 4dd7089..e72375a 100644 (file)
--- a/src/kernel/resource/HostImpl.cpp
+++ b/src/kernel/resource/HostImpl.cpp
@@ -67,6 +67,7 @@ HostImpl::~HostImpl()
 void HostImpl::destroy()
 {
   s4u::Host::on_destruction(*this->get_iface());
+  this->get_iface()->on_this_destruction(*this->get_iface());
   delete this;
 }
 

diff --git a/src/kernel/resource/VirtualMachineImpl.cpp b/src/kernel/resource/VirtualMachineImpl.cpp
index 04f8ada..ea2fa1c 100644 (file)
--- a/src/kernel/resource/VirtualMachineImpl.cpp
+++ b/src/kernel/resource/VirtualMachineImpl.cpp
@@ -126,8 +126,8 @@ VMModel::VMModel(const std::string& name) : HostModel(name)
   s4u::Host::on_state_change_cb(host_state_change);
   s4u::Exec::on_start_cb(add_active_exec);
   s4u::Activity::on_completion_cb(remove_active_exec);
-  s4u::Activity::on_resumed_cb(add_active_activity);
-  s4u::Activity::on_suspended_cb(remove_active_activity);
+  s4u::Activity::on_resume_cb(add_active_activity);
+  s4u::Activity::on_suspend_cb(remove_active_activity);
 }
 
 double VMModel::next_occurring_event(double now)

diff --git a/src/plugins/host_energy.cpp b/src/plugins/host_energy.cpp
index 36e1044..5f6a0e4 100644 (file)
--- a/src/plugins/host_energy.cpp
+++ b/src/plugins/host_energy.cpp
@@ -11,6 +11,7 @@
 #include <simgrid/s4u/VirtualMachine.hpp>
 #include <simgrid/simix.hpp>
 
+#include "src/kernel/activity/ActivityImpl.hpp"
 #include "src/kernel/resource/CpuImpl.hpp"
 #include "src/simgrid/module.hpp"
 
@@ -438,14 +439,13 @@ static void on_action_state_change(simgrid::kernel::resource::CpuAction const& a
 
 /* This callback is fired either when the host changes its state (on/off) ("onStateChange") or its speed
  * (because the user changed the pstate, or because of external trace events) ("onSpeedChange") */
-static void on_host_change(simgrid::s4u::Host const& host)
+static void on_host_change(simgrid::s4u::Host const& h)
 {
-  if (dynamic_cast<simgrid::s4u::VirtualMachine const*>(&host)) // Ignore virtual machines
-    return;
-
-  auto* host_energy = host.extension<HostEnergy>();
+  auto* host = &h;
+  if (const auto* vm = dynamic_cast<simgrid::s4u::VirtualMachine const*>(host)) // Take the PM of virtual machines
+    host = vm->get_pm();
 
-  host_energy->update();
+  host->extension<HostEnergy>()->update();
 }
 
 static void on_host_destruction(simgrid::s4u::Host const& host)
@@ -473,6 +473,13 @@ static void on_simulation_end()
            used_hosts_energy, total_energy - used_hosts_energy);
 }
 
+static void on_activity_suspend_resume(simgrid::s4u::Activity const& activity)
+{
+  if (auto* action = dynamic_cast<simgrid::kernel::resource::CpuAction*>(activity.get_impl()->model_action_);
+      action != nullptr)
+    on_action_state_change(*action, /*ignored*/ action->get_state());
+}
+
 /* **************************** Public interface *************************** */
 
 /** @ingroup plugin_host_energy
@@ -490,8 +497,12 @@ void sg_host_energy_plugin_init()
   simgrid::s4u::Host::on_state_change_cb(&on_host_change);
   simgrid::s4u::Host::on_speed_change_cb(&on_host_change);
   simgrid::s4u::Host::on_destruction_cb(&on_host_destruction);
+  simgrid::s4u::Host::on_exec_state_change_cb(&on_action_state_change);
+  simgrid::s4u::VirtualMachine::on_suspend_cb(&on_host_change);
+  simgrid::s4u::VirtualMachine::on_resume_cb(&on_host_change);
+  simgrid::s4u::Exec::on_suspend_cb(on_activity_suspend_resume);
+  simgrid::s4u::Exec::on_resume_cb(on_activity_suspend_resume);
   simgrid::s4u::Engine::on_simulation_end_cb(&on_simulation_end);
-  simgrid::kernel::resource::CpuAction::on_state_change.connect(&on_action_state_change);
   // We may only have one actor on a node. If that actor executes something like
   //   compute -> recv -> compute
   // the recv operation will not trigger a "CpuAction::on_state_change". This means
@@ -500,10 +511,9 @@ void sg_host_energy_plugin_init()
   // fix that. (If the cpu is not idle, this is not required.)
   simgrid::s4u::Exec::on_start_cb([](simgrid::s4u::Exec const& activity) {
     if (activity.get_host_number() == 1) { // We only run on one host
-      simgrid::s4u::Host* host         = activity.get_host();
+      simgrid::s4u::Host* host = activity.get_host();
       if (const auto* vm = dynamic_cast<simgrid::s4u::VirtualMachine*>(host))
         host = vm->get_pm();
-      xbt_assert(host != nullptr);
       host->extension<HostEnergy>()->update();
     }
   });

diff --git a/src/s4u/s4u_Activity.cpp b/src/s4u/s4u_Activity.cpp
index 7b38f61..90ddcf7 100644 (file)
--- a/src/s4u/s4u_Activity.cpp
+++ b/src/s4u/s4u_Activity.cpp
@@ -26,8 +26,8 @@ namespace s4u {
 
 xbt::signal<void(Activity&)> Activity::on_veto;
 xbt::signal<void(Activity const&)> Activity::on_completion;
-xbt::signal<void(Activity const&)> Activity::on_suspended;
-xbt::signal<void(Activity const&)> Activity::on_resumed;
+xbt::signal<void(Activity const&)> Activity::on_suspend;
+xbt::signal<void(Activity const&)> Activity::on_resume;
 
 std::set<Activity*>* Activity::vetoed_activities_ = nullptr;
 

diff --git a/src/s4u/s4u_Host.cpp b/src/s4u/s4u_Host.cpp
index 7aa931a..1e301fe 100644 (file)
--- a/src/s4u/s4u_Host.cpp
+++ b/src/s4u/s4u_Host.cpp
@@ -34,6 +34,7 @@ xbt::signal<void(Host&)> Host::on_creation;
 xbt::signal<void(Host const&)> Host::on_destruction;
 xbt::signal<void(Host const&)> Host::on_state_change;
 xbt::signal<void(Host const&)> Host::on_speed_change;
+xbt::signal<void(kernel::resource::CpuAction&, kernel::resource::Action::State)> Host::on_exec_state_change;
 #endif
 
 Host* Host::set_cpu(kernel::resource::CpuImpl* cpu)