From: SUTER Frederic <frederic.suter@cc.in2p3.fr>
Date: Wed, 2 Feb 2022 17:59:17 +0000 (+0100)
Subject: more progress in simcalls modernization and MC cleanup
X-Git-Tag: v3.31~516
X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/c7488e38b30db84e294c54da42c2250791c14864

more progress in simcalls modernization and MC cleanup
---

diff --git a/src/kernel/actor/SimcallObserver.cpp b/src/kernel/actor/SimcallObserver.cpp
index 76de9ec90c..458135a901 100644
--- a/src/kernel/actor/SimcallObserver.cpp
+++ b/src/kernel/actor/SimcallObserver.cpp
@@ -169,7 +169,7 @@ int ActivityTestanySimcall::get_max_consider() const
 
 void ActivityTestanySimcall::prepare(int times_considered)
 {
-  next_value_ = times_considered + 1;
+  next_value_ = times_considered;
 }
 
 std::string ActivityTestanySimcall::to_string(int times_considered) const
@@ -195,6 +195,48 @@ std::string ActivityTestanySimcall::dot_label(int times_considered) const
   return res;
 }
 
+bool ActivityTestSimcall::depends(SimcallObserver* other)
+{
+  if (get_issuer() == other->get_issuer())
+    return false;
+
+  if (dynamic_cast<ActivityTestSimcall*>(other))
+    return true;
+
+  auto* comm1 = dynamic_cast<activity::CommImpl*>(activity_);
+  if (comm1 == nullptr)
+    return false;
+
+  if (dynamic_cast<ActivityWaitSimcall*>(other) != nullptr &&
+      (comm1->src_actor_.get() == nullptr || comm1->dst_actor_.get() == nullptr))
+    return false;
+
+  if (comm1->src_buff_ == nullptr || comm1->dst_buff_ == nullptr)
+    return false;
+
+  if (auto* test = dynamic_cast<ActivityTestSimcall*>(other)) {
+    auto* comm2 = dynamic_cast<activity::CommImpl*>(test->get_activity());
+    if (comm2 == nullptr)
+      return false;
+    else if (comm2->src_buff_ == nullptr || comm2->dst_buff_ == nullptr)
+      return false;
+  }
+
+  if (auto* wait = dynamic_cast<ActivityWaitSimcall*>(other)) {
+    auto* comm2 = dynamic_cast<activity::CommImpl*>(wait->get_activity());
+    if (comm2 == nullptr)
+      return false;
+    if (comm1->src_buff_ == comm2->src_buff_ && comm1->dst_buff_ == comm2->dst_buff_)
+      return false;
+    if (comm1->src_buff_ != nullptr && comm1->dst_buff_ != nullptr && comm2->src_buff_ != nullptr &&
+        comm2->dst_buff_ != nullptr && comm1->dst_buff_ != comm2->src_buff_ && comm1->dst_buff_ != comm2->dst_buff_ &&
+        comm2->dst_buff_ != comm1->src_buff_)
+      return false;
+  }
+
+  return true;
+}
+
 std::string ActivityTestSimcall::to_string(int times_considered) const
 {
   std::string res = SimcallObserver::to_string(times_considered) + "Test ";
@@ -215,7 +257,8 @@ std::string ActivityTestSimcall::to_string(int times_considered) const
              "-> " +
              xbt::string_printf("(%ld)%s (%s)])", dst->get_pid(), dst->get_host()->get_cname(), dst->get_cname());
     }
-  }
+  } else
+    xbt_die("Only Comms are supported here for now");
   return res;
 }
 
@@ -231,12 +274,57 @@ std::string ActivityTestSimcall::dot_label(int times_considered) const
   return res;
 }
 
