Change way Mailboxes are create, stored, and destroyed

- keep the global map in EngineImpl (not as a static global in
  MailboxImpl.cpp)
- Delete mailboxes in ~EngineImpl() and get rid of SIMIX_mailbox_exit
- replace MailboxImpl::by_name_or_null and
  MailboxImpl::by_name_or_create by Engine::mailbox_by_name_or_create
  + better match with what is done for hosts, links, and actors
  + Mailbox::by_name cause two searchs in the map. One to check if
    name already points to a mailbox (by_name_or_null) and if not
    search again before creating a new mailbox. As there is no
    Mailbox::by_name_or_null, just keep the latter.
- Revalidate a bunch of tests (message ordering mostly)

diff --git a/examples/c/cloud-masterworker/cloud-masterworker.tesh b/examples/c/cloud-masterworker/cloud-masterworker.tesh
index 594a6e7..e11940f 100644 (file)
--- a/examples/c/cloud-masterworker/cloud-masterworker.tesh
+++ b/examples/c/cloud-masterworker/cloud-masterworker.tesh
@@ -10,8 +10,8 @@ $ ${bindir:=.}/c-cloud-masterworker --log=no_loc ${platfdir}/cluster_backbone.xm
 > [VM00:WRK00:(2) 0.000000] [cloud_masterworker/INFO] WRK00 is listening on mailbox(MBOX:WRK00)
 > [node-0.simgrid.org:master:(1) 0.000000] [cloud_masterworker/INFO] put an actor (WRK01) on VM01
 > [node-0.simgrid.org:master:(1) 0.000000] [cloud_masterworker/INFO] # Send to 2 worker actors
-> [node-0.simgrid.org:master:(1) 0.000000] [cloud_masterworker/INFO] Send to mailbox(MBOX:WRK00)
 > [VM01:WRK01:(3) 0.000000] [cloud_masterworker/INFO] WRK01 is listening on mailbox(MBOX:WRK01)
+> [node-0.simgrid.org:master:(1) 0.000000] [cloud_masterworker/INFO] Send to mailbox(MBOX:WRK00)
 > [VM00:WRK00:(2) 0.090280] [cloud_masterworker/INFO] WRK00 received from mailbox(MBOX:WRK00)
 > [node-0.simgrid.org:master:(1) 0.090280] [cloud_masterworker/INFO] Send to mailbox(MBOX:WRK01)
 > [VM00:WRK00:(2) 0.100280] [cloud_masterworker/INFO] WRK00 executed
@@ -28,8 +28,8 @@ $ ${bindir:=.}/c-cloud-masterworker --log=no_loc ${platfdir}/cluster_backbone.xm
 > [node-0.simgrid.org:master:(1) 10.000000] [cloud_masterworker/INFO] put an actor (WRK03) on VM01
 > [VM00:WRK02:(4) 10.000000] [cloud_masterworker/INFO] WRK02 is listening on mailbox(MBOX:WRK02)
 > [node-0.simgrid.org:master:(1) 10.000000] [cloud_masterworker/INFO] # Send to 4 worker actors
-> [node-0.simgrid.org:master:(1) 10.000000] [cloud_masterworker/INFO] Send to mailbox(MBOX:WRK00)
 > [VM01:WRK03:(5) 10.000000] [cloud_masterworker/INFO] WRK03 is listening on mailbox(MBOX:WRK03)
+> [node-0.simgrid.org:master:(1) 10.000000] [cloud_masterworker/INFO] Send to mailbox(MBOX:WRK00)
 > [VM00:WRK00:(2) 10.090280] [cloud_masterworker/INFO] WRK00 received from mailbox(MBOX:WRK00)
 > [node-0.simgrid.org:master:(1) 10.090280] [cloud_masterworker/INFO] Send to mailbox(MBOX:WRK01)
 > [VM00:WRK00:(2) 10.100280] [cloud_masterworker/INFO] WRK00 executed

