Commit eeae0d90 authored by Maged Michael's avatar Maged Michael Committed by Facebook Github Bot

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
parent b3ef1edc
...@@ -33,7 +33,7 @@ inline uint64_t run_once(int nthreads, int size, int ops) { ...@@ -33,7 +33,7 @@ inline uint64_t run_once(int nthreads, int size, int ops) {
std::atomic<bool> start{false}; std::atomic<bool> start{false};
std::atomic<int> started{0}; std::atomic<int> started{0};
hazptr_owner<void> dummy_hptr[100]; hazptr_holder dummy_hptr[100];
for (int i = 0; i < size; ++i) { for (int i = 0; i < size; ++i) {
s.add(i); s.add(i);
...@@ -66,11 +66,9 @@ inline uint64_t run_once(int nthreads, int size, int ops) { ...@@ -66,11 +66,9 @@ inline uint64_t run_once(int nthreads, int size, int ops) {
susp.rehire(); susp.rehire();
// end time measurement // end time measurement
uint64_t duration = 0;
auto tend = std::chrono::steady_clock::now(); auto tend = std::chrono::steady_clock::now();
duration = std::chrono::duration_cast<std::chrono::nanoseconds>(tend - tbegin) return std::chrono::duration_cast<std::chrono::nanoseconds>(tend - tbegin)
.count(); .count();
return duration;
} }
inline uint64_t bench(std::string name, int nthreads, int size, uint64_t base) { inline uint64_t bench(std::string name, int nthreads, int size, uint64_t base) {
......
...@@ -54,7 +54,7 @@ class LockFreeLIFO { ...@@ -54,7 +54,7 @@ class LockFreeLIFO {
bool pop(T& val) { bool pop(T& val) {
DEBUG_PRINT(this); DEBUG_PRINT(this);
hazptr_owner<Node> hptr; hazptr_holder hptr;
Node* pnode = head_.load(); Node* pnode = head_.load();
do { do {
if (pnode == nullptr) if (pnode == nullptr)
...@@ -64,7 +64,7 @@ class LockFreeLIFO { ...@@ -64,7 +64,7 @@ class LockFreeLIFO {
auto next = pnode->next_; auto next = pnode->next_;
if (head_.compare_exchange_weak(pnode, next)) break; if (head_.compare_exchange_weak(pnode, next)) break;
} while (true); } while (true);
hptr.clear(); hptr.reset();
val = pnode->value_; val = pnode->value_;
pnode->retire(); pnode->retire();
return true; return true;
......
...@@ -103,8 +103,8 @@ class SWMRListSet { ...@@ -103,8 +103,8 @@ class SWMRListSet {
/* Used by readers */ /* Used by readers */
bool contains(const T& val) const { bool contains(const T& val) const {
/* Acquire two hazard pointers for hand-over-hand traversal. */ /* Acquire two hazard pointers for hand-over-hand traversal. */
hazptr_owner<Node> hptr_prev(domain_); hazptr_holder hptr_prev(domain_);
hazptr_owner<Node> hptr_curr(domain_); hazptr_holder hptr_curr(domain_);
while (true) { while (true) {
auto prev = &head_; auto prev = &head_;
auto curr = prev->load(std::memory_order_acquire); auto curr = prev->load(std::memory_order_acquire);
......
...@@ -48,14 +48,14 @@ class WideCAS { ...@@ -48,14 +48,14 @@ class WideCAS {
bool cas(T& u, T& v) { bool cas(T& u, T& v) {
DEBUG_PRINT(this << " " << u << " " << v); DEBUG_PRINT(this << " " << u << " " << v);
Node* n = new Node(v); Node* n = new Node(v);
hazptr_owner<Node> hptr; hazptr_holder hptr;
Node* p; Node* p;
do { do {
p = hptr.get_protected(p_); p = hptr.get_protected(p_);
if (p->val_ != u) { delete n; return false; } if (p->val_ != u) { delete n; return false; }
if (p_.compare_exchange_weak(p, n)) break; if (p_.compare_exchange_weak(p, n)) break;
} while (true); } while (true);
hptr.clear(); hptr.reset();
p->retire(); p->retire();
DEBUG_PRINT(this << " " << p << " " << u << " " << n << " " << v); DEBUG_PRINT(this << " " << p << " " << u << " " << n << " " << v);
return true; return true;
......
...@@ -134,7 +134,7 @@ inline void hazptr_obj_base<T, D>::retire(hazptr_domain& domain, D deleter) { ...@@ -134,7 +134,7 @@ inline void hazptr_obj_base<T, D>::retire(hazptr_domain& domain, D deleter) {
class hazptr_rec { class hazptr_rec {
friend class hazptr_domain; friend class hazptr_domain;
friend class hazptr_tc_entry; friend class hazptr_tc_entry;
template <typename> friend class hazptr_owner; friend class hazptr_holder;
std::atomic<const void*> hazptr_{nullptr}; std::atomic<const void*> hazptr_{nullptr};
hazptr_rec* next_{nullptr}; hazptr_rec* next_{nullptr};
...@@ -149,46 +149,38 @@ class hazptr_rec { ...@@ -149,46 +149,38 @@ class hazptr_rec {
void release() noexcept; void release() noexcept;
}; };
/** hazptr_owner */ /** hazptr_holder */
template <typename T> inline hazptr_holder::hazptr_holder(hazptr_domain& domain) {
inline hazptr_owner<T>::hazptr_owner(hazptr_domain& domain) {
domain_ = &domain; domain_ = &domain;
hazptr_ = domain_->hazptrAcquire(); hazptr_ = domain_->hazptrAcquire();
DEBUG_PRINT(this << " " << domain_ << " " << hazptr_); DEBUG_PRINT(this << " " << domain_ << " " << hazptr_);
if (hazptr_ == nullptr) { std::bad_alloc e; throw e; } if (hazptr_ == nullptr) { std::bad_alloc e; throw e; }
} }
template <typename T> hazptr_holder::~hazptr_holder() {
hazptr_owner<T>::~hazptr_owner() {
DEBUG_PRINT(this); DEBUG_PRINT(this);
domain_->hazptrRelease(hazptr_); domain_->hazptrRelease(hazptr_);
} }
template <typename T> template <typename T>
template <typename A> inline bool hazptr_holder::try_protect(
inline bool hazptr_owner<T>::try_protect(T*& ptr, const A& src) noexcept { T*& ptr,
static_assert( const std::atomic<T*>& src) noexcept {
std::is_same<decltype(std::declval<A>().load()), T*>::value,
"Return type of A::load() must be T*");
DEBUG_PRINT(this << " " << ptr << " " << &src); DEBUG_PRINT(this << " " << ptr << " " << &src);
set(ptr); reset(ptr);
/*** Full fence ***/ hazptr_mb::light(); /*** Full fence ***/ hazptr_mb::light();
T* p = src.load(std::memory_order_acquire); T* p = src.load(std::memory_order_acquire);
if (p != ptr) { if (p != ptr) {
ptr = p; ptr = p;
clear(); reset();
return false; return false;
} }
return true; return true;
} }
template <typename T> template <typename T>
template <typename A> inline T* hazptr_holder::get_protected(const std::atomic<T*>& src) noexcept {
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*");
T* p = src.load(std::memory_order_relaxed); T* p = src.load(std::memory_order_relaxed);
while (!try_protect(p, src)) {} while (!try_protect(p, src)) {}
DEBUG_PRINT(this << " " << p << " " << &src); DEBUG_PRINT(this << " " << p << " " << &src);
...@@ -196,20 +188,18 @@ inline T* hazptr_owner<T>::get_protected(const A& src) noexcept { ...@@ -196,20 +188,18 @@ inline T* hazptr_owner<T>::get_protected(const A& src) noexcept {
} }
template <typename T> 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)); auto p = static_cast<hazptr_obj*>(const_cast<T*>(ptr));
DEBUG_PRINT(this << " " << ptr << " p:" << p); DEBUG_PRINT(this << " " << ptr << " p:" << p);
hazptr_->set(p); hazptr_->set(p);
} }
template <typename T> inline void hazptr_holder::reset(std::nullptr_t) noexcept {
inline void hazptr_owner<T>::clear() noexcept {
DEBUG_PRINT(this); DEBUG_PRINT(this);
hazptr_->clear(); hazptr_->clear();
} }
template <typename T> inline void hazptr_holder::swap(hazptr_holder& rhs) noexcept {
inline void hazptr_owner<T>::swap(hazptr_owner<T>& rhs) noexcept {
DEBUG_PRINT( DEBUG_PRINT(
this << " " << this->hazptr_ << " " << this->domain_ << " -- " this << " " << this->hazptr_ << " " << this->domain_ << " -- "
<< &rhs << " " << rhs.hazptr_ << " " << rhs.domain_); << &rhs << " " << rhs.hazptr_ << " " << rhs.domain_);
...@@ -217,8 +207,7 @@ inline void hazptr_owner<T>::swap(hazptr_owner<T>& rhs) noexcept { ...@@ -217,8 +207,7 @@ inline void hazptr_owner<T>::swap(hazptr_owner<T>& rhs) noexcept {
std::swap(this->hazptr_, rhs.hazptr_); std::swap(this->hazptr_, rhs.hazptr_);
} }
template <typename T> inline void swap(hazptr_holder& lhs, hazptr_holder& rhs) noexcept {
inline void swap(hazptr_owner<T>& lhs, hazptr_owner<T>& rhs) noexcept {
lhs.swap(rhs); lhs.swap(rhs);
} }
......
...@@ -53,8 +53,7 @@ class hazptr_domain { ...@@ -53,8 +53,7 @@ class hazptr_domain {
private: private:
template <typename, typename> template <typename, typename>
friend class hazptr_obj_base; friend class hazptr_obj_base;
template <typename> friend class hazptr_holder;
friend class hazptr_owner;
memory_resource* mr_; memory_resource* mr_;
std::atomic<hazptr_rec*> hazptrs_ = {nullptr}; std::atomic<hazptr_rec*> hazptrs_ = {nullptr};
...@@ -98,50 +97,50 @@ class hazptr_obj_base : public hazptr_obj { ...@@ -98,50 +97,50 @@ class hazptr_obj_base : public hazptr_obj {
Deleter deleter_; 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. */ * hazard pointers, and interface for hazard pointer operations. */
template <typename T> class hazptr_owner { class hazptr_holder {
public: public:
/* Constructor automatically acquires a hazard pointer. */ /* 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. */ /* Destructor automatically clears and releases the owned hazard pointer. */
~hazptr_owner(); ~hazptr_holder();
/* Copy and move constructors and assignment operators are /* Copy and move constructors and assignment operators are
* disallowed because: * 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. */ * - Each hazard pointer may have up to one owner at any time. */
hazptr_owner(const hazptr_owner&) = delete; hazptr_holder(const hazptr_holder&) = delete;
hazptr_owner(hazptr_owner&&) = delete; hazptr_holder(hazptr_holder&&) = delete;
hazptr_owner& operator=(const hazptr_owner&) = delete; hazptr_holder& operator=(const hazptr_holder&) = delete;
hazptr_owner& operator=(hazptr_owner&&) = delete; hazptr_holder& operator=(hazptr_holder&&) = delete;
/** Hazard pointer operations */ /** Hazard pointer operations */
/* Returns a protected pointer from the source */ /* Returns a protected pointer from the source */
template <typename A = std::atomic<T*>> template <typename T>
T* get_protected(const A& src) noexcept; T* get_protected(const std::atomic<T*>& src) noexcept;
/* Return true if successful in protecting ptr if src == ptr after /* Return true if successful in protecting ptr if src == ptr after
* setting the hazard pointer. Otherwise sets ptr to src. */ * setting the hazard pointer. Otherwise sets ptr to src. */
template <typename A = std::atomic<T*>> template <typename T>
bool try_protect(T*& ptr, const A& src) noexcept; bool try_protect(T*& ptr, const std::atomic<T*>& src) noexcept;
/* Set the hazard pointer to ptr */ /* Set the hazard pointer to ptr */
void set(const T* ptr) noexcept; template <typename T>
/* Clear the hazard pointer */ void reset(const T* ptr) noexcept;
void clear() 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 /* Note: The owned hazard pointers remain unmodified during the swap
* and continue to protect the respective objects that they were * and continue to protect the respective objects that they were
* protecting before the swap, if any. */ * protecting before the swap, if any. */
void swap(hazptr_owner&) noexcept; void swap(hazptr_holder&) noexcept;
private: private:
hazptr_domain* domain_; hazptr_domain* domain_;
hazptr_rec* hazptr_; hazptr_rec* hazptr_;
}; };
template <typename T> void swap(hazptr_holder&, hazptr_holder&) noexcept;
void swap(hazptr_owner<T>&, hazptr_owner<T>&) noexcept;
} // namespace hazptr } // namespace hazptr
} // namespace folly } // namespace folly
......
...@@ -74,13 +74,13 @@ TEST_F(HazptrTest, Test1) { ...@@ -74,13 +74,13 @@ TEST_F(HazptrTest, Test1) {
DEBUG_PRINT(""); DEBUG_PRINT("");
DEBUG_PRINT("=== hptr0"); DEBUG_PRINT("=== hptr0");
hazptr_owner<Node1> hptr0; hazptr_holder hptr0;
DEBUG_PRINT("=== hptr1"); DEBUG_PRINT("=== hptr1");
hazptr_owner<Node1> hptr1(myDomain0); hazptr_holder hptr1(myDomain0);
DEBUG_PRINT("=== hptr2"); DEBUG_PRINT("=== hptr2");
hazptr_owner<Node1> hptr2(myDomain1); hazptr_holder hptr2(myDomain1);
DEBUG_PRINT("=== hptr3"); DEBUG_PRINT("=== hptr3");
hazptr_owner<Node1> hptr3; hazptr_holder hptr3;
DEBUG_PRINT(""); DEBUG_PRINT("");
...@@ -91,11 +91,12 @@ TEST_F(HazptrTest, Test1) { ...@@ -91,11 +91,12 @@ TEST_F(HazptrTest, Test1) {
if (hptr0.try_protect(n0, shared0)) {} if (hptr0.try_protect(n0, shared0)) {}
if (hptr1.try_protect(n1, shared1)) {} if (hptr1.try_protect(n1, shared1)) {}
hptr1.clear(); hptr1.reset();
hptr1.set(n2); hptr1.reset(nullptr);
hptr1.reset(n2);
if (hptr2.try_protect(n3, shared3)) {} if (hptr2.try_protect(n3, shared3)) {}
swap(hptr1, hptr2); swap(hptr1, hptr2);
hptr3.clear(); hptr3.reset();
DEBUG_PRINT(""); DEBUG_PRINT("");
...@@ -136,13 +137,13 @@ TEST_F(HazptrTest, Test2) { ...@@ -136,13 +137,13 @@ TEST_F(HazptrTest, Test2) {
DEBUG_PRINT(""); DEBUG_PRINT("");
DEBUG_PRINT("=== hptr0"); DEBUG_PRINT("=== hptr0");
hazptr_owner<Node2> hptr0; hazptr_holder hptr0;
DEBUG_PRINT("=== hptr1"); DEBUG_PRINT("=== hptr1");
hazptr_owner<Node2> hptr1(mineDomain0); hazptr_holder hptr1(mineDomain0);
DEBUG_PRINT("=== hptr2"); DEBUG_PRINT("=== hptr2");
hazptr_owner<Node2> hptr2(mineDomain1); hazptr_holder hptr2(mineDomain1);
DEBUG_PRINT("=== hptr3"); DEBUG_PRINT("=== hptr3");
hazptr_owner<Node2> hptr3; hazptr_holder hptr3;
DEBUG_PRINT(""); DEBUG_PRINT("");
...@@ -153,11 +154,11 @@ TEST_F(HazptrTest, Test2) { ...@@ -153,11 +154,11 @@ TEST_F(HazptrTest, Test2) {
if (hptr0.try_protect(n0, shared0)) {} if (hptr0.try_protect(n0, shared0)) {}
if (hptr1.try_protect(n1, shared1)) {} if (hptr1.try_protect(n1, shared1)) {}
hptr1.clear(); hptr1.reset();
hptr1.set(n2); hptr1.reset(n2);
if (hptr2.try_protect(n3, shared3)) {} if (hptr2.try_protect(n3, shared3)) {}
swap(hptr1, hptr2); swap(hptr1, hptr2);
hptr3.clear(); hptr3.reset();
DEBUG_PRINT(""); DEBUG_PRINT("");
...@@ -254,8 +255,8 @@ TEST_F(HazptrTest, VirtualTest) { ...@@ -254,8 +255,8 @@ TEST_F(HazptrTest, VirtualTest) {
auto bar = new Thing; auto bar = new Thing;
bar->a = i; bar->a = i;
hazptr_owner<Thing> hptr; hazptr_holder hptr;
hptr.set(bar); hptr.reset(bar);
bar->retire(); bar->retire();
EXPECT_EQ(bar->a, i); EXPECT_EQ(bar->a, i);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment