From: Martin Quinson <martin.quinson@loria.fr>
Date: Sat, 11 Feb 2017 15:00:37 +0000 (+0100)
Subject: (partially) convert simix::MailboxImpl to C++
X-Git-Tag: v3_15~418
X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/0921a44d5521b3337b8752aa2a9f06ee1405fb24?hp=bc8fcd9f88eea284c282d7b76f94e8b9294208e2

(partially) convert simix::MailboxImpl to C++
---

diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h
index 45672338d0..053849aa42 100644
--- a/include/simgrid/simix.h
+++ b/include/simgrid/simix.h
@@ -293,7 +293,6 @@ XBT_PUBLIC(e_smx_state_t) simcall_process_sleep(double duration);
 /************************** Comunication simcalls *****************************/
 /***** Rendez-vous points *****/
 
-XBT_PUBLIC(smx_mailbox_t) simcall_mbox_create(const char *name);
 XBT_PUBLIC(void) simcall_mbox_set_receiver(smx_mailbox_t mbox , smx_actor_t process);
 
 /***** Communication simcalls *****/
diff --git a/src/kernel/activity/SynchroComm.cpp b/src/kernel/activity/SynchroComm.cpp
index c08a49317f..f8969b7c98 100644
--- a/src/kernel/activity/SynchroComm.cpp
+++ b/src/kernel/activity/SynchroComm.cpp
@@ -35,8 +35,7 @@ simgrid::kernel::activity::Comm::~Comm()
   }
 
   if(mbox)