diff --git a/examples/c/dht-kademlia/dht-kademlia.tesh b/examples/c/dht-kademlia/dht-kademlia.tesh
index 036e0bb..ba902ef 100644 (file)
--- a/examples/c/dht-kademlia/dht-kademlia.tesh
+++ b/examples/c/dht-kademlia/dht-kademlia.tesh
@@ -17,16 +17,16 @@ $ ${bindir:=.}/c-dht-kademlia ${platfdir}/cluster_backbone.xml ${srcdir:=.}/dht-
 > [  0.000000] (11:node@node-10.simgrid.org) Hi, I'm going to join the network with id 1023
 > [  0.000000] (12:node@node-11.simgrid.org) Hi, I'm going to join the network with id 2047
 > [  0.000000] (13:node@node-12.simgrid.org) Hi, I'm going to join the network with id 4095
-> [780.000000] ( 7:node@node-6.simgrid.org) 5/5 FIND_NODE have succeeded
-> [780.000000] ( 9:node@node-8.simgrid.org) 6/6 FIND_NODE have succeeded
-> [780.000000] ( 3:node@node-2.simgrid.org) 5/5 FIND_NODE have succeeded
+> [780.000000] ( 9:node@node-8.simgrid.org) 5/5 FIND_NODE have succeeded
+> [780.000000] ( 3:node@node-2.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 2:node@node-1.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] (11:node@node-10.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 1:node@node-0.simgrid.org) 7/7 FIND_NODE have succeeded
-> [780.000000] ( 5:node@node-4.simgrid.org) 6/6 FIND_NODE have succeeded
+> [780.000000] ( 5:node@node-4.simgrid.org) 5/5 FIND_NODE have succeeded
+> [780.000000] ( 6:node@node-5.simgrid.org) 6/6 FIND_NODE have succeeded
+> [780.000000] ( 7:node@node-6.simgrid.org) 5/5 FIND_NODE have succeeded
 > [780.000000] (13:node@node-12.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 8:node@node-7.simgrid.org) 5/5 FIND_NODE have succeeded
-> [780.000000] ( 6:node@node-5.simgrid.org) 5/5 FIND_NODE have succeeded
 > [780.000000] (10:node@node-9.simgrid.org) 5/5 FIND_NODE have succeeded
 > [780.000000] (12:node@node-11.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 4:node@node-3.simgrid.org) 5/5 FIND_NODE have succeeded

diff --git a/examples/cpp/comm-ready/s4u-comm-ready.tesh b/examples/cpp/comm-ready/s4u-comm-ready.tesh
index 17e76f5..db7cb0f 100644 (file)
--- a/examples/cpp/comm-ready/s4u-comm-ready.tesh
+++ b/examples/cpp/comm-ready/s4u-comm-ready.tesh
@@ -5,14 +5,14 @@ p Test1 Peer sending and receiving
 $ ${bindir:=.}/s4u-comm-ready ${platfdir}/small_platform_fatpipe.xml s4u-comm-ready_d.xml "--log=root.fmt:[%10.6r]%e(%i:%a@%h)%e%m%n"
 > [  0.000000] (1:peer@Tremblay) Send 'Message 0 from peer 0' to 'peer-1'
 > [  0.000000] (2:peer@Ruby) Send 'Message 0 from peer 1' to 'peer-0'
+> [  0.000000] (3:peer@Perl) Send 'finalize' to 'peer-0'
 > [  0.000000] (1:peer@Tremblay) Send 'Message 0 from peer 0' to 'peer-2'
 > [  0.000000] (2:peer@Ruby) Send 'Message 0 from peer 1' to 'peer-2'
-> [  0.000000] (3:peer@Perl) Send 'finalize' to 'peer-0'
-> [  0.000000] (1:peer@Tremblay) Send 'Message 1 from peer 0' to 'peer-1'
-> [  0.000000] (2:peer@Ruby) Send 'Message 1 from peer 1' to 'peer-0'
 > [  0.000000] (3:peer@Perl) Send 'finalize' to 'peer-1'
 > [  0.000000] (3:peer@Perl) Done dispatching all messages
 > [  0.000000] (3:peer@Perl) Nothing ready to consume yet, I better sleep for a while
