From a41c2d6d60b1e48024b1b93b70be2b6c9bf23f3e Mon Sep 17 00:00:00 2001 From: Hartmut Kaiser Date: Thu, 19 Oct 2023 10:39:22 -0500 Subject: [PATCH] Using shared_mutex for one_size_heap --- .../server/one_size_heap_list.hpp | 8 +- .../components_base/server/wrapper_heap.hpp | 11 +-- .../server/wrapper_heap_list.hpp | 17 ++-- .../src/server/one_size_heap_list.cpp | 83 +++++++++---------- .../src/server/wrapper_heap.cpp | 67 +++++++-------- 5 files changed, 88 insertions(+), 98 deletions(-) diff --git a/libs/full/components_base/include/hpx/components_base/server/one_size_heap_list.hpp b/libs/full/components_base/include/hpx/components_base/server/one_size_heap_list.hpp index 06715a58a00e..2ea37a40956e 100644 --- a/libs/full/components_base/include/hpx/components_base/server/one_size_heap_list.hpp +++ b/libs/full/components_base/include/hpx/components_base/server/one_size_heap_list.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 1998-2017 Hartmut Kaiser +// Copyright (c) 1998-2023 Hartmut Kaiser // Copyright (c) 2011 Bryce Lelbach // // SPDX-License-Identifier: BSL-1.0 @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -30,7 +31,7 @@ namespace hpx { namespace util { using iterator = typename list_type::iterator; using const_iterator = typename list_type::const_iterator; - using mutex_type = hpx::spinlock; + using mutex_type = hpx::shared_mutex; using heap_parameters = wrapper_heap_base::heap_parameters; @@ -107,8 +108,7 @@ namespace hpx { namespace util { std::string name() const; protected: -// mutable mutex_type mtx_; - mutable pthread_rwlock_t rwlock; + mutable mutex_type rwlock_; list_type heap_list_; private: diff --git a/libs/full/components_base/include/hpx/components_base/server/wrapper_heap.hpp b/libs/full/components_base/include/hpx/components_base/server/wrapper_heap.hpp index 9c820e7c0c92..130890293b52 100644 --- a/libs/full/components_base/include/hpx/components_base/server/wrapper_heap.hpp +++ b/libs/full/components_base/include/hpx/components_base/server/wrapper_heap.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -78,7 +79,7 @@ namespace hpx::components::detail { public: explicit wrapper_heap(char const* class_name, std::size_t count, - heap_parameters parameters); + heap_parameters const& parameters); wrapper_heap(); ~wrapper_heap() override; @@ -112,11 +113,11 @@ namespace hpx::components::detail { protected: char* pool_; heap_parameters const parameters_; - alignas(64) std::atomic first_free_; - alignas(64) std::atomic free_size_; + util::cache_aligned_data_derived> first_free_; + util::cache_aligned_data_derived> free_size_; // these values are used for AGAS registration of all elements of this // managed_component heap - alignas(64) mutable mutex_type mtx_; + mutable util::cache_aligned_data_derived mtx_; naming::gid_type base_gid_; public: @@ -158,7 +159,7 @@ namespace hpx::components::detail { using value_type = T; explicit fixed_wrapper_heap(char const* class_name, std::size_t count, - heap_parameters parameters) + heap_parameters const& parameters) : base_type(class_name, count, parameters) { } diff --git a/libs/full/components_base/include/hpx/components_base/server/wrapper_heap_list.hpp b/libs/full/components_base/include/hpx/components_base/server/wrapper_heap_list.hpp index 544a6b3da16e..e83037233b33 100644 --- a/libs/full/components_base/include/hpx/components_base/server/wrapper_heap_list.hpp +++ b/libs/full/components_base/include/hpx/components_base/server/wrapper_heap_list.hpp @@ -9,8 +9,11 @@ #include #include #include +#include #include +#include +#include #include /////////////////////////////////////////////////////////////////////////////// @@ -25,7 +28,7 @@ namespace hpx::components::detail { using value_type = typename Heap::value_type; using storage_type = std::aligned_storage_t::value>; + std::alignment_of_v>; enum { @@ -44,27 +47,23 @@ namespace hpx::components::detail { get_component_type()), base_type::heap_parameters{ heap_capacity, heap_element_alignment, heap_element_size}, - (Heap*) nullptr) + static_cast(nullptr)) , type_(get_component_type()) { } naming::gid_type get_gid(void* p) { - pthread_rwlock_rdlock(&rwlock); + std::shared_lock sl(rwlock_); - using iterator = typename base_type::const_iterator; - - iterator end = heap_list_.end(); - for (iterator it = heap_list_.begin(); it != end; ++it) + auto const end = heap_list_.end(); + for (auto it = heap_list_.begin(); it != end; ++it) { if ((*it)->did_alloc(p)) { - pthread_rwlock_unlock(&rwlock); return (*it)->get_gid(p, type_); } } - pthread_rwlock_unlock(&rwlock); return naming::invalid_gid; } diff --git a/libs/full/components_base/src/server/one_size_heap_list.cpp b/libs/full/components_base/src/server/one_size_heap_list.cpp index 3033e088cf25..bcf5ab5dc3eb 100644 --- a/libs/full/components_base/src/server/one_size_heap_list.cpp +++ b/libs/full/components_base/src/server/one_size_heap_list.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 1998-2021 Hartmut Kaiser +// Copyright (c) 1998-2023 Hartmut Kaiser // Copyright (c) 2011 Bryce Lelbach // // SPDX-License-Identifier: BSL-1.0 @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,7 @@ #include #include #include +#include #include namespace hpx { namespace util { @@ -55,37 +57,35 @@ namespace hpx { namespace util { void* p = nullptr; - pthread_rwlock_rdlock(&rwlock); - - if (!heap_list_.empty()) { - for (auto& heap : heap_list_) - { - bool allocated = heap->alloc(&p, count); + std::shared_lock sl(rwlock_); - if (allocated) + if (!heap_list_.empty()) + { + for (auto& heap : heap_list_) { + if (heap->alloc(&p, count)) + { #if defined(HPX_DEBUG) - // Allocation succeeded, update statistics. - alloc_count_ += count; - if (alloc_count_ - free_count_ > max_alloc_count_) - max_alloc_count_ = alloc_count_ - free_count_; + // Allocation succeeded, update statistics. + alloc_count_ += count; + if (alloc_count_ - free_count_ > max_alloc_count_) + max_alloc_count_ = alloc_count_ - free_count_; #endif - pthread_rwlock_unlock(&rwlock); - return p; - } + return p; + } #if defined(HPX_DEBUG) - LOSH_(info).format( - "{1}::alloc: failed to allocate from heap[{2}] " - "(heap[{2}] has allocated {3} objects and has " - "space for {4} more objects)", - name(), heap->heap_count(), heap->size(), - heap->free_size()); + LOSH_(info).format( + "{1}::alloc: failed to allocate from heap[{2}] " + "(heap[{2}] has allocated {3} objects and has " + "space for {4} more objects)", + name(), heap->heap_count(), heap->size(), + heap->free_size()); #endif + } } } - pthread_rwlock_unlock(&rwlock); // Create new heap. bool result = false; @@ -98,11 +98,10 @@ namespace hpx { namespace util { result = heap->alloc((void**) &p, count); // Add the heap into the list -// mtx_.lock(); - pthread_rwlock_wrlock(&rwlock); - heap_list_.push_front(heap); - pthread_rwlock_unlock(&rwlock); -// mtx_.unlock(); + { + std::unique_lock ul(rwlock_); + heap_list_.push_front(heap); + } if (HPX_UNLIKELY(!result || nullptr == p)) { @@ -149,24 +148,22 @@ namespace hpx { namespace util { if (reschedule(p, count)) return; -// mtx_.lock(); - pthread_rwlock_rdlock(&rwlock); -// mtx_.unlock(); - // Find the heap which allocated this pointer. - for (auto& heap : heap_list_) { - bool did_allocate = heap->did_alloc(p); - if (did_allocate) + std::shared_lock sl(rwlock_); + + // Find the heap which allocated this pointer. + for (auto const& heap : heap_list_) { - heap->free(p, count); + if (heap->did_alloc(p)) + { + heap->free(p, count); #if defined(HPX_DEBUG) - free_count_ += count; + free_count_ += count; #endif - pthread_rwlock_unlock(&rwlock); - return; + return; + } } } - pthread_rwlock_unlock(&rwlock); HPX_THROW_EXCEPTION(hpx::error::bad_parameter, name() + "::free", "pointer {1} was not allocated by this {2}", p, name()); @@ -174,16 +171,14 @@ namespace hpx { namespace util { bool one_size_heap_list::did_alloc(void* p) const { - pthread_rwlock_rdlock(&rwlock); - for (typename list_type::value_type const& heap : heap_list_) + std::shared_lock sl(rwlock_); + for (auto const& heap : heap_list_) { if (heap->did_alloc(p)) { - pthread_rwlock_unlock(&rwlock); return true; } } - pthread_rwlock_unlock(&rwlock); return false; } @@ -191,7 +186,7 @@ namespace hpx { namespace util { { if (class_name_.empty()) { - return std::string("one_size_heap_list(unknown)"); + return {"one_size_heap_list(unknown)"}; } return std::string("one_size_heap_list(") + class_name_ + ")"; } diff --git a/libs/full/components_base/src/server/wrapper_heap.cpp b/libs/full/components_base/src/server/wrapper_heap.cpp index 76f6480cece5..354d0590eff1 100644 --- a/libs/full/components_base/src/server/wrapper_heap.cpp +++ b/libs/full/components_base/src/server/wrapper_heap.cpp @@ -67,7 +67,7 @@ namespace hpx::components::detail { /////////////////////////////////////////////////////////////////////////// wrapper_heap::wrapper_heap(char const* class_name, - [[maybe_unused]] std::size_t count, heap_parameters parameters) + [[maybe_unused]] std::size_t count, heap_parameters const& parameters) : pool_(nullptr) , parameters_(parameters) , first_free_(nullptr) @@ -88,15 +88,15 @@ namespace hpx::components::detail { { throw std::bad_alloc(); } + // use the pool's base address as the first gid, this will also // allow for the ids to be locally resolvable base_gid_ = naming::replace_locality_id( - naming::replace_component_type( - naming::gid_type(pool_), 0), + naming::replace_component_type(naming::gid_type(pool_), 0), agas::get_locality_id()); naming::detail::set_credit_for_gid( - base_gid_, std::int64_t(HPX_GLOBALCREDIT_INITIAL)); + base_gid_, static_cast(HPX_GLOBALCREDIT_INITIAL)); } wrapper_heap::wrapper_heap() @@ -177,7 +177,9 @@ namespace hpx::components::detail { alloc_count_ += count; #endif - char* p = first_free_.fetch_add(count * parameters_.element_size, std::memory_order_relaxed); + char* p = first_free_.fetch_add( + static_cast(count * parameters_.element_size), + std::memory_order_relaxed); if (p + num_bytes > pool_ + total_num_bytes) { @@ -219,7 +221,8 @@ namespace hpx::components::detail { #if defined(HPX_DEBUG) free_count_ += count; #endif - size_t current_free_size = free_size_.fetch_add(count, std::memory_order_relaxed) + count; + size_t const current_free_size = + free_size_.fetch_add(count, std::memory_order_relaxed) + count; // release the pool if this one was the last allocated item if (current_free_size == parameters_.capacity) @@ -247,27 +250,12 @@ namespace hpx::components::detail { [[maybe_unused]] util::itt::heap_internal_access hia; HPX_ASSERT(did_alloc(p)); + HPX_ASSERT(base_gid_); -// if (!base_gid_) -// { -// std::unique_lock l(mtx_); -// if (!base_gid_) -// { -// // use the pool's base address as the first gid, this will also -// // allow for the ids to be locally resolvable -// base_gid_ = naming::replace_locality_id( -// naming::replace_component_type( -// naming::gid_type(pool_), type), -// agas::get_locality_id()); -// -// naming::detail::set_credit_for_gid( -// base_gid_, std::int64_t(HPX_GLOBALCREDIT_INITIAL)); -// } -// } naming::gid_type result = base_gid_; - if (type) { - result = naming::replace_component_type( - result, type); + if (type) + { + result = naming::replace_component_type(result, type); } result.set_lsb(p); @@ -289,15 +277,15 @@ namespace hpx::components::detail { bool wrapper_heap::free_pool() { HPX_ASSERT(pool_); - HPX_ASSERT(first_free_ == pool_ + - parameters_.capacity * parameters_.element_size); + HPX_ASSERT(first_free_ == + pool_ + parameters_.capacity * parameters_.element_size); // unbind in AGAS service if (base_gid_) { naming::gid_type base_gid = naming::invalid_gid; { -// std::unique_lock l(mtx_); + std::unique_lock l(mtx_); if (base_gid_) { base_gid = base_gid_; @@ -324,13 +312,19 @@ namespace hpx::components::detail { return false; } - first_free_ = (reinterpret_cast(pool_) % - parameters_.element_alignment == - 0) ? - pool_ : - pool_ + parameters_.element_alignment; + if (reinterpret_cast(pool_) % + parameters_.element_alignment == + 0) + { + first_free_.store(pool_, std::memory_order_relaxed); + } + else + { + first_free_.store(pool_ + parameters_.element_alignment, + std::memory_order_relaxed); + } -// free_size_ = parameters_.capacity; + free_size_.store(parameters_.capacity, std::memory_order_release); LOSH_(info).format("wrapper_heap ({}): init_pool ({}) size: {}.", !class_name_.empty() ? class_name_.c_str() : "", @@ -370,8 +364,9 @@ namespace hpx::components::detail { parameters_.capacity * parameters_.element_size; allocator_type::free(pool_, total_num_bytes); pool_ = nullptr; -// pool_ = first_free_ = nullptr; - free_size_ = 0; + + first_free_.store(nullptr, std::memory_order_relaxed); + free_size_.store(0, std::memory_order_release); } } } // namespace hpx::components::detail