From: SUTER Frederic Date: Tue, 25 May 2021 14:13:40 +0000 (+0200) Subject: Change way Mailboxes are create, stored, and destroyed X-Git-Tag: v3.28~245 X-Git-Url: http://bilbo.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/0286ae4d1e43c141818b7456b47fd2855643f0e6 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 594a6e7d19..e11940f326 100644 --- 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 036e0bb6e1..ba902efe70 100644 --- 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 17e76f55ad..db7cb0f364 100644 --- 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 2f662a7c8b..1bd34718de 100644 --- 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 09586c26e1..bb1d4f990c 100644 --- 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 a4198498bb..d4023e727d 100644 --- 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:" 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 2dfacd3f19..1519dd9a86 100644 --- 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 get_all_actors() const; std::vector get_filtered_actors(const std::function& filter) const; diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp index 926bdb9288..97e95b0cef 100644 --- 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 ebf440fb3c..11c6e3d187 100644 --- a/src/kernel/EngineImpl.hpp +++ b/src/kernel/EngineImpl.hpp @@ -35,6 +35,8 @@ class EngineImpl { std::map> hosts_; std::map> links_; std::unordered_map netpoints_; + std::unordered_map mailboxes_; + std::unordered_map 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 models_; diff --git a/src/kernel/activity/MailboxImpl.cpp b/src/kernel/activity/MailboxImpl.cpp index 7cb3676fba..04036397c0 100644 --- 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 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 bacbd4de94..14a03d3882 100644 --- a/src/kernel/activity/MailboxImpl.hpp +++ b/src/kernel/activity/MailboxImpl.hpp @@ -9,6 +9,7 @@ #include #include +#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 f802694d46..f664690b5c 100644 --- 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 7aa76eccd1..23b140ff78 100644 --- 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 5831192050..7adea1a9aa 100644 --- 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;