Minor improvements to Rcu.h

Summary:
Minor improvements to `Rcu.h`:
* Spell out the first use of RCU for the sake of people who are not familiar
  with the abbreviation.
* Correct the default template argument type in the comment: it's `RcuTag`, not
  `void`.
* Don't expose `rcu_token` constructor to the users because it's only supposed
  to be obtained via `lock_shared`.
* Make other single-argument constructors explicit to prevent undesirable
  conversions.
* Parameterize `rcu_token` on the tag type to prevent cross-domain use.

Reviewed By: yfeldblum

Differential Revision: D13510133

fbshipit-source-id: d5d214cfa3b30d0857c14ac293da6e4310db1100

folly/experimental/AtomicReadMostlyMainPtr.h

@@ -63,7 +63,7 @@ class AtomicReadMostlyMainPtr {
 
   ReadMostlySharedPtr<T> load(
       std::memory_order order = std::memory_order_seq_cst) const {
-    rcu_token token = detail::atomicReadMostlyDomain->lock_shared();
+    auto token = detail::atomicReadMostlyDomain->lock_shared();
     // Synchronization point with the store in storeLocked().
     auto index = curMainPtrIndex_.load(order);
     auto result = mainPtrs_[index].getShared();

folly/synchronization/Rcu-inl.h

@@ -50,16 +50,16 @@ rcu_domain<Tag>::~rcu_domain() {
 }
 
 template <typename Tag>
-rcu_token rcu_domain<Tag>::lock_shared() {
+rcu_token<Tag> rcu_domain<Tag>::lock_shared() {
   auto idx = version_.load(std::memory_order_acquire);
   idx &= 1;
   counters_.increment(idx);
 
-  return idx;
+  return rcu_token<Tag>(idx);
 }
 
 template <typename Tag>
-void rcu_domain<Tag>::unlock_shared(rcu_token&& token) {
+void rcu_domain<Tag>::unlock_shared(rcu_token<Tag>&& token) {
   DCHECK(0 == token.epoch_ || 1 == token.epoch_);
   counters_.decrement(token.epoch_);
 }

folly/synchronization/Rcu.h

@@ -26,7 +26,7 @@
 #include <folly/synchronization/detail/ThreadCachedInts.h>
 #include <folly/synchronization/detail/ThreadCachedLists.h>
 
-// Implementation of proposed RCU C++ API
+// Implementation of proposed Read-Copy-Update (RCU) C++ API
 // http://open-std.org/JTC1/SC22/WG21/docs/papers/2017/p0566r3.pdf
 
 // Overview
@@ -175,7 +175,7 @@
 //   callers should only ever use the default, global domain.
 //
 //   Creation of a domain takes a template tag argument, which
-//   defaults to void. To access different domains, you have to pass a
+//   defaults to RcuTag. To access different domains, you have to pass a
 //   different tag.  The global domain is preferred for almost all
 //   purposes, unless a different executor is required.
 //
@@ -288,9 +288,9 @@ class rcu_domain;
 
 // Opaque token used to match up lock_shared() and unlock_shared()
 // pairs.
+template <typename Tag>
 class rcu_token {
  public:
-  rcu_token(uint64_t epoch) : epoch_(epoch) {}
   rcu_token() {}
   ~rcu_token() = default;
 
@@ -300,8 +300,9 @@ class rcu_token {
   rcu_token& operator=(rcu_token&& other) = default;
 
  private:
-  template <typename Tag>
-  friend class rcu_domain;
+  explicit rcu_token(uint64_t epoch) : epoch_(epoch) {}
+
+  friend class rcu_domain<Tag>;
   uint64_t epoch_;
 };
 
@@ -317,7 +318,7 @@ class rcu_domain {
    * If an executor is passed, it is used to run calls and delete
    * retired objects.
    */
-  rcu_domain(Executor* executor = nullptr) noexcept;
+  explicit rcu_domain(Executor* executor = nullptr) noexcept;
 
   rcu_domain(const rcu_domain&) = delete;
   rcu_domain(rcu_domain&&) = delete;
@@ -330,8 +331,8 @@ class rcu_domain {
   // all preceding lock_shared() sections are finished.
 
   // Note: can potentially allocate on thread first use.
-  FOLLY_ALWAYS_INLINE rcu_token lock_shared();
-  FOLLY_ALWAYS_INLINE void unlock_shared(rcu_token&&);
+  FOLLY_ALWAYS_INLINE rcu_token<Tag> lock_shared();
+  FOLLY_ALWAYS_INLINE void unlock_shared(rcu_token<Tag>&&);
 
   // Call a function after concurrent critical sections have finished.
   // Does not block unless the queue is full, then may block to wait
@@ -390,10 +391,10 @@ inline rcu_domain<RcuTag>* rcu_default_domain() {
 template <typename Tag = RcuTag>
 class rcu_reader_domain {
  public:
-  FOLLY_ALWAYS_INLINE rcu_reader_domain(
+  explicit FOLLY_ALWAYS_INLINE rcu_reader_domain(
       rcu_domain<Tag>* domain = rcu_default_domain()) noexcept
       : epoch_(domain->lock_shared()), domain_(domain) {}
-  rcu_reader_domain(
+  explicit rcu_reader_domain(
       std::defer_lock_t,
       rcu_domain<Tag>* domain = rcu_default_domain()) noexcept
       : domain_(domain) {}
@@ -433,7 +434,7 @@ class rcu_reader_domain {
   }
 
  private:
-  Optional<rcu_token> epoch_;
+  Optional<rcu_token<Tag>> epoch_;
   rcu_domain<Tag>* domain_;
 };
 

folly/synchronization/test/RcuTest.cpp

@@ -212,7 +212,7 @@ TEST(RcuTest, MoveReaderBetweenThreads) {
 }
 
 TEST(RcuTest, ForkTest) {
-  rcu_token epoch;
+  rcu_token<RcuTag> epoch;
   std::thread t([&]() { epoch = rcu_default_domain()->lock_shared(); });
   t.join();
   auto pid = fork();