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

diff --git a/folly/experimental/hazptr/hazptr-impl.h b/folly/experimental/hazptr/hazptr-impl.h
index 70e9111d9d877aa1b5e70bcf5fb56cd231fec166..bffc026de4d05ddc2e3f8ada1bf6d5b11a48b629 100644 (file)
--- 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 <typename T, typename D>
-inline void hazptr_obj_base<T, D>::retire(
-    hazptr_domain& domain,
-    D deleter,
-    const storage_policy /* policy */) {
+inline void hazptr_obj_base<T, D>::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 <typename T>
-inline hazptr_owner<T>::hazptr_owner(
-    hazptr_domain& domain,
-    const cache_policy /* policy */) {
+inline hazptr_owner<T>::hazptr_owner(hazptr_domain& domain) {
   domain_ = &domain;
   hazptr_ = domain_->hazptrAcquire();
   DEBUG_PRINT(this << " " << domain_ << " " << hazptr_);
@@ -137,11 +132,13 @@ inline void swap(hazptr_owner<T>& lhs, hazptr_owner<T>& 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 e42f9c97e976ac5d783d25fd05e9cad5a96d387b..17d0b4b79e425328db3a5edb95bb78ff51fc9b6f 100644 (file)
--- a/folly/experimental/hazptr/hazptr.h
+++ b/folly/experimental/hazptr/hazptr.h
@@ -20,7 +20,7 @@
 #include <functional>
 #include <memory>
 
-/* Stand-in for std::pmr::memory_resource */
+/* Stand-in for C++17 std::pmr::memory_resource */
 #include <folly/experimental/hazptr/memory_resource.h>
 
 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 <typename, typename>
   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 <typename T, typename Deleter = std::default_delete<T>>
 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 <typename T> 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 <typename T> 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 <typename T> class hazptr_owner {
 template <typename T>
 void swap(hazptr_owner<T>&, hazptr_owner<T>&) 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 f8bf34ec290646a23b737a110596e6be91dd0be2..ddd06421191eba63266ebdf19f0a992b040d756f 100644 (file)
--- 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;
 }