-    SIMIX_mbox_remove(mbox, this);
-
+    mbox->remove(this);
 }
 void simgrid::kernel::activity::Comm::suspend()
 {
@@ -60,7 +59,7 @@ void simgrid::kernel::activity::Comm::cancel()
   /* if the synchro is a waiting state means that it is still in a mbox */
   /* so remove from it and delete it */
   if (state == SIMIX_WAITING) {
-    SIMIX_mbox_remove(mbox, this);
+    mbox->remove(this);
     state = SIMIX_CANCELED;
   }
   else if (!MC_is_active() /* when running the MC there are no surf actions */
diff --git a/src/s4u/s4u_mailbox.cpp b/src/s4u/s4u_mailbox.cpp
index d6b0d967ee..9e5f3f4c9c 100644
--- a/src/s4u/s4u_mailbox.cpp
+++ b/src/s4u/s4u_mailbox.cpp
@@ -22,15 +22,12 @@ const char *Mailbox::name() {
 
 MailboxPtr Mailbox::byName(const char*name)
 {
-  // FIXME: there is a race condition here where two actors run Mailbox::byName
-  // on a non-existent mailbox during the same scheduling round. Both will be
-  // interrupted in the simcall creating the underlying simix mbox.
-  // Only one simix object will be created, but two S4U objects will be created.
-  // Only one S4U object will be stored in the hashmap and used, and the other
-  // one will be leaked.
-  smx_mailbox_t mbox = SIMIX_mbox_get_by_name(name);
-  if (mbox == nullptr)
-    mbox = simcall_mbox_create(name);
+  simix::MailboxImpl* mbox = simix::MailboxImpl::byNameOrNull(name);
+  if (mbox == nullptr) {
+    mbox = simix::kernelImmediate([name] {
+      return simix::MailboxImpl::byNameOrCreate(name);
+    });
+  }
   return MailboxPtr(&mbox->piface_, true);
 }
 
diff --git a/src/simix/MailboxImpl.cpp b/src/simix/MailboxImpl.cpp
index c30d784966..993644819a 100644
--- a/src/simix/MailboxImpl.cpp
+++ b/src/simix/MailboxImpl.cpp
@@ -15,24 +15,6 @@ void SIMIX_mailbox_exit()
 {
   xbt_dict_free(&mailboxes);
 }
-
-/******************************************************************************/
-/*                           Rendez-Vous Points                               */
-/******************************************************************************/
-
-smx_mailbox_t SIMIX_mbox_create(const char* name)
-{
-  xbt_assert(name, "Mailboxes must have a name");
-  /* two processes may have pushed the same mbox_create simcall at the same time */
-  smx_mailbox_t mbox = static_cast<smx_mailbox_t>(xbt_dict_get_or_null(mailboxes, name));
-  if (!mbox) {
-    mbox = new simgrid::simix::MailboxImpl(name);
-    XBT_DEBUG("Creating a mailbox at %p with name %s", mbox, name);
-    xbt_dict_set(mailboxes, mbox->name_, mbox, nullptr);
-  }
-  return mbox;
-}
-
 void SIMIX_mbox_free(void* data)
 {
   XBT_DEBUG("mbox free %p", data);
@@ -40,10 +22,9 @@ void SIMIX_mbox_free(void* data)
   delete mbox;
 }
 
-smx_mailbox_t SIMIX_mbox_get_by_name(const char* name)
-{
-  return static_cast<smx_mailbox_t>(xbt_dict_get_or_null(mailboxes, name));
-}
+/******************************************************************************/
+/*                           Rendez-Vous Points                               */
+/******************************************************************************/
 
 /**
  *  \brief set the receiver of the rendez vous point to allow eager sends
@@ -55,32 +36,50 @@ void SIMIX_mbox_set_receiver(smx_mailbox_t mbox, smx_actor_t process)
   mbox->permanent_receiver = process;
 }
 
-/**
- *  \brief Pushes a communication synchro into a rendez-vous point
- *  \param mbox The mailbox
- *  \param synchro The communication synchro
+namespace simgrid {
+namespace simix {
+/** @brief Returns the mailbox of that name, or nullptr */
+MailboxImpl* MailboxImpl::byNameOrNull(const char* name)
+{
+  return static_cast<smx_mailbox_t>(xbt_dict_get_or_null(mailboxes, name));
+}
+/** @brief Returns the mailbox of that name, newly created on need */
+MailboxImpl* MailboxImpl::byNameOrCreate(const char* name)
+{
+  xbt_assert(name, "Mailboxes must have a name");
+  /* two processes may have pushed the same mbox_create simcall at the same time */
+  smx_mailbox_t mbox = static_cast<smx_mailbox_t>(xbt_dict_get_or_null(mailboxes, name));
+  if (!mbox) {
+    mbox = new simgrid::simix::MailboxImpl(name);
+    XBT_DEBUG("Creating a mailbox at %p with name %s", mbox, name);
+    xbt_dict_set(mailboxes, mbox->name_, mbox, nullptr);
+  }
+  return mbox;
+}
+/** @brief Pushes a communication activity into a mailbox
+ *  @param activity What to add
  */
-void SIMIX_mbox_push(smx_mailbox_t mbox, smx_activity_t synchro)
+void MailboxImpl::push(smx_activity_t synchro)
 {
   simgrid::kernel::activity::Comm* comm = static_cast<simgrid::kernel::activity::Comm*>(synchro);
-  mbox->comm_queue.push_back(comm);
-  comm->mbox = mbox;
+  this->comm_queue.push_back(comm);
+  comm->mbox = this;
 }
 
-/**
- *  \brief Removes a communication synchro from a rendez-vous point
- *  \param mbox The rendez-vous point
- *  \param synchro The communication synchro
+/** @brief Removes a communication activity from a mailbox
+ *  @param activity What to remove
  */
-void SIMIX_mbox_remove(smx_mailbox_t mbox, smx_activity_t synchro)
+void MailboxImpl::remove(smx_activity_t activity)
 {
-  simgrid::kernel::activity::Comm* comm = static_cast<simgrid::kernel::activity::Comm*>(synchro);
+  simgrid::kernel::activity::Comm* comm = static_cast<simgrid::kernel::activity::Comm*>(activity);
 
   comm->mbox = nullptr;
-  for (auto it = mbox->comm_queue.begin(); it != mbox->comm_queue.end(); it++)
+  for (auto it = this->comm_queue.begin(); it != this->comm_queue.end(); it++)
     if (*it == comm) {
-      mbox->comm_queue.erase(it);
+      this->comm_queue.erase(it);
       return;
     }
-  xbt_die("Cannot remove the comm %p that is not part of the mailbox %s", comm, mbox->name_);
+  xbt_die("Cannot remove the comm %p that is not part of the mailbox %s", comm, this->name_);
+}
+}
 }
diff --git a/src/simix/MailboxImpl.hpp b/src/simix/MailboxImpl.hpp
index 16c7560124..bd9c49b62b 100644
--- a/src/simix/MailboxImpl.hpp
+++ b/src/simix/MailboxImpl.hpp
@@ -18,17 +18,23 @@ namespace simix {
 /** @brief Rendez-vous point datatype */
 
 class MailboxImpl {
-public:
   explicit MailboxImpl(const char* name)
       : piface_(this), name_(xbt_strdup(name)), comm_queue(MAX_MAILBOX_SIZE), done_comm_queue(MAX_MAILBOX_SIZE)
   {
   }
+
+public:
   ~MailboxImpl() { xbt_free(name_); }
 
+  static MailboxImpl* byNameOrNull(const char* name);
+  static MailboxImpl* byNameOrCreate(const char* name);
+  void push(smx_activity_t synchro);
+  void remove(smx_activity_t activity);
   simgrid::s4u::Mailbox piface_; // Our interface
   char* name_;
-  boost::circular_buffer_space_optimized<smx_activity_t> comm_queue;
+
   boost::intrusive_ptr<simgrid::simix::ActorImpl> permanent_receiver; //process which the mailbox is attached to
+  boost::circular_buffer_space_optimized<smx_activity_t> comm_queue;
   boost::circular_buffer_space_optimized<smx_activity_t> done_comm_queue;//messages already received in the permanent receive mode
 };
 }
@@ -36,11 +42,6 @@ public:
 
 XBT_PRIVATE void SIMIX_mailbox_exit();
 
-XBT_PRIVATE smx_mailbox_t SIMIX_mbox_create(const char* name);
-XBT_PRIVATE smx_mailbox_t SIMIX_mbox_get_by_name(const char* name);
-XBT_PRIVATE void SIMIX_mbox_remove(smx_mailbox_t mbox, smx_activity_t comm);
-XBT_PRIVATE void SIMIX_mbox_push(smx_mailbox_t mbox, smx_activity_t synchro);
-
 XBT_PRIVATE void SIMIX_mbox_set_receiver(smx_mailbox_t mbox, smx_actor_t proc);
 
 #endif /* SIMIX_MAILBOXIMPL_H */
diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp
index bd4370014b..a580e9aac9 100644
--- a/src/simix/libsmx.cpp
+++ b/src/simix/libsmx.cpp
@@ -367,17 +367,6 @@ e_smx_state_t simcall_process_sleep(double duration)
   return (e_smx_state_t) simcall_BODY_process_sleep(duration);
 }
 