+bool ActivityWaitSimcall::is_enabled() const
+{
+  /* FIXME: check also that src and dst processes are not suspended */
+  const auto* comm = dynamic_cast<activity::CommImpl*>(activity_);
+  if (comm == nullptr)
+    xbt_die("Only Comms are supported here for now");
+
+  if (comm->src_timeout_ || comm->dst_timeout_) {
+    /* If it has a timeout it will be always be enabled (regardless of who declared the timeout),
+     * because even if the communication is not ready, it can timeout and won't block. */
+    if (_sg_mc_timeout == 1)
+      return true;
+  }
+  /* On the other hand if it hasn't a timeout, check if the comm is ready.*/
+  else if (comm->detached() && comm->src_actor_ == nullptr && comm->get_state() == activity::State::READY)
+    return (comm->dst_actor_ != nullptr);
+  return (comm->src_actor_ && comm->dst_actor_);
+}
+
+bool ActivityWaitSimcall::depends(SimcallObserver* other)
+{
+  if (get_issuer() == other->get_issuer())
+    return false;
+
+  /* Timeouts in wait transitions are not considered by the independence theorem, thus assumed dependent */
+  if (auto* wait = dynamic_cast<ActivityWaitSimcall*>(other)) {
+    if (timeout_ > 0 || wait->get_timeout() > 0)
+      return true;
+    auto* comm1 = dynamic_cast<activity::CommImpl*>(activity_);
+    auto* comm2 = dynamic_cast<activity::CommImpl*>(wait->get_activity());
+
+    if (comm1 == nullptr || comm2 == nullptr) // One wait at least in not on a Comm
+      return true;
+
+    if (comm1->src_buff_ == comm2->src_buff_ && comm1->dst_buff_ == comm2->dst_buff_)
+      return false;
+    if (comm1->src_buff_ != nullptr && comm1->dst_buff_ != nullptr && comm2->src_buff_ != nullptr &&
+        comm2->dst_buff_ != nullptr && comm1->dst_buff_ != comm2->src_buff_ && comm1->dst_buff_ != comm2->dst_buff_ &&
+        comm2->dst_buff_ != comm1->src_buff_)
+      return false;
+  }
+
+  return true;
+}
 std::string ActivityWaitSimcall::to_string(int times_considered) const
 {
   std::string res = SimcallObserver::to_string(times_considered);
   auto* comm      = dynamic_cast<activity::CommImpl*>(activity_);
   if (comm == nullptr)
-    return res;
+    xbt_die("Only Comms are supported here for now");
+
   if (times_considered == -1) {
     res += "WaitTimeout(comm=" + (XBT_LOG_ISENABLED(mc_observer, xbt_log_priority_verbose)
                                       ? xbt::string_printf("%p)", comm)
@@ -265,17 +353,24 @@ std::string ActivityWaitSimcall::dot_label(int times_considered) const
     auto dst = comm->dst_actor_;
     res += " [(" + std::to_string(src ? src->get_pid() : 0) + ")";
     res += "->(" + std::to_string(dst ? dst->get_pid() : 0) + ")]";
-  }
+  } else
+    xbt_die("Only Comms are supported here for now");
   return res;
 }
 
-bool ActivityWaitSimcall::is_enabled() const
+std::string ActivityWaitanySimcall::dot_label(int times_considered) const
 {
-  /* FIXME: check also that src and dst processes are not suspended */
-  const auto* comm = dynamic_cast<activity::CommImpl*>(activity_);
-  if (comm == nullptr)
-    return true;
+  return SimcallObserver::dot_label(times_considered) +
+         xbt::string_printf("WaitAny [%d of %zu]", times_considered + 1, activities_.size());
+}
 
