From b67d191c3627cb8e64017cfedcbbbafe9f4dfcc3 Mon Sep 17 00:00:00 2001 From: Gabriel Corona Date: Thu, 16 Jun 2016 09:37:17 +0200 Subject: [PATCH] [simix] Create actors by value (instead of new()-ing and leaking them) --- examples/s4u/basic/s4u_basic.cpp | 4 ++-- examples/s4u/io/s4u_io.cpp | 2 +- examples/s4u/mutex/s4u_mutex.cpp | 14 +++++-------- include/simgrid/s4u/actor.hpp | 36 ++++++++++++++++++++++++-------- src/s4u/s4u_actor.cpp | 8 ++----- 5 files changed, 37 insertions(+), 27 deletions(-) diff --git a/examples/s4u/basic/s4u_basic.cpp b/examples/s4u/basic/s4u_basic.cpp index 991b3f7e7f..c08cd6dbb4 100644 --- a/examples/s4u/basic/s4u_basic.cpp +++ b/examples/s4u/basic/s4u_basic.cpp @@ -34,8 +34,8 @@ public: int main(int argc, char **argv) { simgrid::s4u::Engine *e = new simgrid::s4u::Engine(&argc,argv); e->loadPlatform("../../platforms/two_hosts.xml"); - new simgrid::s4u::Actor("worker", simgrid::s4u::Host::by_name("Tremblay"), Worker()); - new simgrid::s4u::Actor("master", simgrid::s4u::Host::by_name("Jupiter"), 0, Master()); + simgrid::s4u::Actor("worker", simgrid::s4u::Host::by_name("Tremblay"), Worker()); + simgrid::s4u::Actor("master", simgrid::s4u::Host::by_name("Jupiter"), 0, Master()); e->run(); return 0; } diff --git a/examples/s4u/io/s4u_io.cpp b/examples/s4u/io/s4u_io.cpp index c4494a3035..b3aaa11a71 100644 --- a/examples/s4u/io/s4u_io.cpp +++ b/examples/s4u/io/s4u_io.cpp @@ -104,7 +104,7 @@ int main(int argc, char **argv) { simgrid::s4u::Engine *e = new simgrid::s4u::Engine(&argc,argv); e->loadPlatform("../../platforms/storage/storage.xml"); - new simgrid::s4u::Actor("host", simgrid::s4u::Host::by_name("denise"), MyHost()); + simgrid::s4u::Actor("host", simgrid::s4u::Host::by_name("denise"), MyHost()); e->run(); return 0; } diff --git a/examples/s4u/mutex/s4u_mutex.cpp b/examples/s4u/mutex/s4u_mutex.cpp index f46efa9fc4..3d0ba54f62 100644 --- a/examples/s4u/mutex/s4u_mutex.cpp +++ b/examples/s4u/mutex/s4u_mutex.cpp @@ -39,7 +39,7 @@ public: // This class is an example of how to use lock_guard with simgrid mutex class WorkerLockGuard { simgrid::s4u::Mutex mutex_; -int *results_; + int *results_; public: WorkerLockGuard(int *res, simgrid::s4u::Mutex mutex) : mutex_(std::move(mutex)), results_(res) {}; @@ -63,24 +63,20 @@ public: void operator()() { int res = 0; simgrid::s4u::Mutex mutex; - simgrid::s4u::Actor* workers[NB_ACTOR*2]; + simgrid::s4u::Actor workers[NB_ACTOR*2]; for (int i = 0; i < NB_ACTOR * 2 ; i++) { // To create a worker use the static method simgrid::s4u::Actor. if((i % 2) == 0 ) - workers[i] = new simgrid::s4u::Actor("worker", + workers[i] = simgrid::s4u::Actor("worker", simgrid::s4u::Host::by_name("Jupiter"), WorkerLockGuard(&res, mutex)); else - workers[i] = new simgrid::s4u::Actor("worker", + workers[i] = simgrid::s4u::Actor("worker", simgrid::s4u::Host::by_name("Tremblay"), Worker(&res, mutex)); } - for (int i = 0; i < NB_ACTOR ; i++) { - delete workers[i]; - } - simgrid::s4u::this_actor::sleep(10); XBT_INFO("Results is -> %d", res); } @@ -90,7 +86,7 @@ public: int main(int argc, char **argv) { simgrid::s4u::Engine *e = new simgrid::s4u::Engine(&argc,argv); e->loadPlatform("../../platforms/two_hosts.xml"); - new simgrid::s4u::Actor("main", simgrid::s4u::Host::by_name("Tremblay"), 0, MainActor()); + simgrid::s4u::Actor("main", simgrid::s4u::Host::by_name("Tremblay"), 0, MainActor()); e->run(); return 0; } diff --git a/include/simgrid/s4u/actor.hpp b/include/simgrid/s4u/actor.hpp index 24728db6a9..aa5877d5d1 100644 --- a/include/simgrid/s4u/actor.hpp +++ b/include/simgrid/s4u/actor.hpp @@ -48,7 +48,7 @@ namespace s4u { * }; * * // From your main or from another actor, create your actor on the host Jupiter - * new Actor("worker", simgrid::s4u::Host::by_name("Jupiter"), worker); + * Actor("worker", simgrid::s4u::Host::by_name("Jupiter"), worker); * @endcode * * But some people prefer to encapsulate their actors in classes and @@ -68,7 +68,7 @@ namespace s4u { * }; * * // From your main or from another actor, create your actor. Note the () after Worker - * new Actor("worker", simgrid::s4u::Host::by_name("Jupiter"), Worker()); + * Actor("worker", simgrid::s4u::Host::by_name("Jupiter"), Worker()); * @endcode * * @section s4u_actor_flesh Fleshing your actor @@ -117,15 +117,38 @@ namespace s4u { /** @brief Simulation Agent (see \ref s4u_actor)*/ XBT_PUBLIC_CLASS Actor { - explicit Actor(smx_process_t smx_proc); public: + Actor() : pimpl_(nullptr) {} + Actor(smx_process_t smx_proc) : + pimpl_(SIMIX_process_ref(smx_proc)) {} + ~Actor() + { + SIMIX_process_unref(pimpl_); + } + + // Copy+move (with the copy-and-swap idiom): + Actor(Actor const& actor) : pimpl_(SIMIX_process_ref(actor.pimpl_)) {} + friend void swap(Actor& first, Actor& second) + { + using std::swap; + swap(first.pimpl_, second.pimpl_); + } + Actor& operator=(Actor actor) + { + swap(*this, actor); + return *this; + } + Actor(Actor&& actor) : pimpl_(nullptr) + { + swap(*this, actor); + } + Actor(const char* name, s4u::Host *host, double killTime, std::function code); Actor(const char* name, s4u::Host *host, std::function code) : Actor(name, host, -1, std::move(code)) {}; template Actor(const char* name, s4u::Host *host, C code) : Actor(name, host, -1, std::function(std::move(code))) {} - ~Actor(); /** Retrieves the actor that have the given PID (or NULL if not existing) */ //static Actor *byPid(int pid); not implemented @@ -163,11 +186,6 @@ public: /** Ask kindly to all actors to die. Only the issuer will survive. */ static void killAll(); -protected: - smx_process_t getInferior() - { - return pimpl_; - } private: smx_process_t pimpl_ = nullptr; }; diff --git a/src/s4u/s4u_actor.cpp b/src/s4u/s4u_actor.cpp index 2170f7447e..d5d5e9963d 100644 --- a/src/s4u/s4u_actor.cpp +++ b/src/s4u/s4u_actor.cpp @@ -16,19 +16,15 @@ XBT_LOG_NEW_DEFAULT_CATEGORY(s4u_actor,"S4U actors"); using namespace simgrid; -s4u::Actor::Actor(smx_process_t smx_proc) : pimpl_(smx_proc) {} - s4u::Actor::Actor(const char* name, s4u::Host *host, double killTime, std::function code) { // TODO, when autorestart is used, the std::function is copied so the new // instance will get a fresh (reinitialized) state. Is this what we want? - this->pimpl_ = simcall_process_create( + this->pimpl_ = SIMIX_process_ref(simcall_process_create( name, std::move(code), nullptr, host->name().c_str(), - killTime, nullptr, 0); + killTime, nullptr, 0)); } -s4u::Actor::~Actor() {} - void s4u::Actor::join() { simcall_process_join(pimpl_, -1); } -- 2.20.1