+> [  0.000000] (1:peer@Tremblay) Send 'Message 1 from peer 0' to 'peer-1'
+> [  0.000000] (2:peer@Ruby) Send 'Message 1 from peer 1' to 'peer-0'
 > [  0.000000] (1:peer@Tremblay) Send 'Message 1 from peer 0' to 'peer-2'
 > [  0.000000] (2:peer@Ruby) Send 'Message 1 from peer 1' to 'peer-2'
 > [  0.000000] (2:peer@Ruby) Send 'Message 2 from peer 1' to 'peer-0'

diff --git a/examples/cpp/comm-waitall/s4u-comm-waitall.tesh b/examples/cpp/comm-waitall/s4u-comm-waitall.tesh
index 2f662a7..1bd3471 100644 (file)
--- a/examples/cpp/comm-waitall/s4u-comm-waitall.tesh
+++ b/examples/cpp/comm-waitall/s4u-comm-waitall.tesh
@@ -1,9 +1,9 @@
 #!/usr/bin/env tesh
 
 $ ${bindir:=.}/s4u-comm-waitall ${platfdir}/small_platform_fatpipe.xml s4u-comm-waitall_d.xml "--log=root.fmt:[%10.6r]%e(%i:%a@%h)%e%m%n"
-> [  0.000000] (1:sender@Tremblay) Send 'Message 0' to 'receiver-0'
 > [  0.000000] (2:receiver@Ruby) Wait for my first message
 > [  0.000000] (3:receiver@Perl) Wait for my first message
+> [  0.000000] (1:sender@Tremblay) Send 'Message 0' to 'receiver-0'
 > [  0.000000] (1:sender@Tremblay) Send 'Message 1' to 'receiver-1'
 > [  0.000000] (1:sender@Tremblay) Send 'Message 2' to 'receiver-0'
 > [  0.000000] (1:sender@Tremblay) Send 'Message 3' to 'receiver-1'

diff --git a/examples/cpp/dht-kademlia/s4u-dht-kademlia.tesh b/examples/cpp/dht-kademlia/s4u-dht-kademlia.tesh
index 09586c2..bb1d4f9 100644 (file)
--- a/examples/cpp/dht-kademlia/s4u-dht-kademlia.tesh
+++ b/examples/cpp/dht-kademlia/s4u-dht-kademlia.tesh
@@ -18,15 +18,15 @@ $ ${bindir:=.}/s4u-dht-kademlia ${platfdir}/cluster_backbone.xml ${srcdir:=.}/s4
 > [  0.000000] (12:node@node-11.simgrid.org) Hi, I'm going to join the network with id 2047
 > [  0.000000] (13:node@node-12.simgrid.org) Hi, I'm going to join the network with id 4095
 > [780.000000] ( 7:node@node-6.simgrid.org) 5/5 FIND_NODE have succeeded
-> [780.000000] ( 9:node@node-8.simgrid.org) 6/6 FIND_NODE have succeeded
-> [780.000000] ( 3:node@node-2.simgrid.org) 5/5 FIND_NODE have succeeded
+> [780.000000] ( 9:node@node-8.simgrid.org) 5/5 FIND_NODE have succeeded
+> [780.000000] ( 3:node@node-2.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 2:node@node-1.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] (11:node@node-10.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 1:node@node-0.simgrid.org) 7/7 FIND_NODE have succeeded
-> [780.000000] ( 5:node@node-4.simgrid.org) 6/6 FIND_NODE have succeeded
+> [780.000000] ( 5:node@node-4.simgrid.org) 5/5 FIND_NODE have succeeded
 > [780.000000] (13:node@node-12.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 8:node@node-7.simgrid.org) 5/5 FIND_NODE have succeeded
-> [780.000000] ( 6:node@node-5.simgrid.org) 5/5 FIND_NODE have succeeded
+> [780.000000] ( 6:node@node-5.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] (10:node@node-9.simgrid.org) 5/5 FIND_NODE have succeeded
 > [780.000000] (12:node@node-11.simgrid.org) 6/6 FIND_NODE have succeeded
 > [780.000000] ( 4:node@node-3.simgrid.org) 5/5 FIND_NODE have succeeded