-/**
- *  \ingroup simix_mbox_management
- *  \brief Creates a new rendez-vous point
- *  \param name The name of the rendez-vous point
- *  \return The created rendez-vous point
- */
-smx_mailbox_t simcall_mbox_create(const char *name)
-{
-  return simcall_BODY_mbox_create(name);
-}
-
 void simcall_mbox_set_receiver(smx_mailbox_t mbox, smx_actor_t process)
 {
   simcall_BODY_mbox_set_receiver(mbox, process);
diff --git a/src/simix/popping_accessors.h b/src/simix/popping_accessors.h
index c4108f4e64..5070dfdf83 100644
--- a/src/simix/popping_accessors.h
+++ b/src/simix/popping_accessors.h
@@ -284,19 +284,6 @@ static inline void simcall_process_restart__set__result(smx_simcall_t simcall, s
     simgrid::simix::marshal<smx_actor_t>(simcall->result, result);
 }
 
-static inline const char* simcall_mbox_create__get__name(smx_simcall_t simcall) {
-  return simgrid::simix::unmarshal<const char*>(simcall->args[0]);
-}
-static inline void simcall_mbox_create__set__name(smx_simcall_t simcall, const char* arg) {
-    simgrid::simix::marshal<const char*>(simcall->args[0], arg);
-}
-static inline smx_mailbox_t simcall_mbox_create__get__result(smx_simcall_t simcall){
-    return simgrid::simix::unmarshal<smx_mailbox_t>(simcall->result);
-}
-static inline void simcall_mbox_create__set__result(smx_simcall_t simcall, smx_mailbox_t result){
-    simgrid::simix::marshal<smx_mailbox_t>(simcall->result, result);
-}
-
 static inline smx_mailbox_t simcall_mbox_set_receiver__get__mbox(smx_simcall_t simcall) {
   return simgrid::simix::unmarshal<smx_mailbox_t>(simcall->args[0]);
 }
diff --git a/src/simix/popping_bodies.cpp b/src/simix/popping_bodies.cpp
index 774b702eb5..0bcb22dc13 100644
--- a/src/simix/popping_bodies.cpp
+++ b/src/simix/popping_bodies.cpp
@@ -143,12 +143,6 @@ inline static smx_actor_t simcall_BODY_process_restart(smx_actor_t process) {
     return simcall<smx_actor_t, smx_actor_t>(SIMCALL_PROCESS_RESTART, process);
   }
   
-inline static smx_mailbox_t simcall_BODY_mbox_create(const char* name) {
-    /* Go to that function to follow the code flow through the simcall barrier */
-    if (0) SIMIX_mbox_create(name);
-    return simcall<smx_mailbox_t, const char*>(SIMCALL_MBOX_CREATE, name);
-  }
-  
 inline static void simcall_BODY_mbox_set_receiver(smx_mailbox_t mbox, smx_actor_t receiver) {
     /* Go to that function to follow the code flow through the simcall barrier */
     if (0) SIMIX_mbox_set_receiver(mbox, receiver);
diff --git a/src/simix/popping_enum.h b/src/simix/popping_enum.h
index b10d358b45..27a3f572db 100644
--- a/src/simix/popping_enum.h
+++ b/src/simix/popping_enum.h
@@ -36,7 +36,6 @@ typedef enum {
   SIMCALL_PROCESS_ON_EXIT,
   SIMCALL_PROCESS_AUTO_RESTART_SET,
   SIMCALL_PROCESS_RESTART,
-  SIMCALL_MBOX_CREATE,
   SIMCALL_MBOX_SET_RECEIVER,
   SIMCALL_COMM_IPROBE,
   SIMCALL_COMM_SEND,
diff --git a/src/simix/popping_generated.cpp b/src/simix/popping_generated.cpp
index 742c404c69..689f2ef2b9 100644
--- a/src/simix/popping_generated.cpp
+++ b/src/simix/popping_generated.cpp
@@ -42,7 +42,6 @@ const char* simcall_names[] = {
     "SIMCALL_PROCESS_ON_EXIT",
     "SIMCALL_PROCESS_AUTO_RESTART_SET",
     "SIMCALL_PROCESS_RESTART",
-    "SIMCALL_MBOX_CREATE",
     "SIMCALL_MBOX_SET_RECEIVER",
     "SIMCALL_COMM_IPROBE",
     "SIMCALL_COMM_SEND",
@@ -185,11 +184,6 @@ case SIMCALL_PROCESS_RESTART:
       SIMIX_simcall_answer(simcall);
       break;
 
-case SIMCALL_MBOX_CREATE:
-      simgrid::simix::marshal<smx_mailbox_t>(simcall->result, SIMIX_mbox_create(simgrid::simix::unmarshal<const char*>(simcall->args[0])));
-      SIMIX_simcall_answer(simcall);
-      break;
-
 case SIMCALL_MBOX_SET_RECEIVER:
       SIMIX_mbox_set_receiver(simgrid::simix::unmarshal<smx_mailbox_t>(simcall->args[0]), simgrid::simix::unmarshal<smx_actor_t>(simcall->args[1]));
       SIMIX_simcall_answer(simcall);
diff --git a/src/simix/simcalls.in b/src/simix/simcalls.in
index 9adf7d2407..77ce2e8f53 100644
--- a/src/simix/simcalls.in
+++ b/src/simix/simcalls.in
@@ -57,7 +57,6 @@ void          process_on_exit(smx_actor_t process, int_f_pvoid_pvoid_t fun, void
 void          process_auto_restart_set(smx_actor_t process, int auto_restart) [[nohandler]];
 smx_actor_t process_restart(smx_actor_t process);
 
-smx_mailbox_t mbox_create(const char* name) [[nohandler]];
 void          mbox_set_receiver(smx_mailbox_t mbox, smx_actor_t receiver) [[nohandler]];
 
 smx_activity_t comm_iprobe(smx_mailbox_t mbox, int type, int src, int tag, simix_match_func_t match_fun, void* data);
diff --git a/src/simix/smx_network.cpp b/src/simix/smx_network.cpp
index dc519f0fe2..63171c5247 100644
--- a/src/simix/smx_network.cpp
+++ b/src/simix/smx_network.cpp
@@ -121,7 +121,7 @@ XBT_PRIVATE smx_activity_t simcall_HANDLER_comm_isend(smx_simcall_t simcall, smx
       XBT_DEBUG("pushing a message into the permanent receive fifo %p, comm %p", mbox, &(other_comm));
 
     }else{
-      SIMIX_mbox_push(mbox, this_synchro);
+      mbox->push(this_synchro);
     }
   } else {
     XBT_DEBUG("Receive already pushed");
@@ -201,7 +201,7 @@ smx_activity_t SIMIX_comm_irecv(smx_actor_t dst_proc, smx_mailbox_t mbox, void *
     if (!other_synchro)  {
       XBT_DEBUG("We have messages in the permanent receive list, but not the one we are looking for, pushing request into fifo");
       other_synchro = this_synchro;
-      SIMIX_mbox_push(mbox, this_synchro);
+      mbox->push(this_synchro);
     } else {
       simgrid::kernel::activity::Comm *other_comm = static_cast<simgrid::kernel::activity::Comm*>(other_synchro);
 
@@ -226,7 +226,7 @@ smx_activity_t SIMIX_comm_irecv(smx_actor_t dst_proc, smx_mailbox_t mbox, void *
     if (!other_synchro) {
       XBT_DEBUG("Receive pushed first %zu", mbox->comm_queue.size());
       other_synchro = this_synchro;
-      SIMIX_mbox_push(mbox, this_synchro);
+      mbox->push(this_synchro);
     } else {
       this_synchro->unref();
       simgrid::kernel::activity::Comm *other_comm = static_cast<simgrid::kernel::activity::Comm*>(other_synchro);
@@ -531,7 +531,7 @@ void SIMIX_comm_finish(smx_activity_t synchro)
 
     /* If the synchro is still in a rendez-vous point then remove from it */
     if (comm->mbox)
-      SIMIX_mbox_remove(comm->mbox, synchro);
+      comm->mbox->remove(synchro);
 
     XBT_DEBUG("SIMIX_comm_finish: synchro state = %d", (int)synchro->state);
 
diff --git a/src/smpi/smpi_global.cpp b/src/smpi/smpi_global.cpp
index 526ad14920..a36fe01a20 100644
--- a/src/smpi/smpi_global.cpp
+++ b/src/smpi/smpi_global.cpp
@@ -4,18 +4,19 @@
 /* This program is free software; you can redistribute it and/or modify it
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
+#include "mc/mc.h"
 #include "private.h"
 #include "private.hpp"
+#include "simgrid/s4u/Mailbox.hpp"
+#include "simgrid/sg_config.h"
 #include "smpi_mpi_dt_private.h"
-#include "mc/mc.h"
+#include "src/kernel/activity/SynchroComm.hpp"
 #include "src/mc/mc_record.h"
-#include "xbt/replay.h"
-#include "surf/surf.h"
-#include "src/simix/smx_private.h"
-#include "simgrid/sg_config.h"
 #include "src/mc/mc_replay.h"
 #include "src/msg/msg_private.h"
-#include "src/kernel/activity/SynchroComm.hpp"
+#include "src/simix/smx_private.h"
+#include "surf/surf.h"
+#include "xbt/replay.h"
 
 #include <float.h> /* DBL_MAX */
 #include <fstream>
@@ -46,8 +47,8 @@ typedef struct s_smpi_process_data {
   double simulated;
   int *argc;
   char ***argv;
-  smx_mailbox_t mailbox;
-  smx_mailbox_t mailbox_small;
+  simgrid::s4u::MailboxPtr mailbox;
+  simgrid::s4u::MailboxPtr mailbox_small;
   xbt_mutex_t mailboxes_mutex;
   xbt_os_timer_t timer;
   MPI_Comm comm_self;
@@ -144,7 +145,7 @@ void smpi_process_init(int *argc, char ***argv)
     data->argc = argc;
     data->argv = argv;
     // set the process attached to the mailbox
-    simcall_mbox_set_receiver(data->mailbox_small, proc);
+    simcall_mbox_set_receiver(data->mailbox_small->getImpl(), proc);
     XBT_DEBUG("<%d> New process in the game: %p", index, proc);
   }
   xbt_assert(smpi_process_data(),
@@ -281,13 +282,13 @@ MPI_Comm smpi_process_comm_world()
 smx_mailbox_t smpi_process_mailbox()
 {
   smpi_process_data_t data = smpi_process_data();
-  return data->mailbox;
+  return data->mailbox->getImpl();
 }
 
 smx_mailbox_t smpi_process_mailbox_small()
 {
   smpi_process_data_t data = smpi_process_data();
-  return data->mailbox_small;
+  return data->mailbox_small->getImpl();
 }
 
 xbt_mutex_t smpi_process_mailboxes_mutex()
@@ -299,13 +300,13 @@ xbt_mutex_t smpi_process_mailboxes_mutex()
 smx_mailbox_t smpi_process_remote_mailbox(int index)
 {
   smpi_process_data_t data = smpi_process_remote_data(index);
-  return data->mailbox;
+  return data->mailbox->getImpl();
 }
 
 smx_mailbox_t smpi_process_remote_mailbox_small(int index)
 {
   smpi_process_data_t data = smpi_process_remote_data(index);
-  return data->mailbox_small;
+  return data->mailbox_small->getImpl();
 }
 
 xbt_mutex_t smpi_process_remote_mailboxes_mutex(int index)
@@ -562,8 +563,8 @@ void smpi_global_init()
     process_data[i]                       = new s_smpi_process_data_t;
     process_data[i]->argc                 = nullptr;
     process_data[i]->argv                 = nullptr;
-    process_data[i]->mailbox              = simcall_mbox_create(get_mailbox_name(name, i));
-    process_data[i]->mailbox_small        = simcall_mbox_create(get_mailbox_name_small(name, i));
+    process_data[i]->mailbox              = simgrid::s4u::Mailbox::byName(get_mailbox_name(name, i));
+    process_data[i]->mailbox_small        = simgrid::s4u::Mailbox::byName(get_mailbox_name_small(name, i));
     process_data[i]->mailboxes_mutex      = xbt_mutex_init();
     process_data[i]->timer                = xbt_os_timer_new();
     if (MC_is_active())