+bool ActivityWaitanySimcall::is_enabled() const
+{
+  // FIXME: deal with other kind of activities (Exec and I/Os)
+  // FIXME: Can be factored with ActivityWaitSimcall::is_enabled()
+  const auto* comm = dynamic_cast<activity::CommImpl*>(activities_[next_value_]);
+  if (comm == nullptr)
+    xbt_die("Only Comms are supported here for now");
   if (comm->src_timeout_ || comm->dst_timeout_) {
     /* If it has a timeout it will be always be enabled (regardless of who declared the timeout),
      * because even if the communication is not ready, it can timeout and won't block. */
@@ -288,37 +383,14 @@ bool ActivityWaitSimcall::is_enabled() const
   return (comm->src_actor_ && comm->dst_actor_);
 }
 
-std::string ActivityWaitanySimcall::dot_label(int times_considered) const
-{
-  return SimcallObserver::dot_label(times_considered) +
-         xbt::string_printf("WaitAny [%d of %zu]", times_considered + 1, activities_.size());
-}
-
-bool ActivityWaitanySimcall::is_enabled() const
-{
-  // FIXME: deal with other kind of activities (Exec and I/Os)
-  for (auto act : activities_) {
-    const auto* comm = dynamic_cast<activity::CommImpl*>(act);
-    if (comm != nullptr && comm->src_actor_ && comm->dst_actor_)
-      return true;
-  }
-  return false;
-}
-
 int ActivityWaitanySimcall::get_max_consider() const
 {
-  // Only Comms are of interest to MC for now. When all types of activities can be consider, this function can simply
-  // return the size of activities_.
-  int count = 0;
-  for (const auto& act : activities_)
-    if (dynamic_cast<activity::CommImpl*>(act) != nullptr)
-      count++;
-  return count;
+  return static_cast<int>(activities_.size());
 }
 
 void ActivityWaitanySimcall::prepare(int times_considered)
 {
-  next_value_ = times_considered + 1;
+  next_value_ = times_considered;
 }
 
 std::string ActivityWaitanySimcall::to_string(int times_considered) const
@@ -331,6 +403,8 @@ std::string ActivityWaitanySimcall::to_string(int times_considered) const
              (XBT_LOG_ISENABLED(mc_observer, xbt_log_priority_verbose) ? xbt::string_printf("%p", comm)
                                                                        : "(verbose only)") +
              xbt::string_printf("(%d of %zu))", times_considered + 1, count);
+    else
+      xbt_die("Only Comms are supported here for now");
   } else
     res += "comm at idx " + std::to_string(times_considered) + ")";
   return res;
diff --git a/src/kernel/actor/SimcallObserver.hpp b/src/kernel/actor/SimcallObserver.hpp
index 54d6fa853e..57f79b3f8c 100644
--- a/src/kernel/actor/SimcallObserver.hpp
+++ b/src/kernel/actor/SimcallObserver.hpp
@@ -177,6 +177,7 @@ public:
   }
   SimcallObserver* clone() override { return new ActivityTestSimcall(get_issuer(), activity_); }
   bool is_visible() const override { return true; }
+  bool depends(SimcallObserver* other);
   std::string to_string(int times_considered) const override;
   std::string dot_label(int times_considered) const override;
   activity::ActivityImpl* get_activity() const { return activity_; }