diff --git a/examples/smpi/smpi_s4u_masterworker/s4u_smpi.tesh b/examples/smpi/smpi_s4u_masterworker/s4u_smpi.tesh
index a419849..d4023e7 100644 (file)
--- a/examples/smpi/smpi_s4u_masterworker/s4u_smpi.tesh
+++ b/examples/smpi/smpi_s4u_masterworker/s4u_smpi.tesh
@@ -2,14 +2,14 @@ p Test the use of SMPI+MSG in the same file, as well as several different SMPI i
 $ ./masterworker_mailbox_smpi ${srcdir:=.}/../../platforms/small_platform_with_routers.xml ${srcdir:=.}/deployment_masterworker_mailbox_smpi.xml --log=smpi.:info --cfg=smpi/simulate-computation:no
 > [0.000000] [xbt_cfg/INFO] Configuration change: Set 'smpi/simulate-computation' to 'no'
 > [0.000000] [smpi_config/INFO] You did not set the power of the host running the simulation.  The timings will certainly not be accurate.  Use the option "--cfg=smpi/host-speed:<flops>" to set its value.  Check https://simgrid.org/doc/latest/Configuring_SimGrid.html#automatic-benchmarking-of-smpi-code for more information.
-> [Tremblay:master:(1) 0.000000] [msg_test/INFO] Got 2 workers and 20 tasks to process
-> [Tremblay:master:(1) 0.000000] [msg_test/INFO] Sending task 0 of 20 to mailbox 'Ginette'
 > [Ginette:master_mpi:(4) 0.000000] [msg_test/INFO] here for rank 0
 > [Bourassa:master_mpi:(5) 0.000000] [msg_test/INFO] here for rank 1
 > [Ginette:alltoall_mpi:(6) 0.000000] [msg_test/INFO] alltoall for rank 0
 > [Bourassa:alltoall_mpi:(7) 0.000000] [msg_test/INFO] alltoall for rank 1
 > [Jupiter:alltoall_mpi:(8) 0.000000] [msg_test/INFO] alltoall for rank 2
 > [Fafard:alltoall_mpi:(9) 0.000000] [msg_test/INFO] alltoall for rank 3
+> [Tremblay:master:(1) 0.000000] [msg_test/INFO] Got 2 workers and 20 tasks to process
+> [Tremblay:master:(1) 0.000000] [msg_test/INFO] Sending task 0 of 20 to mailbox 'Ginette'
 > [Ginette:master_mpi:(4) 0.000000] [msg_test/INFO] After comm 0
 > [Ginette:master_mpi:(4) 0.000000] [msg_test/INFO] After finalize 0 0
 > [Bourassa:master_mpi:(5) 0.016871] [msg_test/INFO] After comm 1

diff --git a/include/simgrid/s4u/Engine.hpp b/include/simgrid/s4u/Engine.hpp
index 2dfacd3..1519dd9 100644 (file)
--- a/include/simgrid/s4u/Engine.hpp
+++ b/include/simgrid/s4u/Engine.hpp
@@ -121,6 +121,8 @@ public:
   Link* link_by_name(const std::string& name) const;
   Link* link_by_name_or_null(const std::string& name) const;
 
+  Mailbox* mailbox_by_name_or_create(const std::string& name) const;
+
   size_t get_actor_count() const;
   std::vector<ActorPtr> get_all_actors() const;
   std::vector<ActorPtr> get_filtered_actors(const std::function<bool(ActorPtr)>& filter) const;

diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp
index 926bdb9..97e95b0 100644 (file)
--- a/src/kernel/EngineImpl.cpp
+++ b/src/kernel/EngineImpl.cpp
@@ -50,7 +50,10 @@ EngineImpl::~EngineImpl()
     if (kv.second)
       kv.second->destroy();
 
