From e4c3e60acadc7b5ca6238866641567bae9539067 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Mon, 9 May 2016 22:18:37 +0200 Subject: [PATCH] simix::Synchro: Factorize the refcounting for child classes --- src/simix/Synchro.cpp | 15 ++++++++++++ src/simix/Synchro.h | 5 ++++ src/simix/SynchroComm.cpp | 48 ++++++++++++++++----------------------- src/simix/SynchroComm.hpp | 3 +-- src/simix/SynchroExec.cpp | 17 ++++---------- src/simix/SynchroExec.hpp | 4 +--- src/simix/smx_network.cpp | 16 ++++++------- src/simix/smx_process.cpp | 2 +- 8 files changed, 54 insertions(+), 56 deletions(-) diff --git a/src/simix/Synchro.cpp b/src/simix/Synchro.cpp index c10c46c616..9d94aba497 100644 --- a/src/simix/Synchro.cpp +++ b/src/simix/Synchro.cpp @@ -13,3 +13,18 @@ simgrid::simix::Synchro::~Synchro() { xbt_fifo_free(simcalls); xbt_free(name); } + +void simgrid::simix::Synchro::ref() +{ + refcount++; +} +void simgrid::simix::Synchro::unref() +{ + xbt_assert(refcount > 0, + "This synchro has a negative refcount! You can only call test() or wait() once per synchronization."); + + refcount--; + if (refcount>0) + return; + delete this; +} diff --git a/src/simix/Synchro.h b/src/simix/Synchro.h index d119a8d251..dc5a9496e9 100644 --- a/src/simix/Synchro.h +++ b/src/simix/Synchro.h @@ -25,6 +25,11 @@ namespace simix { virtual void suspend()=0; virtual void resume()=0; + + void ref(); + void unref(); + private: + int refcount=1; }; }} // namespace simgrid::simix #else /* not C++ */ diff --git a/src/simix/SynchroComm.cpp b/src/simix/SynchroComm.cpp index 5a5ccb5a52..b074baa3aa 100644 --- a/src/simix/SynchroComm.cpp +++ b/src/simix/SynchroComm.cpp @@ -14,12 +14,30 @@ XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(simix_network); simgrid::simix::Comm::Comm(e_smx_comm_type_t _type) { state = SIMIX_WAITING; this->type = _type; - refcount = 1; src_data=NULL; dst_data=NULL; XBT_DEBUG("Create communicate synchro %p", this); } + +simgrid::simix::Comm::~Comm() +{ + XBT_DEBUG("Really free communication %p", this); + + cleanupSurf(); + + if (detached && state != SIMIX_DONE) { + /* the communication has failed and was detached: + * we have to free the buffer */ + if (clean_fun) + clean_fun(src_buff); + src_buff = NULL; + } + + if(mbox) + SIMIX_mbox_remove(mbox, this); + +} void simgrid::simix::Comm::suspend() { /* FIXME: shall we suspend also the timeout synchro? */ @@ -73,34 +91,6 @@ double simgrid::simix::Comm::remains() } } -void simgrid::simix::Comm::unref() -{ - XBT_DEBUG("Destroy synchro %p (refcount: %d), state: %d", this, refcount, (int)state); - - xbt_assert(refcount > 0, - "This comm has a negative refcount! You must not call test() or wait() more than once on a given communication."); - - refcount--; - if (refcount > 0) - return; - XBT_DEBUG("Really free communication %p; refcount is now %d", this, refcount); - - cleanupSurf(); - - if (detached && state != SIMIX_DONE) { - /* the communication has failed and was detached: - * we have to free the buffer */ - if (clean_fun) - clean_fun(src_buff); - src_buff = NULL; - } - - if(mbox) - SIMIX_mbox_remove(mbox, this); - - delete this; -} - /** @brief This is part of the cleanup process, probably an internal command */ void simgrid::simix::Comm::cleanupSurf() { diff --git a/src/simix/SynchroComm.hpp b/src/simix/SynchroComm.hpp index 0f85b832cf..6d4d7b7ba5 100644 --- a/src/simix/SynchroComm.hpp +++ b/src/simix/SynchroComm.hpp @@ -20,13 +20,13 @@ namespace simgrid { namespace simix { XBT_PUBLIC_CLASS Comm : public Synchro { + ~Comm(); public: Comm(e_smx_comm_type_t type); void suspend(); void resume(); void cancel(); double remains(); - void unref(); void cleanupSurf(); // FIXME: make me protected e_smx_comm_type_t type; /* Type of the communication (SIMIX_COMM_SEND or SIMIX_COMM_RECEIVE) */ @@ -37,7 +37,6 @@ namespace simix { (comm.mbox set to NULL when the communication is removed from the mailbox (used as garbage collector)) */ #endif - int refcount = 1; /* Number of processes involved in the cond */ bool detached = false; /* If detached or not */ void (*clean_fun)(void*); /* Function to clean the detached src_buf if something goes wrong */ diff --git a/src/simix/SynchroExec.cpp b/src/simix/SynchroExec.cpp index 02ce12513a..e206b73f44 100644 --- a/src/simix/SynchroExec.cpp +++ b/src/simix/SynchroExec.cpp @@ -6,6 +6,11 @@ #include "src/simix/SynchroExec.hpp" #include "src/surf/surf_interface.hpp" +simgrid::simix::Exec::~Exec() +{ + if (surf_exec) + surf_exec->unref(); +} void simgrid::simix::Exec::suspend() { if (surf_exec) @@ -25,15 +30,3 @@ double simgrid::simix::Exec::remains() return 0; } - -void simgrid::simix::Exec::unref() -{ - refcount--; - if (refcount > 0) - return; - - if (surf_exec) - surf_exec->unref(); - - delete this; -} diff --git a/src/simix/SynchroExec.hpp b/src/simix/SynchroExec.hpp index 7466a86425..7904cfe69d 100644 --- a/src/simix/SynchroExec.hpp +++ b/src/simix/SynchroExec.hpp @@ -13,16 +13,14 @@ namespace simgrid { namespace simix { XBT_PUBLIC_CLASS Exec : public Synchro { + ~Exec(); public: void suspend(); void resume(); double remains(); - void unref(); sg_host_t host; /* The host where the execution takes place */ surf_action_t surf_exec; /* The Surf execution action encapsulated */ - private: - int refcount = 1; }; }} // namespace simgrid::simix diff --git a/src/simix/smx_network.cpp b/src/simix/smx_network.cpp index 40144d40ff..af29a4c42a 100644 --- a/src/simix/smx_network.cpp +++ b/src/simix/smx_network.cpp @@ -137,7 +137,7 @@ static smx_synchro_t _find_matching_comm(std::deque *deque, e_smx XBT_DEBUG("Found a matching communication synchro %p", comm); if (remove_matching) deque->erase(it); - comm->refcount++; + comm->ref(); #if HAVE_MC comm->mbox_cpy = comm->mbox; #endif @@ -197,7 +197,7 @@ smx_synchro_t simcall_HANDLER_comm_isend(smx_simcall_t simcall, smx_process_t sr //this mailbox is for small messages, which have to be sent right now other_synchro->state = SIMIX_READY; other_comm->dst_proc=mbox->permanent_receiver; - other_comm->refcount++; + other_comm->ref(); mbox->done_comm_queue->push_back(other_synchro); other_comm->mbox=mbox; XBT_DEBUG("pushing a message into the permanent receive fifo %p, comm %p", mbox, &(other_comm)); @@ -218,8 +218,8 @@ smx_synchro_t simcall_HANDLER_comm_isend(smx_simcall_t simcall, smx_process_t sr /* if the communication synchro is detached then decrease the refcount * by one, so it will be eliminated by the receiver's destroy call */ if (detached) { - other_comm->detached = 1; - other_comm->refcount--; + other_comm->detached = true; + other_comm->ref(); other_comm->clean_fun = clean_fun; } else { other_comm->clean_fun = NULL; @@ -295,7 +295,7 @@ smx_synchro_t SIMIX_comm_irecv(smx_process_t dst_proc, smx_mailbox_t mbox, void other_comm->type = SIMIX_COMM_DONE; other_comm->mbox = NULL; } - other_comm->refcount--; + other_comm->unref(); static_cast(this_synchro)->unref(); } } else { @@ -374,10 +374,8 @@ smx_synchro_t SIMIX_comm_iprobe(smx_process_t dst_proc, smx_mailbox_t mbox, int other_synchro = _find_matching_comm(mbox->comm_queue, (e_smx_comm_type_t) smx_type, match_fun, data, this_comm,/*remove_matching*/false); } - if(other_synchro) { - simgrid::simix::Comm *other_comm = static_cast(other_synchro); - other_comm->refcount--; - } + if(other_synchro) + other_synchro->unref(); this_comm->unref(); return other_synchro; diff --git a/src/simix/smx_process.cpp b/src/simix/smx_process.cpp index b6c82b29f4..0d46b87398 100644 --- a/src/simix/smx_process.cpp +++ b/src/simix/smx_process.cpp @@ -91,7 +91,7 @@ void SIMIX_process_cleanup(smx_process_t process) comm, (int)comm->state, comm->src_proc, comm->dst_proc); comm->dst_proc = NULL; - if (comm->detached && comm->refcount == 1 && comm->src_proc != NULL) { + if (comm->detached && /* FIXME: This code should be moved within comm->unref() anyway. comm->refcount == 1 &&*/ comm->src_proc != NULL) { /* the comm will be freed right now, remove it from the sender */ xbt_fifo_remove(comm->src_proc->comms, comm); } -- 2.20.1