Update hazard pointer interface to standard proposal P0233R4

Summary:
Updated to the interface to be in synch with the latest standard proposal in P0233R4 as follows:
- Renamed hazptr_owner as hazptr_holder.
- Combined hazptr_holder member functions set() and clear() as reset().
- Replaced the template parameter A for hazptr_holder member function templates try_protect() and get_protected with atomic<T*>.
- Moved the template parameter T from the class hazptr_holder to its member functions try_protect(), get_protected(), and reset().
- Added a non-template overload of hazptr_holder::reset() with an optional nullptr_t parameter.
- Removed the template parameter T from the free function swap() as hazptr_holder is no longer a template.

Reviewed By: davidtgoldblatt

Differential Revision: D5283863

fbshipit-source-id: 2bc1a09f4f844aa72d9b7dff9c450540bbe09972

diff --git a/folly/experimental/hazptr/bench/HazptrBench.h b/folly/experimental/hazptr/bench/HazptrBench.h
index 9499d051ba96e02ac3433822ea4434c46b378dd1..b21a8e8057aa461673841edb9430c25ecb29ce85 100644 (file)
--- a/folly/experimental/hazptr/bench/HazptrBench.h
+++ b/folly/experimental/hazptr/bench/HazptrBench.h
@@ -33,7 +33,7 @@ inline uint64_t run_once(int nthreads, int size, int ops) {
   std::atomic<bool> start{false};
   std::atomic<int> started{0};
 
-  hazptr_owner<void> dummy_hptr[100];
+  hazptr_holder dummy_hptr[100];
 
   for (int i = 0; i < size; ++i) {
     s.add(i);
@@ -66,11 +66,9 @@ inline uint64_t run_once(int nthreads, int size, int ops) {
 
   susp.rehire();
   // end time measurement
-  uint64_t duration = 0;
   auto tend = std::chrono::steady_clock::now();
-  duration = std::chrono::duration_cast<std::chrono::nanoseconds>(tend - tbegin)
-                 .count();
-  return duration;
+  return std::chrono::duration_cast<std::chrono::nanoseconds>(tend - tbegin)
+      .count();
 }
 
 inline uint64_t bench(std::string name, int nthreads, int size, uint64_t base) {

diff --git a/folly/experimental/hazptr/example/LockFreeLIFO.h b/folly/experimental/hazptr/example/LockFreeLIFO.h
index 23f1fd7c86a168d82676094c05333c0123bcbc5c..cbcf88e0dbeee64fba5ad5ae046523b7415a1997 100644 (file)
--- a/folly/experimental/hazptr/example/LockFreeLIFO.h
+++ b/folly/experimental/hazptr/example/LockFreeLIFO.h
@@ -54,7 +54,7 @@ class LockFreeLIFO {
 
   bool pop(T& val) {
     DEBUG_PRINT(this);
-    hazptr_owner<Node> hptr;
+    hazptr_holder hptr;
     Node* pnode = head_.load();
     do {
       if (pnode == nullptr)
@@ -64,7 +64,7 @@ class LockFreeLIFO {
       auto next = pnode->next_;
       if (head_.compare_exchange_weak(pnode, next)) break;
     } while (true);
-    hptr.clear();
+    hptr.reset();
     val = pnode->value_;
     pnode->retire();
     return true;

diff --git a/folly/experimental/hazptr/example/SWMRList.h b/folly/experimental/hazptr/example/SWMRList.h
index 1b4eaa6e82891772ce3d6afcd0a67d46162a5fa3..d0bc9785e8777dc8f82b7788e35504fe3d6c1c69 100644 (file)
--- a/folly/experimental/hazptr/example/SWMRList.h
+++ b/folly/experimental/hazptr/example/SWMRList.h
@@ -103,8 +103,8 @@ class SWMRListSet {
   /* Used by readers */
   bool contains(const T& val) const {
     /* Acquire two hazard pointers for hand-over-hand traversal. */
-    hazptr_owner<Node> hptr_prev(domain_);
-    hazptr_owner<Node> hptr_curr(domain_);
+    hazptr_holder hptr_prev(domain_);
+    hazptr_holder hptr_curr(domain_);
     while (true) {
       auto prev = &head_;
       auto curr = prev->load(std::memory_order_acquire);

diff --git a/folly/experimental/hazptr/example/WideCAS.h b/folly/experimental/hazptr/example/WideCAS.h
index 1c9f35dd7e4771c08b3f5c7b45ed8d4f8bfc4e4e..9d44c2e4a4beb7a5955f11ca0145405650f8c837 100644 (file)
--- a/folly/experimental/hazptr/example/WideCAS.h
+++ b/folly/experimental/hazptr/example/WideCAS.h
@@ -48,14 +48,14 @@ class WideCAS {
   bool cas(T& u, T& v) {
     DEBUG_PRINT(this << " " << u << " " << v);
     Node* n = new Node(v);
-    hazptr_owner<Node> hptr;
+    hazptr_holder hptr;
     Node* p;
     do {
       p = hptr.get_protected(p_);
       if (p->val_ != u) { delete n; return false; }
       if (p_.compare_exchange_weak(p, n)) break;
     } while (true);
-    hptr.clear();
+    hptr.reset();
     p->retire();
     DEBUG_PRINT(this << " " << p << " " << u << " " << n << " " << v);
     return true;

diff --git a/folly/experimental/hazptr/hazptr-impl.h b/folly/experimental/hazptr/hazptr-impl.h
index 3c3254bca36a0529b240566f77e31ba6b4f26169..49bdf23f7c33b8b1ae368cc76b1919288cb3001d 100644 (file)
--- a/folly/experimental/hazptr/hazptr-impl.h
+++ b/folly/experimental/hazptr/hazptr-impl.h
@@ -134,7 +134,7 @@ inline void hazptr_obj_base<T, D>::retire(hazptr_domain& domain, D deleter) {
 class hazptr_rec {
   friend class hazptr_domain;
   friend class hazptr_tc_entry;
-  template <typename> friend class hazptr_owner;
+  friend class hazptr_holder;
 
   std::atomic<const void*> hazptr_{nullptr};
   hazptr_rec* next_{nullptr};
@@ -149,46 +149,38 @@ class hazptr_rec {
   void release() noexcept;
 };
 
-/** hazptr_owner */
+/** hazptr_holder */
 
-template <typename T>
-inline hazptr_owner<T>::hazptr_owner(hazptr_domain& domain) {
+inline hazptr_holder::hazptr_holder(hazptr_domain& domain) {
   domain_ = &domain;
   hazptr_ = domain_->hazptrAcquire();
   DEBUG_PRINT(this << " " << domain_ << " " << hazptr_);
   if (hazptr_ == nullptr) { std::bad_alloc e; throw e; }
 }
 
-template <typename T>
-hazptr_owner<T>::~hazptr_owner() {
+hazptr_holder::~hazptr_holder() {
   DEBUG_PRINT(this);
   domain_->hazptrRelease(hazptr_);
 }
 
 template <typename T>
-template <typename A>
-inline bool hazptr_owner<T>::try_protect(T*& ptr, const A& src) noexcept {
-  static_assert(
-      std::is_same<decltype(std::declval<A>().load()), T*>::value,
-      "Return type of A::load() must be T*");
+inline bool hazptr_holder::try_protect(
+    T*& ptr,
+    const std::atomic<T*>& src) noexcept {
   DEBUG_PRINT(this << " " << ptr << " " << &src);
-  set(ptr);
+  reset(ptr);
   /*** Full fence ***/ hazptr_mb::light();
   T* p = src.load(std::memory_order_acquire);
   if (p != ptr) {
     ptr = p;
-    clear();
+    reset();
     return false;
   }
   return true;
 }
 
 template <typename T>
-template <typename A>
-inline T* hazptr_owner<T>::get_protected(const A& src) noexcept {
-  static_assert(
-      std::is_same<decltype(std::declval<A>().load()), T*>::value,
-      "Return type of A::load() must be T*");
+inline T* hazptr_holder::get_protected(const std::atomic<T*>& src) noexcept {
   T* p = src.load(std::memory_order_relaxed);
   while (!try_protect(p, src)) {}
   DEBUG_PRINT(this << " " << p << " " << &src);
@@ -196,20 +188,18 @@ inline T* hazptr_owner<T>::get_protected(const A& src) noexcept {
 }
 
 template <typename T>
-inline void hazptr_owner<T>::set(const T* ptr) noexcept {
+inline void hazptr_holder::reset(const T* ptr) noexcept {
   auto p = static_cast<hazptr_obj*>(const_cast<T*>(ptr));
   DEBUG_PRINT(this << " " << ptr << " p:" << p);
   hazptr_->set(p);
 }
 
-template <typename T>
-inline void hazptr_owner<T>::clear() noexcept {
+inline void hazptr_holder::reset(std::nullptr_t) noexcept {
   DEBUG_PRINT(this);
   hazptr_->clear();
 }
 
-template <typename T>
-inline void hazptr_owner<T>::swap(hazptr_owner<T>& rhs) noexcept {
+inline void hazptr_holder::swap(hazptr_holder& rhs) noexcept {
   DEBUG_PRINT(
     this << " " <<  this->hazptr_ << " " << this->domain_ << " -- "
     << &rhs << " " << rhs.hazptr_ << " " << rhs.domain_);
@@ -217,8 +207,7 @@ inline void hazptr_owner<T>::swap(hazptr_owner<T>& rhs) noexcept {
   std::swap(this->hazptr_, rhs.hazptr_);
 }
 
-template <typename T>
-inline void swap(hazptr_owner<T>& lhs, hazptr_owner<T>& rhs) noexcept {
+inline void swap(hazptr_holder& lhs, hazptr_holder& rhs) noexcept {
   lhs.swap(rhs);
 }
 

diff --git a/folly/experimental/hazptr/hazptr.h b/folly/experimental/hazptr/hazptr.h
index fdc9901feee6485c28a73291c3313eef3c006617..3bd2fc034fd973fbbccebd255519e2158ec0b8e4 100644 (file)
--- a/folly/experimental/hazptr/hazptr.h
+++ b/folly/experimental/hazptr/hazptr.h
@@ -53,8 +53,7 @@ class hazptr_domain {
  private:
   template <typename, typename>
   friend class hazptr_obj_base;
-  template <typename>
-  friend class hazptr_owner;
+  friend class hazptr_holder;
 
   memory_resource* mr_;
   std::atomic<hazptr_rec*> hazptrs_ = {nullptr};
@@ -98,50 +97,50 @@ class hazptr_obj_base : public hazptr_obj {
   Deleter deleter_;
 };
 
-/** hazptr_owner: Template for automatic acquisition and release of
+/** hazptr_holder: Class for automatic acquisition and release of
  *  hazard pointers, and interface for hazard pointer operations. */
-template <typename T> class hazptr_owner {
+class hazptr_holder {
  public:
   /* Constructor automatically acquires a hazard pointer. */
-  explicit hazptr_owner(hazptr_domain& domain = default_hazptr_domain());
+  explicit hazptr_holder(hazptr_domain& domain = default_hazptr_domain());
   /* Destructor automatically clears and releases the owned hazard pointer. */
-  ~hazptr_owner();
+  ~hazptr_holder();
 
   /* Copy and move constructors and assignment operators are
    * disallowed because:
-   * - Each hazptr_owner owns exactly one hazard pointer at any time.
+   * - Each hazptr_holder owns exactly one hazard pointer at any time.
    * - Each hazard pointer may have up to one owner at any time. */
-  hazptr_owner(const hazptr_owner&) = delete;
-  hazptr_owner(hazptr_owner&&) = delete;
-  hazptr_owner& operator=(const hazptr_owner&) = delete;
-  hazptr_owner& operator=(hazptr_owner&&) = delete;
+  hazptr_holder(const hazptr_holder&) = delete;
+  hazptr_holder(hazptr_holder&&) = delete;
+  hazptr_holder& operator=(const hazptr_holder&) = delete;
+  hazptr_holder& operator=(hazptr_holder&&) = delete;
 
   /** Hazard pointer operations */
   /* Returns a protected pointer from the source */
-  template <typename A = std::atomic<T*>>
-  T* get_protected(const A& src) noexcept;
+  template <typename T>
+  T* get_protected(const std::atomic<T*>& src) noexcept;
   /* Return true if successful in protecting ptr if src == ptr after
    * setting the hazard pointer.  Otherwise sets ptr to src. */
-  template <typename A = std::atomic<T*>>
-  bool try_protect(T*& ptr, const A& src) noexcept;
+  template <typename T>
+  bool try_protect(T*& ptr, const std::atomic<T*>& src) noexcept;
   /* Set the hazard pointer to ptr */
-  void set(const T* ptr) noexcept;
-  /* Clear the hazard pointer */
-  void clear() noexcept;
+  template <typename T>
+  void reset(const T* ptr) noexcept;
+  /* Set the hazard pointer to nullptr */
+  void reset(std::nullptr_t = nullptr) noexcept;
 
-  /* Swap ownership of hazard pointers between hazptr_owner-s. */
+  /* Swap ownership of hazard pointers between hazptr_holder-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. */
-  void swap(hazptr_owner&) noexcept;
+  void swap(hazptr_holder&) noexcept;
 
  private:
   hazptr_domain* domain_;
   hazptr_rec* hazptr_;
 };
 
-template <typename T>
-void swap(hazptr_owner<T>&, hazptr_owner<T>&) noexcept;
+void swap(hazptr_holder&, hazptr_holder&) noexcept;
 
 } // namespace hazptr
 } // namespace folly

diff --git a/folly/experimental/hazptr/test/HazptrTest.cpp b/folly/experimental/hazptr/test/HazptrTest.cpp
index 45452b5800a8773a2fb412bdd237f2f9e6e8733b..5c135db47ea2e161c92afc6fe227056604cf481c 100644 (file)
--- a/folly/experimental/hazptr/test/HazptrTest.cpp
+++ b/folly/experimental/hazptr/test/HazptrTest.cpp
@@ -74,13 +74,13 @@ TEST_F(HazptrTest, Test1) {
   DEBUG_PRINT("");
 
   DEBUG_PRINT("=== hptr0");
-  hazptr_owner<Node1> hptr0;
+  hazptr_holder hptr0;
   DEBUG_PRINT("=== hptr1");
-  hazptr_owner<Node1> hptr1(myDomain0);
+  hazptr_holder hptr1(myDomain0);
   DEBUG_PRINT("=== hptr2");
-  hazptr_owner<Node1> hptr2(myDomain1);
+  hazptr_holder hptr2(myDomain1);
   DEBUG_PRINT("=== hptr3");
-  hazptr_owner<Node1> hptr3;
+  hazptr_holder hptr3;
 
   DEBUG_PRINT("");
 
@@ -91,11 +91,12 @@ TEST_F(HazptrTest, Test1) {
 
   if (hptr0.try_protect(n0, shared0)) {}
   if (hptr1.try_protect(n1, shared1)) {}
-  hptr1.clear();
-  hptr1.set(n2);
+  hptr1.reset();
+  hptr1.reset(nullptr);
+  hptr1.reset(n2);
   if (hptr2.try_protect(n3, shared3)) {}
   swap(hptr1, hptr2);
-  hptr3.clear();
+  hptr3.reset();
 
   DEBUG_PRINT("");
 
@@ -136,13 +137,13 @@ TEST_F(HazptrTest, Test2) {
   DEBUG_PRINT("");
 
   DEBUG_PRINT("=== hptr0");
-  hazptr_owner<Node2> hptr0;
+  hazptr_holder hptr0;
   DEBUG_PRINT("=== hptr1");
-  hazptr_owner<Node2> hptr1(mineDomain0);
+  hazptr_holder hptr1(mineDomain0);
   DEBUG_PRINT("=== hptr2");
-  hazptr_owner<Node2> hptr2(mineDomain1);
+  hazptr_holder hptr2(mineDomain1);
   DEBUG_PRINT("=== hptr3");
-  hazptr_owner<Node2> hptr3;
+  hazptr_holder hptr3;
 
   DEBUG_PRINT("");
 
@@ -153,11 +154,11 @@ TEST_F(HazptrTest, Test2) {
 
   if (hptr0.try_protect(n0, shared0)) {}
   if (hptr1.try_protect(n1, shared1)) {}
-  hptr1.clear();
-  hptr1.set(n2);
+  hptr1.reset();
+  hptr1.reset(n2);
   if (hptr2.try_protect(n3, shared3)) {}
   swap(hptr1, hptr2);
-  hptr3.clear();
+  hptr3.reset();
 
   DEBUG_PRINT("");
 
@@ -254,8 +255,8 @@ TEST_F(HazptrTest, VirtualTest) {
     auto bar = new Thing;
     bar->a = i;
 
-    hazptr_owner<Thing> hptr;
-    hptr.set(bar);
+    hazptr_holder hptr;
+    hptr.reset(bar);
     bar->retire();
     EXPECT_EQ(bar->a, i);
   }