@@ -184,7 +185,7 @@ public:
 
 class ActivityTestanySimcall : public ResultingSimcall<ssize_t> {
   const std::vector<activity::ActivityImpl*>& activities_;
-  int next_value_ = -1;
+  int next_value_ = 0;
 
 public:
   ActivityTestanySimcall(ActorImpl* actor, const std::vector<activity::ActivityImpl*>& activities)
@@ -213,6 +214,7 @@ public:
   SimcallObserver* clone() override { return new ActivityWaitSimcall(get_issuer(), activity_, timeout_); }
   bool is_visible() const override { return true; }
   bool is_enabled() const override;
+  bool depends(SimcallObserver* other);
   std::string to_string(int times_considered) const override;
   std::string dot_label(int times_considered) const override;
   activity::ActivityImpl* get_activity() const { return activity_; }
@@ -223,7 +225,7 @@ public:
 class ActivityWaitanySimcall : public ResultingSimcall<ssize_t> {
   const std::vector<activity::ActivityImpl*>& activities_;
   const double timeout_;
-  int next_value_ = -1;
+  int next_value_ = 0;
 
 public:
   ActivityWaitanySimcall(ActorImpl* actor, const std::vector<activity::ActivityImpl*>& activities, double timeout)
diff --git a/src/mc/api.cpp b/src/mc/api.cpp
index a02f0fc09c..5e25c4c423 100644
--- a/src/mc/api.cpp
+++ b/src/mc/api.cpp
@@ -54,32 +54,6 @@ static std::string buff_size_to_string(size_t buff_size)
 
 static void simcall_translate(smx_simcall_t req, Remote<kernel::activity::CommImpl>& buffered_comm);
 
-static bool request_is_enabled_by_idx(const RemoteProcess& process, smx_simcall_t req, unsigned int idx)
-{
-  kernel::activity::ActivityImpl* remote_act = nullptr;
-  if (auto wait = dynamic_cast<kernel::actor::ActivityWaitSimcall*>(req->observer_))
-    /* FIXME: check also that src and dst processes are not suspended */
-    remote_act = wait->get_activity();
-  else if (auto waitany = dynamic_cast<kernel::actor::ActivityWaitanySimcall*>(req->observer_))
-    remote_act = waitany->get_activities().at(idx);
-  else if (auto testany = dynamic_cast<kernel::actor::ActivityTestanySimcall*>(req->observer_))
-    remote_act = testany->get_activities().at(idx);
-
-  switch (req->call_) {
-    case Simcall::COMM_WAIT:
-    case Simcall::COMM_WAITANY:
-    case Simcall::COMM_TESTANY:
-      break;
-    default:
-      return true;
-  }
-
-  Remote<kernel::activity::CommImpl> temp_comm;
-  process.read(temp_comm, remote(static_cast<kernel::activity::CommImpl*>(remote_act)));
-  const kernel::activity::CommImpl* comm = temp_comm.get_buffer();
-  return comm->src_actor_.get() && comm->dst_actor_.get();
-}
-
 /* Search an enabled transition for the given process.
  *
  * This can be seen as an iterator returning the next transition of the process.
@@ -111,60 +85,11 @@ static inline smx_simcall_t MC_state_choose_request_for_process(const RemoteProc
     if (actor->simcall_.mc_max_consider_ <= procstate->get_times_considered())
       procstate->set_done();
     req = &actor->simcall_;
-  } else
-    switch (actor->simcall_.call_) {
-      case Simcall::COMM_WAITANY:
-        while (procstate->get_times_considered() < simcall_comm_waitany__get__count(&actor->simcall_)) {
-          if (simgrid::mc::request_is_enabled_by_idx(process, &actor->simcall_, procstate->get_times_considered())) {
-            state->transition_.times_considered_ = procstate->get_times_considered_and_inc();
-            break;
-          }
-          procstate->get_times_considered_and_inc();
-        }
-
-        if (procstate->get_times_considered() >= simcall_comm_waitany__get__count(&actor->simcall_))
-          procstate->set_done();
-        if (state->transition_.times_considered_ != -1)
-          req = &actor->simcall_;
-        break;
-
-      case Simcall::COMM_TESTANY:
-        while (procstate->get_times_considered() < simcall_comm_testany__get__count(&actor->simcall_)) {
-          if (simgrid::mc::request_is_enabled_by_idx(process, &actor->simcall_, procstate->get_times_considered())) {
-            state->transition_.times_considered_ = procstate->get_times_considered_and_inc();
-            break;
-          }
-          procstate->get_times_considered_and_inc();
-        }
-
-        if (procstate->get_times_considered() >= simcall_comm_testany__get__count(&actor->simcall_))
-          procstate->set_done();
-        if (state->transition_.times_considered_ != -1)
-          req = &actor->simcall_;
-        break;
-
-      case Simcall::COMM_WAIT: {
-        simgrid::mc::RemotePtr<simgrid::kernel::activity::CommImpl> remote_act =
-            remote(simcall_comm_wait__get__comm(&actor->simcall_));
-        simgrid::mc::Remote<simgrid::kernel::activity::CommImpl> temp_act;
-        process.read(temp_act, remote_act);
-        const simgrid::kernel::activity::CommImpl* act = temp_act.get_buffer();
-        if (act->src_actor_.get() && act->dst_actor_.get())
-          state->transition_.times_considered_ = 0; // OK
-        else if (act->src_actor_.get() == nullptr && act->get_state() == simgrid::kernel::activity::State::READY &&
-                 act->detached())
-          state->transition_.times_considered_ = 0; // OK
-        procstate->set_done();
-        req = &actor->simcall_;
-        break;
-      }
-
-      default:
-        procstate->set_done();
-        state->transition_.times_considered_ = 0;
-        req                                  = &actor->simcall_;
-        break;
-    }
+  } else {
+    procstate->set_done();
+    state->transition_.times_considered_ = 0;
+    req                                  = &actor->simcall_;
+  }
   if (not req)
     return nullptr;
 
@@ -256,7 +181,7 @@ bool Api::simcall_check_dependency(smx_simcall_t req1, smx_simcall_t req2) const
   const auto TEST  = Simcall::COMM_TEST;
   const auto WAIT  = Simcall::COMM_WAIT;
 
-  if (req1->issuer_ == req2->issuer_)
+  if (req1->issuer_ == req2->issuer_) // Done in observer for TEST and WAIT
     return false;
 
   /* The independence theorem only consider 4 simcalls. All others are dependent with anything. */
@@ -265,11 +190,6 @@ bool Api::simcall_check_dependency(smx_simcall_t req1, smx_simcall_t req2) const
   if (req2->call_ != ISEND && req2->call_ != IRECV && req2->call_ != TEST && req2->call_ != WAIT)
     return true;
 
-  /* Timeouts in wait transitions are not considered by the independence theorem, thus assumed dependent */
-  if ((req1->call_ == WAIT && simcall_comm_wait__get__timeout(req1) > 0) ||
-      (req2->call_ == WAIT && simcall_comm_wait__get__timeout(req2) > 0))
-    return true;
-
   /* Make sure that req1 and req2 are in alphabetic order */
   if (req1->call_ > req2->call_) {
     auto temp = req1;
@@ -277,7 +197,6 @@ bool Api::simcall_check_dependency(smx_simcall_t req1, smx_simcall_t req2) const
     req2      = temp;
   }
 
-  auto comm1 = get_comm_or_nullptr(req1);
   auto comm2 = get_comm_or_nullptr(req2);
 
   /* First case: that's not the same kind of request (we also know that req1 < req2 alphabetically) */
@@ -313,26 +232,6 @@ bool Api::simcall_check_dependency(smx_simcall_t req1, smx_simcall_t req2) const
     return false;
 #endif
 
-    if (req1->call_ == TEST && req2->call_ == WAIT &&
-        (comm1->src_actor_.get() == nullptr || comm1->dst_actor_.get() == nullptr))
-      return false;
-
-    if (req1->call_ == TEST &&
-        (simcall_comm_test__get__comm(req1) == nullptr || comm1->src_buff_ == nullptr || comm1->dst_buff_ == nullptr))
-      return false;
-    if (req2->call_ == TEST &&
-        (simcall_comm_test__get__comm(req2) == nullptr || comm2->src_buff_ == nullptr || comm2->dst_buff_ == nullptr))
-      return false;
-
-    if (req1->call_ == TEST && req2->call_ == WAIT && comm1->src_buff_ == comm2->src_buff_ &&
-        comm1->dst_buff_ == comm2->dst_buff_)
-      return false;
-
-    if (req1->call_ == TEST && req2->call_ == WAIT && comm1->src_buff_ != nullptr && comm1->dst_buff_ != nullptr &&
-        comm2->src_buff_ != nullptr && comm2->dst_buff_ != nullptr && comm1->dst_buff_ != comm2->src_buff_ &&
-        comm1->dst_buff_ != comm2->dst_buff_ && comm2->dst_buff_ != comm1->src_buff_)
-      return false;
-
     return true;
   }
 
@@ -342,14 +241,6 @@ bool Api::simcall_check_dependency(smx_simcall_t req1, smx_simcall_t req2) const
       return simcall_comm_isend__get__mbox(req1) == simcall_comm_isend__get__mbox(req2);
     case IRECV:
       return simcall_comm_irecv__get__mbox(req1) == simcall_comm_irecv__get__mbox(req2);
-    case WAIT:
-      if (comm1->src_buff_ == comm2->src_buff_ && comm1->dst_buff_ == comm2->dst_buff_)
-        return false;
-      if (comm1->src_buff_ != nullptr && comm1->dst_buff_ != nullptr && comm2->src_buff_ != nullptr &&
-          comm2->dst_buff_ != nullptr && comm1->dst_buff_ != comm2->src_buff_ && comm1->dst_buff_ != comm2->dst_buff_ &&
-          comm2->dst_buff_ != comm1->src_buff_)
-        return false;
-      return true;
     default:
       return true;
   }