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

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
parent 21d7adcd
...@@ -34,10 +34,7 @@ constexpr hazptr_domain::hazptr_domain(memory_resource* mr) noexcept ...@@ -34,10 +34,7 @@ constexpr hazptr_domain::hazptr_domain(memory_resource* mr) noexcept
/** hazptr_obj_base */ /** hazptr_obj_base */
template <typename T, typename D> template <typename T, typename D>
inline void hazptr_obj_base<T, D>::retire( inline void hazptr_obj_base<T, D>::retire(hazptr_domain& domain, D deleter) {
hazptr_domain& domain,
D deleter,
const storage_policy /* policy */) {
DEBUG_PRINT(this << " " << &domain); DEBUG_PRINT(this << " " << &domain);
deleter_ = std::move(deleter); deleter_ = std::move(deleter);
reclaim_ = [](hazptr_obj* p) { reclaim_ = [](hazptr_obj* p) {
...@@ -67,9 +64,7 @@ class hazptr_rec { ...@@ -67,9 +64,7 @@ class hazptr_rec {
/** hazptr_owner */ /** hazptr_owner */
template <typename T> template <typename T>
inline hazptr_owner<T>::hazptr_owner( inline hazptr_owner<T>::hazptr_owner(hazptr_domain& domain) {
hazptr_domain& domain,
const cache_policy /* policy */) {
domain_ = &domain; domain_ = &domain;
hazptr_ = domain_->hazptrAcquire(); hazptr_ = domain_->hazptrAcquire();
DEBUG_PRINT(this << " " << domain_ << " " << hazptr_); DEBUG_PRINT(this << " " << domain_ << " " << hazptr_);
...@@ -137,11 +132,13 @@ inline void swap(hazptr_owner<T>& lhs, hazptr_owner<T>& rhs) noexcept { ...@@ -137,11 +132,13 @@ inline void swap(hazptr_owner<T>& lhs, hazptr_owner<T>& rhs) noexcept {
// [TODO]: // [TODO]:
// - Thread caching of hazptr_rec-s // - Thread caching of hazptr_rec-s
// - Private storage of retired objects // - Private storage of retired objects
// - Control of reclamation (when and by whom)
// - Optimized memory order // - Optimized memory order
/** Definition of default_hazptr_domain() */ /** Definition of default_hazptr_domain() */
inline hazptr_domain& default_hazptr_domain() { inline hazptr_domain& default_hazptr_domain() {
static hazptr_domain d; static hazptr_domain d;
DEBUG_PRINT(&d);
return d; return d;
} }
...@@ -171,6 +168,7 @@ inline void hazptr_rec::release() noexcept { ...@@ -171,6 +168,7 @@ inline void hazptr_rec::release() noexcept {
/** hazptr_obj */ /** hazptr_obj */
inline const void* hazptr_obj::getObjPtr() const { inline const void* hazptr_obj::getObjPtr() const {
DEBUG_PRINT(this);
return this; return this;
} }
...@@ -294,17 +292,5 @@ inline void hazptr_domain::bulkReclaim() { ...@@ -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 folly
} // namespace hazptr } // namespace hazptr
...@@ -20,7 +20,7 @@ ...@@ -20,7 +20,7 @@
#include <functional> #include <functional>
#include <memory> #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> #include <folly/experimental/hazptr/memory_resource.h>
namespace folly { namespace folly {
...@@ -49,8 +49,6 @@ class hazptr_domain { ...@@ -49,8 +49,6 @@ class hazptr_domain {
hazptr_domain& operator=(const hazptr_domain&) = delete; hazptr_domain& operator=(const hazptr_domain&) = delete;
hazptr_domain& operator=(hazptr_domain&&) = delete; hazptr_domain& operator=(hazptr_domain&&) = delete;
void try_reclaim();
private: private:
template <typename, typename> template <typename, typename>
friend class hazptr_obj_base; friend class hazptr_obj_base;
...@@ -71,6 +69,7 @@ class hazptr_domain { ...@@ -71,6 +69,7 @@ class hazptr_domain {
int pushRetired(hazptr_obj* head, hazptr_obj* tail, int count); int pushRetired(hazptr_obj* head, hazptr_obj* tail, int count);
void tryBulkReclaim(); void tryBulkReclaim();
void bulkReclaim(); void bulkReclaim();
void try_reclaim();
}; };
/** Get the default hazptr_domain */ /** Get the default hazptr_domain */
...@@ -91,15 +90,11 @@ class hazptr_obj { ...@@ -91,15 +90,11 @@ class hazptr_obj {
template <typename T, typename Deleter = std::default_delete<T>> template <typename T, typename Deleter = std::default_delete<T>>
class hazptr_obj_base : private hazptr_obj { class hazptr_obj_base : private hazptr_obj {
public: public:
/* Policy for storing retired objects */
enum class storage_policy { priv, shared };
/* Retire a removed object and pass the responsibility for /* Retire a removed object and pass the responsibility for
* reclaiming it to the hazptr library */ * reclaiming it to the hazptr library */
void retire( void retire(
hazptr_domain& domain = default_hazptr_domain(), hazptr_domain& domain = default_hazptr_domain(),
Deleter reclaim = {}, Deleter reclaim = {});
const storage_policy policy = storage_policy::shared);
private: private:
Deleter deleter_; Deleter deleter_;
...@@ -109,14 +104,8 @@ class hazptr_obj_base : private hazptr_obj { ...@@ -109,14 +104,8 @@ class hazptr_obj_base : private hazptr_obj {
* hazard pointers, and interface for hazard pointer operations. */ * hazard pointers, and interface for hazard pointer operations. */
template <typename T> class hazptr_owner { template <typename T> class hazptr_owner {
public: public:
/* Policy for caching hazard pointers */
enum class cache_policy { cache, nocache };
/* Constructor automatically acquires a hazard pointer. */ /* Constructor automatically acquires a hazard pointer. */
explicit hazptr_owner( explicit hazptr_owner(hazptr_domain& domain = default_hazptr_domain());
hazptr_domain& domain = default_hazptr_domain(),
const cache_policy policy = cache_policy::nocache);
/* Destructor automatically clears and releases the owned hazard pointer. */ /* Destructor automatically clears and releases the owned hazard pointer. */
~hazptr_owner(); ~hazptr_owner();
...@@ -140,7 +129,7 @@ template <typename T> class hazptr_owner { ...@@ -140,7 +129,7 @@ template <typename T> class hazptr_owner {
/* Clear the hazard pointer */ /* Clear the hazard pointer */
void clear() noexcept; 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 /* 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. */
...@@ -154,26 +143,6 @@ template <typename T> class hazptr_owner { ...@@ -154,26 +143,6 @@ template <typename T> class hazptr_owner {
template <typename T> template <typename T>
void swap(hazptr_owner<T>&, hazptr_owner<T>&) noexcept; 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 hazptr
} // namespace folly } // namespace folly
......
...@@ -248,8 +248,6 @@ int main(int argc, char** argv) { ...@@ -248,8 +248,6 @@ int main(int argc, char** argv) {
testing::InitGoogleTest(&argc, argv); testing::InitGoogleTest(&argc, argv);
google::ParseCommandLineFlags(&argc, &argv, true); google::ParseCommandLineFlags(&argc, &argv, true);
auto ret = RUN_ALL_TESTS(); auto ret = RUN_ALL_TESTS();
DEBUG_PRINT("================================================= after tests");
default_hazptr_domain().try_reclaim();
DEBUG_PRINT("================================================= end main"); DEBUG_PRINT("================================================= end main");
return ret; return ret;
} }
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