From c45de72c4d1753ffca4f8e528909e4729da5191b Mon Sep 17 00:00:00 2001 From: Maged Michael Date: Mon, 31 Oct 2016 14:47:41 -0700 Subject: [PATCH] Update hazard pointers interface and implementation Summary: The main purpose of this diff and this library at this point is to be a public reference for the C++ standard committee and whoever is interested in the proposal to the committee. This diff aims to be consistent with the latest version of the proposal (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0233r2.pdf). The current interface proposal focuses on the core components (domain, object base, and raii owner). Once, that part is settled we can add to the interface: - Thread local options (thread caching of hazard pointers and private storage of retired objects) - Programmer control of reclamation (when and by which threads) Also, at this point the implementation does not optimize memory ordering. I removed hazptr_domain::try_reclaim() from the public interface at this point. The latest update to interface aims to relieve the programmer from the need to take spacial precautions against shutdown fiascos involving reclamation functions of objects stored by the default domain. Please let me know if you have any concerns about this. Having said that about the current purpose of this library, I really appreciate any comments (in this diff or separately) on the interface in general and any suggestions for the eventual optimized implementation. Reviewed By: davidtgoldblatt Differential Revision: D4104381 fbshipit-source-id: df98adf6fd9b7a93406cb8eeca8fe2ad12388139 --- folly/experimental/hazptr/hazptr-impl.h | 24 +++-------- folly/experimental/hazptr/hazptr.h | 41 +++---------------- folly/experimental/hazptr/test/HazptrTest.cpp | 2 - 3 files changed, 10 insertions(+), 57 deletions(-) diff --git a/folly/experimental/hazptr/hazptr-impl.h b/folly/experimental/hazptr/hazptr-impl.h index 70e9111d..bffc026d 100644 --- a/folly/experimental/hazptr/hazptr-impl.h +++ b/folly/experimental/hazptr/hazptr-impl.h @@ -34,10 +34,7 @@ constexpr hazptr_domain::hazptr_domain(memory_resource* mr) noexcept /** hazptr_obj_base */ template -inline void hazptr_obj_base::retire( - hazptr_domain& domain, - D deleter, - const storage_policy /* policy */) { +inline void hazptr_obj_base::retire(hazptr_domain& domain, D deleter) { DEBUG_PRINT(this << " " << &domain); deleter_ = std::move(deleter); reclaim_ = [](hazptr_obj* p) { @@ -67,9 +64,7 @@ class hazptr_rec { /** hazptr_owner */ template -inline hazptr_owner::hazptr_owner( - hazptr_domain& domain, - const cache_policy /* policy */) { +inline hazptr_owner::hazptr_owner(hazptr_domain& domain) { domain_ = &domain; hazptr_ = domain_->hazptrAcquire(); DEBUG_PRINT(this << " " << domain_ << " " << hazptr_); @@ -137,11 +132,13 @@ inline void swap(hazptr_owner& lhs, hazptr_owner& rhs) noexcept { // [TODO]: // - Thread caching of hazptr_rec-s // - Private storage of retired objects +// - Control of reclamation (when and by whom) // - Optimized memory order /** Definition of default_hazptr_domain() */ inline hazptr_domain& default_hazptr_domain() { static hazptr_domain d; + DEBUG_PRINT(&d); return d; } @@ -171,6 +168,7 @@ inline void hazptr_rec::release() noexcept { /** hazptr_obj */ inline const void* hazptr_obj::getObjPtr() const { + DEBUG_PRINT(this); return this; } @@ -294,17 +292,5 @@ inline void hazptr_domain::bulkReclaim() { } } -/** hazptr_user */ - -inline void hazptr_user::flush() { - DEBUG_PRINT(""); -} - -/** hazptr_remover */ - -inline void hazptr_remover::flush() { - DEBUG_PRINT(""); -} - } // namespace folly } // namespace hazptr diff --git a/folly/experimental/hazptr/hazptr.h b/folly/experimental/hazptr/hazptr.h index e42f9c97..17d0b4b7 100644 --- a/folly/experimental/hazptr/hazptr.h +++ b/folly/experimental/hazptr/hazptr.h @@ -20,7 +20,7 @@ #include #include -/* Stand-in for std::pmr::memory_resource */ +/* Stand-in for C++17 std::pmr::memory_resource */ #include namespace folly { @@ -49,8 +49,6 @@ class hazptr_domain { hazptr_domain& operator=(const hazptr_domain&) = delete; hazptr_domain& operator=(hazptr_domain&&) = delete; - void try_reclaim(); - private: template friend class hazptr_obj_base; @@ -71,6 +69,7 @@ class hazptr_domain { int pushRetired(hazptr_obj* head, hazptr_obj* tail, int count); void tryBulkReclaim(); void bulkReclaim(); + void try_reclaim(); }; /** Get the default hazptr_domain */ @@ -91,15 +90,11 @@ class hazptr_obj { template > class hazptr_obj_base : private hazptr_obj { public: - /* Policy for storing retired objects */ - enum class storage_policy { priv, shared }; - /* Retire a removed object and pass the responsibility for * reclaiming it to the hazptr library */ void retire( hazptr_domain& domain = default_hazptr_domain(), - Deleter reclaim = {}, - const storage_policy policy = storage_policy::shared); + Deleter reclaim = {}); private: Deleter deleter_; @@ -109,14 +104,8 @@ class hazptr_obj_base : private hazptr_obj { * hazard pointers, and interface for hazard pointer operations. */ template class hazptr_owner { public: - /* Policy for caching hazard pointers */ - enum class cache_policy { cache, nocache }; - /* Constructor automatically acquires a hazard pointer. */ - explicit hazptr_owner( - hazptr_domain& domain = default_hazptr_domain(), - const cache_policy policy = cache_policy::nocache); - + explicit hazptr_owner(hazptr_domain& domain = default_hazptr_domain()); /* Destructor automatically clears and releases the owned hazard pointer. */ ~hazptr_owner(); @@ -140,7 +129,7 @@ template class hazptr_owner { /* Clear the hazard pointer */ void clear() noexcept; - /* Swap ownership of hazard ponters between hazptr_owner-s. */ + /* Swap ownership of hazard pointers between hazptr_owner-s. */ /* Note: The owned hazard pointers remain unmodified during the swap * and continue to protect the respective objects that they were * protecting before the swap, if any. */ @@ -154,26 +143,6 @@ template class hazptr_owner { template void swap(hazptr_owner&, hazptr_owner&) noexcept; -/** hazptr_user: Thread-specific interface for users of hazard - * pointers (i.e., threads that own hazard pointers by using - * hazptr_owner. */ -class hazptr_user { - public: - /* Release all hazptr_rec-s cached by this thread */ - static void flush(); -}; - -/** hazptr_remover: Thread-specific interface for removers of objects - * protected by hazard pointersd, i.e., threads that call the retire - * member function of hazptr_obj_base. */ -class hazptr_remover { - public: - /* Pass responsibility of reclaiming any retired objects stored - * privately by this thread to the hazptr_domain to which they - * belong. */ - static void flush(); -}; - } // namespace hazptr } // namespace folly diff --git a/folly/experimental/hazptr/test/HazptrTest.cpp b/folly/experimental/hazptr/test/HazptrTest.cpp index f8bf34ec..ddd06421 100644 --- a/folly/experimental/hazptr/test/HazptrTest.cpp +++ b/folly/experimental/hazptr/test/HazptrTest.cpp @@ -248,8 +248,6 @@ int main(int argc, char** argv) { testing::InitGoogleTest(&argc, argv); google::ParseCommandLineFlags(&argc, &argv, true); auto ret = RUN_ALL_TESTS(); - DEBUG_PRINT("================================================= after tests"); - default_hazptr_domain().try_reclaim(); DEBUG_PRINT("================================================= end main"); return ret; } -- 2.34.1