-      /* Free the remaining data structures */
+  for (auto const& kv : mailboxes_)
+    delete kv.second;
+
+    /* Free the remaining data structures */
 #if SIMGRID_HAVE_MC
   xbt_dynar_free(&actors_vector_);
   xbt_dynar_free(&dead_actors_vector_);

diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp
index ebf440f..11c6e3d 100644 (file)
--- a/src/kernel/EngineImpl.hpp
+++ b/src/kernel/EngineImpl.hpp
@@ -35,6 +35,8 @@ class EngineImpl {
   std::map<std::string, s4u::Host*, std::less<>> hosts_;
   std::map<std::string, resource::LinkImpl*, std::less<>> links_;
   std::unordered_map<std::string, routing::NetPoint*> netpoints_;
+  std::unordered_map<std::string, activity::MailboxImpl*> mailboxes_;
+
   std::unordered_map<std::string, actor::ActorCodeFactory> registered_functions; // Maps function names to actor code
   actor::ActorCodeFactory default_function; // Function to use as a fallback when the provided name matches nothing
   std::vector<resource::Model*> models_;

diff --git a/src/kernel/activity/MailboxImpl.cpp b/src/kernel/activity/MailboxImpl.cpp
index 7cb3676..0403639 100644 (file)
--- a/src/kernel/activity/MailboxImpl.cpp
+++ b/src/kernel/activity/MailboxImpl.cpp
@@ -10,15 +10,6 @@
 
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(simix_mailbox, simix, "Mailbox implementation");
 
-static std::unordered_map<std::string, smx_mailbox_t> mailboxes;
-
-void SIMIX_mailbox_exit()
-{
-  for (auto const& elm : mailboxes)
-    delete elm.second;
-  mailboxes.clear();
-}
-
 /******************************************************************************/
 /*                           Rendez-Vous Points                               */
 /******************************************************************************/
@@ -26,29 +17,7 @@ void SIMIX_mailbox_exit()
 namespace simgrid {
 namespace kernel {
 namespace activity {
-/** @brief Returns the mailbox of that name, or nullptr */
-MailboxImpl* MailboxImpl::by_name_or_null(const std::string& name)
-{
-  auto mbox = mailboxes.find(name);
-  if (mbox != mailboxes.end())
-    return mbox->second;
-  else
-    return nullptr;
-}
 
-/** @brief Returns the mailbox of that name, newly created on need */
-MailboxImpl* MailboxImpl::by_name_or_create(const std::string& name)
-{
-  /* two actors may have pushed the same mbox_create simcall at the same time */
-  auto m = mailboxes.find(name);
-  if (m == mailboxes.end()) {
-    auto* mbox = new MailboxImpl(name);
-    XBT_DEBUG("Creating a mailbox at %p with name %s", mbox, name.c_str());
-    mailboxes[name] = mbox;
-    return mbox;
-  } else
-    return m->second;
-}
 /** @brief set the receiver of the mailbox to allow eager sends
  *  @param actor The receiving dude
  */

diff --git a/src/kernel/activity/MailboxImpl.hpp b/src/kernel/activity/MailboxImpl.hpp
index bacbd4d..14a03d3 100644 (file)
--- a/src/kernel/activity/MailboxImpl.hpp
+++ b/src/kernel/activity/MailboxImpl.hpp
@@ -9,6 +9,7 @@
 #include <boost/circular_buffer.hpp>
 #include <xbt/string.hpp>
 
+#include "simgrid/s4u/Engine.hpp"
 #include "simgrid/s4u/Mailbox.hpp"
 #include "src/kernel/activity/CommImpl.hpp"
 #include "src/kernel/actor/ActorImpl.hpp"
@@ -25,6 +26,8 @@ class MailboxImpl {
   s4u::Mailbox piface_;
   xbt::string name_;
 
+  friend s4u::Engine;
+  friend s4u::Mailbox* s4u::Engine::mailbox_by_name_or_create(const std::string& name) const;
   friend s4u::Mailbox;
   friend s4u::Mailbox* s4u::Mailbox::by_name(const std::string& name);
   friend mc::CommunicationDeterminismChecker;
@@ -32,10 +35,12 @@ class MailboxImpl {
   explicit MailboxImpl(const std::string& name) : piface_(this), name_(name) {}
 
 public:
+  /** @brief Public interface */
+  const s4u::Mailbox* get_iface() const { return &piface_; }
+  s4u::Mailbox* get_iface() { return &piface_; }
+
   const xbt::string& get_name() const { return name_; }
   const char* get_cname() const { return name_.c_str(); }
-  static MailboxImpl* by_name_or_null(const std::string& name);
-  static MailboxImpl* by_name_or_create(const std::string& name);
   void set_receiver(s4u::ActorPtr actor);
   void push(CommImplPtr comm);
   void remove(const CommImplPtr& comm);
@@ -52,6 +57,4 @@ public:
 } // namespace kernel
 } // namespace simgrid
 
-XBT_PRIVATE void SIMIX_mailbox_exit();
-
 #endif

diff --git a/src/s4u/s4u_Engine.cpp b/src/s4u/s4u_Engine.cpp
index f802694..f664690 100644 (file)
--- a/src/s4u/s4u_Engine.cpp
+++ b/src/s4u/s4u_Engine.cpp
@@ -242,6 +242,23 @@ Link* Engine::link_by_name_or_null(const std::string& name) const
   return link == pimpl->links_.end() ? nullptr : link->second->get_iface();
 }
 
+/** @brief Find a mailox from its name or create one if it does not exist) */
+Mailbox* Engine::mailbox_by_name_or_create(const std::string& name) const
+{
+  /* two actors may have pushed the same mbox_create simcall at the same time */
+  kernel::activity::MailboxImpl* mbox = kernel::actor::simcall([&name, this] {
+    auto m = pimpl->mailboxes_.find(name);
+    if (m == pimpl->mailboxes_.end()) {
+      auto* mbox = new kernel::activity::MailboxImpl(name);
+      XBT_DEBUG("Creating a mailbox at %p with name %s", mbox, name.c_str());
+      pimpl->mailboxes_[name] = mbox;
+      return mbox;
+    } else
+      return m->second;
+  });
+  return mbox->get_iface();
+}
+
 void Engine::link_register(const std::string& name, const Link* link)
 {
   pimpl->links_[name] = link->get_impl();

diff --git a/src/s4u/s4u_Mailbox.cpp b/src/s4u/s4u_Mailbox.cpp
index 7aa76ec..23b140f 100644 (file)
--- a/src/s4u/s4u_Mailbox.cpp
+++ b/src/s4u/s4u_Mailbox.cpp
@@ -4,6 +4,7 @@
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
 #include "simgrid/s4u/Comm.hpp"
+#include "simgrid/s4u/Engine.hpp"
 #include "simgrid/s4u/Mailbox.hpp"
 #include "src/kernel/activity/MailboxImpl.hpp"
 
@@ -27,11 +28,7 @@ const char* Mailbox::get_cname() const
 
 Mailbox* Mailbox::by_name(const std::string& name)
 {
-  kernel::activity::MailboxImpl* mbox = kernel::activity::MailboxImpl::by_name_or_null(name);
-  if (mbox == nullptr) {
-    mbox = kernel::actor::simcall([&name] { return kernel::activity::MailboxImpl::by_name_or_create(name); });
-  }
-  return &mbox->piface_;
+  return Engine::get_instance()->mailbox_by_name_or_create(name);
 }
 
 bool Mailbox::empty() const

diff --git a/src/simix/smx_global.cpp b/src/simix/smx_global.cpp
index 5831192..7adea1a 100644 (file)
--- a/src/simix/smx_global.cpp
+++ b/src/simix/smx_global.cpp
@@ -233,10 +233,6 @@ void SIMIX_clean()
   engine->run_all_actors();
   engine->empty_trash();
 
-  /* Exit the SIMIX network module */
-  SIMIX_mailbox_exit();
-
-
   /* Let's free maestro now */
   delete simix_global->maestro_;
   simix_global->maestro_ = nullptr;