From ac080087b39ef79ff497d3992ff04b3e20fe40b2 Mon Sep 17 00:00:00 2001 From: Frederic Suter Date: Thu, 10 Oct 2019 12:16:28 +0200 Subject: [PATCH 1/1] use CRTP to factor refcounting across activity types --- include/simgrid/s4u/Activity.hpp | 12 ++++++++++++ include/simgrid/s4u/Comm.hpp | 4 ---- include/simgrid/s4u/Exec.hpp | 7 +------ include/simgrid/s4u/Io.hpp | 4 ---- src/s4u/s4u_Comm.cpp | 11 ----------- src/s4u/s4u_Exec.cpp | 13 ------------- src/s4u/s4u_Io.cpp | 12 ------------ 7 files changed, 13 insertions(+), 50 deletions(-) diff --git a/include/simgrid/s4u/Activity.hpp b/include/simgrid/s4u/Activity.hpp index db486e99da..5f3999f27b 100644 --- a/include/simgrid/s4u/Activity.hpp +++ b/include/simgrid/s4u/Activity.hpp @@ -7,6 +7,7 @@ #define SIMGRID_S4U_ACTIVITY_HPP #include "xbt/asserts.h" +#include #include #include @@ -99,8 +100,19 @@ private: std::string name_ = ""; std::string tracing_category_ = ""; void* user_data_ = nullptr; + std::atomic_int_fast32_t refcount_{0}; public: +#ifndef DOXYGEN + friend XBT_PUBLIC void intrusive_ptr_release(AnyActivity* a) + { + if (a->refcount_.fetch_sub(1, std::memory_order_release) == 1) { + std::atomic_thread_fence(std::memory_order_acquire); + delete a; + } + } + friend XBT_PUBLIC void intrusive_ptr_add_ref(AnyActivity* a) { a->refcount_.fetch_add(1, std::memory_order_relaxed); } +#endif AnyActivity* set_name(const std::string& name) { xbt_assert(get_state() == State::INITED, "Cannot change the name of an activity after its start"); diff --git a/include/simgrid/s4u/Comm.hpp b/include/simgrid/s4u/Comm.hpp index 612266e7da..153be7e069 100644 --- a/include/simgrid/s4u/Comm.hpp +++ b/include/simgrid/s4u/Comm.hpp @@ -9,7 +9,6 @@ #include #include -#include #include #include @@ -29,7 +28,6 @@ class XBT_PUBLIC Comm : public Activity_T { void* src_buff_ = nullptr; size_t src_buff_size_ = sizeof(void*); std::string tracing_category_ = ""; - std::atomic_int_fast32_t refcount_{0}; /* FIXME: expose these elements in the API */ bool detached_ = false; int (*match_fun_)(void*, void*, kernel::activity::CommImpl*) = nullptr; @@ -40,8 +38,6 @@ class XBT_PUBLIC Comm : public Activity_T { public: #ifndef DOXYGEN - friend XBT_PUBLIC void intrusive_ptr_release(Comm* c); - friend XBT_PUBLIC void intrusive_ptr_add_ref(Comm* c); friend Mailbox; // Factory of comms #endif diff --git a/include/simgrid/s4u/Exec.hpp b/include/simgrid/s4u/Exec.hpp index e55b407a25..3611c5489d 100644 --- a/include/simgrid/s4u/Exec.hpp +++ b/include/simgrid/s4u/Exec.hpp @@ -11,8 +11,6 @@ #include #include -#include - namespace simgrid { namespace s4u { @@ -25,21 +23,18 @@ class XBT_PUBLIC Exec : public Activity_T { double priority_ = 1.0; double bound_ = 0.0; double timeout_ = 0.0; - std::atomic_int_fast32_t refcount_{0}; protected: Exec(); - virtual ~Exec() = default; public: + virtual ~Exec() = default; #ifndef DOXYGEN Exec(Exec const&) = delete; Exec& operator=(Exec const&) = delete; friend ExecSeq; friend ExecPar; - friend XBT_PUBLIC void intrusive_ptr_release(Exec* e); - friend XBT_PUBLIC void intrusive_ptr_add_ref(Exec* e); #endif static xbt::signal on_start; static xbt::signal on_completion; diff --git a/include/simgrid/s4u/Io.hpp b/include/simgrid/s4u/Io.hpp index c9d55d65c9..b4ef0c27b1 100644 --- a/include/simgrid/s4u/Io.hpp +++ b/include/simgrid/s4u/Io.hpp @@ -9,7 +9,6 @@ #include #include -#include #include namespace simgrid { @@ -29,15 +28,12 @@ private: Disk* disk_ = nullptr; sg_size_t size_ = 0; OpType type_ = OpType::READ; - std::atomic_int_fast32_t refcount_{0}; explicit Io(sg_storage_t storage, sg_size_t size, OpType type); explicit Io(sg_disk_t disk, sg_size_t size, OpType type); public: #ifndef DOXYGEN - friend XBT_PUBLIC void intrusive_ptr_release(simgrid::s4u::Io* i); - friend XBT_PUBLIC void intrusive_ptr_add_ref(simgrid::s4u::Io* i); friend Disk; // Factory of IOs friend Storage; // Factory of IOs #endif diff --git a/src/s4u/s4u_Comm.cpp b/src/s4u/s4u_Comm.cpp index c50b10a632..98965dc78c 100644 --- a/src/s4u/s4u_Comm.cpp +++ b/src/s4u/s4u_Comm.cpp @@ -235,16 +235,5 @@ Actor* Comm::get_sender() return sender_ ? sender_->ciface() : nullptr; } -void intrusive_ptr_release(simgrid::s4u::Comm* c) -{ - if (c->refcount_.fetch_sub(1, std::memory_order_release) == 1) { - std::atomic_thread_fence(std::memory_order_acquire); - delete c; - } -} -void intrusive_ptr_add_ref(simgrid::s4u::Comm* c) -{ - c->refcount_.fetch_add(1, std::memory_order_relaxed); -} } // namespace s4u } // namespace simgrid diff --git a/src/s4u/s4u_Exec.cpp b/src/s4u/s4u_Exec.cpp index 0bee8d4dab..2511d76097 100644 --- a/src/s4u/s4u_Exec.cpp +++ b/src/s4u/s4u_Exec.cpp @@ -68,19 +68,6 @@ Exec* Exec::cancel() return this; } -void intrusive_ptr_release(simgrid::s4u::Exec* e) -{ - if (e->refcount_.fetch_sub(1, std::memory_order_release) == 1) { - std::atomic_thread_fence(std::memory_order_acquire); - delete e; - } -} - -void intrusive_ptr_add_ref(simgrid::s4u::Exec* e) -{ - e->refcount_.fetch_add(1, std::memory_order_relaxed); -} - /** @brief change the execution bound * This means changing the maximal amount of flops per second that it may consume, regardless of what the host may * deliver. Currently, this cannot be changed once the exec started. diff --git a/src/s4u/s4u_Io.cpp b/src/s4u/s4u_Io.cpp index 8f1f764685..30ebf50f7d 100644 --- a/src/s4u/s4u_Io.cpp +++ b/src/s4u/s4u_Io.cpp @@ -96,17 +96,5 @@ sg_size_t Io::get_performed_ioops() [this]() { return boost::static_pointer_cast(pimpl_)->get_performed_ioops(); }); } -void intrusive_ptr_release(simgrid::s4u::Io* i) -{ - if (i->refcount_.fetch_sub(1, std::memory_order_release) == 1) { - std::atomic_thread_fence(std::memory_order_acquire); - delete i; - } -} - -void intrusive_ptr_add_ref(simgrid::s4u::Io* i) -{ - i->refcount_.fetch_add(1, std::memory_order_relaxed); -} } // namespace s4u } // namespace simgrid -- 2.20.1