privatize ExecImpl::timeoutDetector and partially ExecImpl::surfAction

I fail to understand why a direct access to ExecImpl::surfAction is
mandatory

diff --git a/src/kernel/activity/ExecImpl.cpp b/src/kernel/activity/ExecImpl.cpp
index 259d5cb..ac22116 100644 (file)
--- a/src/kernel/activity/ExecImpl.cpp
+++ b/src/kernel/activity/ExecImpl.cpp
@@ -45,16 +45,22 @@ simgrid::kernel::activity::ExecImpl::~ExecImpl()
 void simgrid::kernel::activity::ExecImpl::suspend()
 {
   XBT_VERB("This exec is suspended (remain: %f)", surfAction_->get_remains());
-  if (surfAction_)
+  if (surfAction_ != nullptr)
     surfAction_->suspend();
 }
 
 void simgrid::kernel::activity::ExecImpl::resume()
 {
   XBT_VERB("This exec is resumed (remain: %f)", surfAction_->get_remains());
-  if (surfAction_)
+  if (surfAction_ != nullptr)
     surfAction_->resume();
 }
+void simgrid::kernel::activity::ExecImpl::cancel()
+{
+  XBT_VERB("This exec %p is canceled", this);
+  if (surfAction_ != nullptr)
+    surfAction_->cancel();
+}
 
 double simgrid::kernel::activity::ExecImpl::get_remaining()
 {
@@ -78,6 +84,11 @@ void simgrid::kernel::activity::ExecImpl::set_bound(double bound)
   if (surfAction_)
     surfAction_->set_bound(bound);
 }
+void simgrid::kernel::activity::ExecImpl::set_priority(double priority)
+{
+  if (surfAction_)
+    surfAction_->set_priority(priority);
+}
 
 void simgrid::kernel::activity::ExecImpl::post()
 {

diff --git a/src/kernel/activity/ExecImpl.hpp b/src/kernel/activity/ExecImpl.hpp
index 6aa17c9..59876f3 100644 (file)
--- a/src/kernel/activity/ExecImpl.hpp
+++ b/src/kernel/activity/ExecImpl.hpp
@@ -21,16 +21,21 @@ public:
                     s4u::Host* host);
   void suspend() override;
   void resume() override;
+  void cancel();
   void post() override;
   double get_remaining();
   double get_remaining_ratio();
   void set_bound(double bound);
+  void set_priority(double priority);
   virtual ActivityImpl* migrate(s4u::Host* to);
 
   /* The host where the execution takes place. nullptr means this is a parallel exec (and only surf knows the hosts) */
-  s4u::Host* host_                  = nullptr;
-  resource::Action* surfAction_     = nullptr; /* The Surf execution action encapsulated */
+  s4u::Host* host_ = nullptr;
+  resource::Action* surfAction_; /* The Surf execution action encapsulated */
+private:
   resource::Action* timeoutDetector = nullptr;
+
+public:
   static simgrid::xbt::signal<void(kernel::activity::ExecImplPtr)> onCreation;
   static simgrid::xbt::signal<void(kernel::activity::ExecImplPtr)> onCompletion;
   static simgrid::xbt::signal<void(simgrid::kernel::activity::ExecImplPtr, simgrid::s4u::Host*)> onMigration;

diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp
index 3647e35..5b479f9 100644 (file)
--- a/src/simix/libsmx.cpp
+++ b/src/simix/libsmx.cpp
@@ -117,12 +117,10 @@ void simcall_execution_cancel(smx_activity_t execution)
 {
   simgrid::kernel::activity::ExecImplPtr exec =
       boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(execution);
-  if (not exec->surfAction_)
+  if (exec->surfAction_ == nullptr) // FIXME: One test fails if I remove this, but I don't get why...
     return;
   simgrid::simix::kernelImmediate([exec] {
-    XBT_DEBUG("Cancel synchro %p", exec.get());
-    if (exec->surfAction_)
-      exec->surfAction_->cancel();
+    exec->cancel();
   });
 }
 
@@ -142,8 +140,7 @@ void simcall_execution_set_priority(smx_activity_t execution, double priority)
 
     simgrid::kernel::activity::ExecImplPtr exec =
         boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(execution);
-    if (exec->surfAction_)
-      exec->surfAction_->set_priority(priority);
+    exec->set_priority(priority);
   });
 }
 
@@ -160,8 +157,7 @@ void simcall_execution_set_bound(smx_activity_t execution, double bound)
   simgrid::simix::kernelImmediate([execution, bound] {
     simgrid::kernel::activity::ExecImplPtr exec =
         boost::static_pointer_cast<simgrid::kernel::activity::ExecImpl>(execution);
-    if (exec->surfAction_)
-      exec->surfAction_->set_bound(bound);
+    exec->set_bound(bound);
   });
 }
 

diff --git a/src/simix/smx_host.cpp b/src/simix/smx_host.cpp
index 82a0566..cdd6351 100644 (file)
--- a/src/simix/smx_host.cpp
+++ b/src/simix/smx_host.cpp
@@ -139,7 +139,7 @@ boost::intrusive_ptr<simgrid::kernel::activity::ExecImpl>
 SIMIX_execution_start(const char* name, double flops_amount, double priority, double bound, sg_host_t host)
 {
   /* set surf's action */
-  simgrid::kernel::resource::Action* surf_action      = nullptr;
+  simgrid::kernel::resource::Action* surf_action = nullptr;
   if (not MC_is_active() && not MC_record_replay_is_active()) {
     surf_action = host->pimpl_cpu->execution_start(flops_amount);
     surf_action->set_priority(priority);