From 94f511befd8ea7dea78746c1f6ae818a778e67e7 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 6 Jan 2019 14:21:53 +0100 Subject: [PATCH] change static fields in SwappedCtx into regular fields of the factory and other cleanups --- src/kernel/context/ContextBoost.cpp | 2 -- src/kernel/context/ContextRaw.cpp | 2 -- src/kernel/context/ContextRaw.hpp | 1 - src/kernel/context/ContextSwapped.cpp | 36 ++++++++------------------- src/kernel/context/ContextSwapped.hpp | 21 +++++++--------- src/kernel/context/ContextUnix.cpp | 2 -- src/kernel/context/ContextUnix.hpp | 1 - 7 files changed, 20 insertions(+), 45 deletions(-) diff --git a/src/kernel/context/ContextBoost.cpp b/src/kernel/context/ContextBoost.cpp index 49cbec80a8..9ae90ca5ae 100644 --- a/src/kernel/context/ContextBoost.cpp +++ b/src/kernel/context/ContextBoost.cpp @@ -58,8 +58,6 @@ BoostContext::~BoostContext() if (not this->stack_) delete this->fc_; #endif - if (this == get_maestro()) - set_maestro(nullptr); } void BoostContext::wrapper(BoostContext::arg_type arg) diff --git a/src/kernel/context/ContextRaw.cpp b/src/kernel/context/ContextRaw.cpp index fe8243da01..770a35cfe7 100644 --- a/src/kernel/context/ContextRaw.cpp +++ b/src/kernel/context/ContextRaw.cpp @@ -215,8 +215,6 @@ RawContext::RawContext(std::function code, void_pfn_smxprocess_t cleanup } } -RawContext::~RawContext() = default; - void RawContext::wrapper(void* arg) { RawContext* context = static_cast(arg); diff --git a/src/kernel/context/ContextRaw.hpp b/src/kernel/context/ContextRaw.hpp index b16fe7c612..c30e4ecf4d 100644 --- a/src/kernel/context/ContextRaw.hpp +++ b/src/kernel/context/ContextRaw.hpp @@ -29,7 +29,6 @@ class RawContext : public SwappedContext { public: RawContext(std::function code, void_pfn_smxprocess_t cleanup_func, smx_actor_t process, SwappedContextFactory* factory); - ~RawContext() override; void swap_into(SwappedContext* to) override; diff --git a/src/kernel/context/ContextSwapped.cpp b/src/kernel/context/ContextSwapped.cpp index fc2663efcc..50b96bb288 100644 --- a/src/kernel/context/ContextSwapped.cpp +++ b/src/kernel/context/ContextSwapped.cpp @@ -34,33 +34,20 @@ namespace simgrid { namespace kernel { namespace context { -/* Parallel execution */ -simgrid::xbt::Parmap* SwappedContext::parmap_; -std::atomic SwappedContext::threads_working_; /* number of threads that have started their work */ +/* rank of the execution thread */ thread_local uintptr_t SwappedContext::worker_id_; /* thread-specific storage for the thread id */ -std::vector SwappedContext::workers_context_; /* space to save the worker's context in each thread */ SwappedContextFactory::SwappedContextFactory(std::string name) : ContextFactory(name), parallel_(SIMIX_context_is_parallel()) { SwappedContext::set_maestro(nullptr); - SwappedContext::initialize(parallel_); -} -SwappedContextFactory::~SwappedContextFactory() -{ - SwappedContext::finalize(); -} - -void SwappedContext::initialize(bool parallel) -{ parmap_ = nullptr; // will be created lazily with the right parameters if needed (ie, in parallel) - if (parallel) { + if (parallel_) { workers_context_.clear(); workers_context_.resize(SIMIX_context_get_nthreads(), nullptr); } } - -void SwappedContext::finalize() +SwappedContextFactory::~SwappedContextFactory() { delete parmap_; parmap_ = nullptr; @@ -166,12 +153,11 @@ void SwappedContextFactory::run_all() * for the ones of the simulated processes that must run. */ if (parallel_) { - SwappedContext::threads_working_ = 0; + threads_working_ = 0; // We lazily create the parmap so that all options are actually processed when doing so. - if (SwappedContext::parmap_ == nullptr) - SwappedContext::parmap_ = - new simgrid::xbt::Parmap(SIMIX_context_get_nthreads(), SIMIX_context_get_parallel_mode()); + if (parmap_ == nullptr) + parmap_ = new simgrid::xbt::Parmap(SIMIX_context_get_nthreads(), SIMIX_context_get_parallel_mode()); // Usually, Parmap::apply() executes the provided function on all elements of the array. // Here, the executed function does not return the control to the parmap before all the array is processed: @@ -179,7 +165,7 @@ void SwappedContextFactory::run_all() // the control to the parmap. Instead, it uses parmap_->next() to steal another work, and does it directly. // It only yields back to worker_context when the work array is exhausted. // - So, resume() is only launched from the parmap for the first job of each minion. - SwappedContext::parmap_->apply( + parmap_->apply( [](smx_actor_t process) { SwappedContext* context = static_cast(process->context_); context->resume(); @@ -205,10 +191,10 @@ void SwappedContext::resume() { if (factory_->parallel_) { // Save the thread number (my body) in an os-thread-specific area - worker_id_ = threads_working_.fetch_add(1, std::memory_order_relaxed); + worker_id_ = factory_->threads_working_.fetch_add(1, std::memory_order_relaxed); // Save my current soul (either maestro, or one of the minions) in a permanent area SwappedContext* worker_context = static_cast(self()); - workers_context_[worker_id_] = worker_context; + factory_->workers_context_[worker_id_] = worker_context; // Switch my soul and the actor's one Context::set_current(this); worker_context->swap_into(this); @@ -234,7 +220,7 @@ void SwappedContext::suspend() { if (factory_->parallel_) { // Get some more work to directly swap into the next executable actor instead of yielding back to the parmap - boost::optional next_work = parmap_->next(); + boost::optional next_work = factory_->parmap_->next(); SwappedContext* next_context; if (next_work) { // There is a next soul to embody (ie, another executable actor) @@ -244,7 +230,7 @@ void SwappedContext::suspend() // All actors were run, go back to the parmap context XBT_DEBUG("No more processes to run"); // worker_id_ is the identity of my body, stored in thread_local when starting the scheduling round - next_context = workers_context_[worker_id_]; + next_context = factory_->workers_context_[worker_id_]; // When given that soul, the body will wait for the next scheduling round } diff --git a/src/kernel/context/ContextSwapped.hpp b/src/kernel/context/ContextSwapped.hpp index 2794f4a140..6e56bb166f 100644 --- a/src/kernel/context/ContextSwapped.hpp +++ b/src/kernel/context/ContextSwapped.hpp @@ -22,11 +22,15 @@ public: ~SwappedContextFactory() override; void run_all() override; -protected: +private: bool parallel_; -private: unsigned long process_index_ = 0; // Next actor to execute during sequential run_all() + + /* For the parallel execution */ + simgrid::xbt::Parmap* parmap_; + std::vector workers_context_; /* space to save the worker's context in each thread */ + std::atomic threads_working_; /* number of threads that have started their work */ }; class SwappedContext : public Context { @@ -35,22 +39,16 @@ public: SwappedContextFactory* factory); virtual ~SwappedContext(); - static void initialize(bool parallel); // Initialize the module, using the options - static void finalize(); // Finalize the module - void suspend() override; virtual void resume(); void stop() override; - virtual void swap_into(SwappedContext* to) = 0; // Defined in subclasses + virtual void swap_into(SwappedContext* to) = 0; // Defined in Raw, Boost and UContext subclasses static SwappedContext* get_maestro() { return maestro_context_; } static void set_maestro(SwappedContext* maestro) { maestro_context_ = maestro; } - /* For the parallel execution */ // FIXME killme - static simgrid::xbt::Parmap* parmap_; - static std::vector workers_context_; - static std::atomic threads_working_; + // FIXME: Killme static thread_local uintptr_t worker_id_; protected: @@ -58,8 +56,7 @@ protected: private: static SwappedContext* maestro_context_; - /* For sequential and parallel run_all() */ - SwappedContextFactory* factory_; + SwappedContextFactory* factory_; // for sequential and parallel run_all() }; } // namespace context diff --git a/src/kernel/context/ContextUnix.cpp b/src/kernel/context/ContextUnix.cpp index 35316d9cac..de85d2ff1a 100644 --- a/src/kernel/context/ContextUnix.cpp +++ b/src/kernel/context/ContextUnix.cpp @@ -66,8 +66,6 @@ UContext::UContext(std::function code, void_pfn_smxprocess_t cleanup_fun #endif } -UContext::~UContext() = default; - // The name of this function is currently hardcoded in the code (as string). // Do not change it without fixing those references as well. void UContext::smx_ctx_sysv_wrapper(int i1, int i2) diff --git a/src/kernel/context/ContextUnix.hpp b/src/kernel/context/ContextUnix.hpp index 00c4832144..e6961a8c97 100644 --- a/src/kernel/context/ContextUnix.hpp +++ b/src/kernel/context/ContextUnix.hpp @@ -28,7 +28,6 @@ class UContext : public SwappedContext { public: UContext(std::function code, void_pfn_smxprocess_t cleanup_func, smx_actor_t process, SwappedContextFactory* factory); - ~UContext() override; void swap_into(SwappedContext* to) override; -- 2.20.1