update Synchronized to use LockTraits

Summary:
Update the Synchronized code to use the new LockTraits added in D3504625.
This also removes the acquireRead*() and releaseRead*() adapter functions that
had been defined for various other lock types, which are no longer needed.

Reviewed By: yfeldblum

Differential Revision: D3512310

fbshipit-source-id: daedd47c0378aebd479dbfe7aba24978deb9cc05

diff --git a/folly/RWSpinLock.h b/folly/RWSpinLock.h
index 5bbdca6f4e5ed5f00efeabc7ef4bc99db8dc90d0..eea433cfd0d38374598596f05883873f89f3bfe3 100644 (file)
--- a/folly/RWSpinLock.h
+++ b/folly/RWSpinLock.h
@@ -457,12 +457,6 @@ class RWSpinLock {
     RWSpinLock* lock_;
   };
 
     RWSpinLock* lock_;
   };
 

-  // Synchronized<> adaptors
-  friend void acquireRead(RWSpinLock& l) { return l.lock_shared(); }
-  friend void acquireReadWrite(RWSpinLock& l) { return l.lock(); }
-  friend void releaseRead(RWSpinLock& l) { return l.unlock_shared(); }
-  friend void releaseReadWrite(RWSpinLock& l) { return l.unlock(); }
-

  private:
   std::atomic<int32_t> bits_;
 };
  private:
   std::atomic<int32_t> bits_;
 };


@@ -760,20 +754,6 @@ class RWTicketSpinLockT {
     friend class ReadHolder;
     RWSpinLock *lock_;
   };
     friend class ReadHolder;
     RWSpinLock *lock_;
   };

-
-  // Synchronized<> adaptors.
-  friend void acquireRead(RWTicketSpinLockT& mutex) {
-    mutex.lock_shared();
-  }
-  friend void acquireReadWrite(RWTicketSpinLockT& mutex) {
-    mutex.lock();
-  }
-  friend void releaseRead(RWTicketSpinLockT& mutex) {
-    mutex.unlock_shared();
-  }
-  friend void releaseReadWrite(RWTicketSpinLockT& mutex) {
-    mutex.unlock();
-  }

 };
 
 typedef RWTicketSpinLockT<32> RWTicketSpinLock32;
 };
 
 typedef RWTicketSpinLockT<32> RWTicketSpinLock32;

diff --git a/folly/SpinLock.h b/folly/SpinLock.h
index 5ab446056fc781b9a9551d0862f5a05667c9be75..48d20f1af346a8687356aff4f40f028602865954 100644 (file)
--- a/folly/SpinLock.h
+++ b/folly/SpinLock.h
@@ -64,11 +64,4 @@ class SpinLockGuardImpl : private boost::noncopyable {
 
 typedef SpinLockGuardImpl<SpinLock> SpinLockGuard;
 
 
 typedef SpinLockGuardImpl<SpinLock> SpinLockGuard;
 

-namespace detail {
-template <class T>
-struct HasLockUnlock;
-
-template <>
-struct HasLockUnlock<folly::SpinLock> : public std::true_type {};
-}

 }
 }

diff --git a/folly/Synchronized.h b/folly/Synchronized.h
index f9fee2abf059e2e03204c59702389c3e3fd0fd65..3d40280bd315b813c243326df14b78be9683b551 100644 (file)
--- a/folly/Synchronized.h
+++ b/folly/Synchronized.h
@@ -25,179 +25,17 @@
 
 #include <boost/thread.hpp>
 #include <folly/LockTraits.h>
 
 #include <boost/thread.hpp>
 #include <folly/LockTraits.h>

+#include <folly/LockTraitsBoost.h>

 #include <folly/Preprocessor.h>
 #include <folly/SharedMutex.h>
 #include <folly/Traits.h>
 #include <mutex>
 #include <type_traits>
 
 #include <folly/Preprocessor.h>
 #include <folly/SharedMutex.h>
 #include <folly/Traits.h>
 #include <mutex>
 #include <type_traits>
 

-// Temporarily provide FOLLY_LOCK_TRAITS_HAVE_TIMED_MUTEXES under the legacy
-// FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES name.  This definition will be
-// removed shortly in an upcoming diff to make Synchronized fully utilize
-// LockTraits.
-#define FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES \
-  FOLLY_LOCK_TRAITS_HAVE_TIMED_MUTEXES
-

 namespace folly {
 
 namespace detail {
 enum InternalDoNotUse {};
 namespace folly {
 
 namespace detail {
 enum InternalDoNotUse {};

-
-/**
- * Free function adaptors for std:: and boost::
- */
-
-/**
- * Yields true iff T has .lock() and .unlock() member functions. This
- * is done by simply enumerating the mutexes with this interface in
- * std and boost.
- */
-template <class T>
-struct HasLockUnlock {
-  enum { value = IsOneOf<T
-      , std::mutex
-      , std::recursive_mutex
-      , boost::mutex
-      , boost::recursive_mutex
-      , boost::shared_mutex
-#if FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
-      , std::timed_mutex
-      , std::recursive_timed_mutex
-      , boost::timed_mutex
-      , boost::recursive_timed_mutex
-#endif
-      >::value };
-};
-
-/**
- * Yields true iff T has .lock_shared() and .unlock_shared() member functions.
- * This is done by simply enumerating the mutexes with this interface.
- */
-template <class T>
-struct HasLockSharedUnlockShared {
-  enum { value = IsOneOf<T
-      , boost::shared_mutex
-      >::value };
-};
-
-/**
- * Acquires a mutex for reading by calling .lock().
- *
- * This variant is not appropriate for shared mutexes.
- */
-template <class T>
-typename std::enable_if<
-  HasLockUnlock<T>::value && !HasLockSharedUnlockShared<T>::value>::type
-acquireRead(T& mutex) {
-  mutex.lock();
-}
-
-/**
- * Acquires a mutex for reading by calling .lock_shared().
- *
- * This variant is not appropriate for nonshared mutexes.
- */
-template <class T>
-typename std::enable_if<HasLockSharedUnlockShared<T>::value>::type
-acquireRead(T& mutex) {
-  mutex.lock_shared();
-}
-
-/**
- * Acquires a mutex for reading and writing by calling .lock().
- */
-template <class T>
-typename std::enable_if<HasLockUnlock<T>::value>::type
-acquireReadWrite(T& mutex) {
-  mutex.lock();
-}
-
-#if FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
-/**
- * Acquires a mutex for reading by calling .try_lock_shared_for(). This applies
- * to boost::shared_mutex.
- */
-template <class T>
-typename std::enable_if<
-  IsOneOf<T
-      , boost::shared_mutex
-      >::value, bool>::type
-acquireRead(T& mutex,
-            unsigned int milliseconds) {
-  return mutex.try_lock_shared_for(boost::chrono::milliseconds(milliseconds));
-}
-
-/**
- * Acquires a mutex for reading and writing with timeout by calling
- * .try_lock_for(). This applies to two of the std mutex classes as
- * enumerated below.
- */
-template <class T>
-typename std::enable_if<
-  IsOneOf<T
-      , std::timed_mutex
-      , std::recursive_timed_mutex
-      >::value, bool>::type
-acquireReadWrite(T& mutex,
-                 unsigned int milliseconds) {
-  // work around try_lock_for bug in some gcc versions, see
-  // http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54562
-  // TODO: Fixed in gcc-4.9.0.
-  return mutex.try_lock()
-      || (milliseconds > 0 &&
-          mutex.try_lock_until(std::chrono::system_clock::now() +
-                               std::chrono::milliseconds(milliseconds)));
-}
-
-/**
- * Acquires a mutex for reading and writing with timeout by calling
- * .try_lock_for(). This applies to three of the boost mutex classes as
- * enumerated below.
- */
-template <class T>
-typename std::enable_if<
-  IsOneOf<T
-      , boost::shared_mutex
-      , boost::timed_mutex
-      , boost::recursive_timed_mutex
-      >::value, bool>::type
-acquireReadWrite(T& mutex,
-                 unsigned int milliseconds) {
-  return mutex.try_lock_for(boost::chrono::milliseconds(milliseconds));
-}
-#endif // FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
-
-/**
- * Releases a mutex previously acquired for reading by calling
- * .unlock(). The exception is boost::shared_mutex, which has a
- * special primitive called .unlock_shared().
- */
-template <class T>
-typename std::enable_if<
-  HasLockUnlock<T>::value && !HasLockSharedUnlockShared<T>::value>::type
-releaseRead(T& mutex) {
-  mutex.unlock();
-}
-
-/**
- * Special case for boost::shared_mutex.
- */
-template <class T>
-typename std::enable_if<HasLockSharedUnlockShared<T>::value>::type
-releaseRead(T& mutex) {
-  mutex.unlock_shared();
-}
-
-/**
- * Releases a mutex previously acquired for reading-writing by calling
- * .unlock().
- */
-template <class T>
-typename std::enable_if<HasLockUnlock<T>::value>::type
-releaseReadWrite(T& mutex) {
-  mutex.unlock();
-}
-

 } // namespace detail
 
 /**
 } // namespace detail
 
 /**


@@ -209,20 +47,19 @@ releaseReadWrite(T& mutex) {
  * reviewer. In contrast, the code that uses Synchronized<T> correctly
  * looks simple and intuitive.
  *
  * reviewer. In contrast, the code that uses Synchronized<T> correctly
  * looks simple and intuitive.
  *

- * The second parameter must be a mutex type. Supported mutexes are
- * std::mutex, std::recursive_mutex, std::timed_mutex,
- * std::recursive_timed_mutex, boost::mutex, boost::recursive_mutex,
- * boost::shared_mutex, boost::timed_mutex,
- * boost::recursive_timed_mutex, and the folly/RWSpinLock.h
- * classes.
+ * The second parameter must be a mutex type.  Any mutex type supported by
+ * LockTraits<Mutex> can be used.  By default any class with lock() and
+ * unlock() methods will work automatically.  LockTraits can be specialized to
+ * teach Locked how to use other custom mutex types.  See the documentation in
+ * LockTraits.h for additional details.

  *
  *

- * You may define Synchronized support by defining 4-6 primitives in
- * the same namespace as the mutex class (found via ADL).  The
- * primitives are: acquireRead, acquireReadWrite, releaseRead, and
- * releaseReadWrite. Two optional primitives for timout operations are
- * overloads of acquireRead and acquireReadWrite. For signatures,
- * refer to the namespace detail below, which implements the
- * primitives for mutexes in std and boost.
+ * Supported mutexes that work by default include std::mutex,
+ * std::recursive_mutex, std::timed_mutex, std::recursive_timed_mutex,
+ * folly::SharedMutex, folly::RWSpinLock, and folly::SpinLock.
+ * Include LockTraitsBoost.h to get additional LockTraits specializations to
+ * support the following boost mutex types: boost::mutex,
+ * boost::recursive_mutex, boost::shared_mutex, boost::timed_mutex, and
+ * boost::recursive_timed_mutex.

  */
 template <class T, class Mutex = SharedMutex>
 struct Synchronized {
  */
 template <class T, class Mutex = SharedMutex>
 struct Synchronized {


@@ -372,8 +209,8 @@ struct Synchronized {
      * milliseconds. If not, the LockedPtr will be subsequently null.
      */
     LockedPtr(Synchronized* parent, unsigned int milliseconds) {
      * milliseconds. If not, the LockedPtr will be subsequently null.
      */
     LockedPtr(Synchronized* parent, unsigned int milliseconds) {

-      using namespace detail;
-      if (acquireReadWrite(parent->mutex_, milliseconds)) {
+      std::chrono::milliseconds chronoMS(milliseconds);
+      if (LockTraits<Mutex>::try_lock_for(parent->mutex_, chronoMS)) {

         parent_ = parent;
         return;
       }
         parent_ = parent;
         return;
       }


@@ -415,8 +252,9 @@ struct Synchronized {
      * Destructor releases.
      */
     ~LockedPtr() {
      * Destructor releases.
      */
     ~LockedPtr() {

-      using namespace detail;
-      if (parent_) releaseReadWrite(parent_->mutex_);
+      if (parent_) {
+        LockTraits<Mutex>::unlock(parent_->mutex_);
+      }

     }
 
     /**
     }
 
     /**


@@ -435,8 +273,7 @@ struct Synchronized {
      */
     struct Unsynchronizer {
       explicit Unsynchronizer(LockedPtr* p) : parent_(p) {
      */
     struct Unsynchronizer {
       explicit Unsynchronizer(LockedPtr* p) : parent_(p) {

-        using namespace detail;
-        releaseReadWrite(parent_->parent_->mutex_);
+        LockTraits<Mutex>::unlock(parent_->parent_->mutex_);

       }
       Unsynchronizer(const Unsynchronizer&) = delete;
       Unsynchronizer& operator=(const Unsynchronizer&) = delete;
       }
       Unsynchronizer(const Unsynchronizer&) = delete;
       Unsynchronizer& operator=(const Unsynchronizer&) = delete;


@@ -457,8 +294,9 @@ struct Synchronized {
 
   private:
     void acquire() {
 
   private:
     void acquire() {

-      using namespace detail;
-      if (parent_) acquireReadWrite(parent_->mutex_);
+      if (parent_) {
+        LockTraits<Mutex>::lock(parent_->mutex_);
+      }

     }
 
     // This is the entire state of LockedPtr.
     }
 
     // This is the entire state of LockedPtr.


@@ -490,10 +328,8 @@ struct Synchronized {
       acquire();
     }
     ConstLockedPtr(const Synchronized* parent, unsigned int milliseconds) {
       acquire();
     }
     ConstLockedPtr(const Synchronized* parent, unsigned int milliseconds) {

-      using namespace detail;
-      if (acquireRead(
-            parent->mutex_,
-            milliseconds)) {
+      if (try_lock_shared_or_unique_for(
+              parent->mutex_, std::chrono::milliseconds(milliseconds))) {

         parent_ = parent;
         return;
       }
         parent_ = parent;
         return;
       }


@@ -509,8 +345,9 @@ struct Synchronized {
       }
     }
     ~ConstLockedPtr() {
       }
     }
     ~ConstLockedPtr() {

-      using namespace detail;
-      if (parent_) releaseRead(parent_->mutex_);
+      if (parent_) {
+        unlock_shared_or_unique(parent_->mutex_);
+      }

     }
 
     const T* operator->() const {
     }
 
     const T* operator->() const {


@@ -519,14 +356,12 @@ struct Synchronized {
 
     struct Unsynchronizer {
       explicit Unsynchronizer(ConstLockedPtr* p) : parent_(p) {
 
     struct Unsynchronizer {
       explicit Unsynchronizer(ConstLockedPtr* p) : parent_(p) {

-        using namespace detail;
-        releaseRead(parent_->parent_->mutex_);
+        unlock_shared_or_unique(parent_->parent_->mutex_);

       }
       Unsynchronizer(const Unsynchronizer&) = delete;
       Unsynchronizer& operator=(const Unsynchronizer&) = delete;
       ~Unsynchronizer() {
       }
       Unsynchronizer(const Unsynchronizer&) = delete;
       Unsynchronizer& operator=(const Unsynchronizer&) = delete;
       ~Unsynchronizer() {

-        using namespace detail;
-        acquireRead(parent_->parent_->mutex_);
+        lock_shared_or_unique(parent_->parent_->mutex_);

       }
       ConstLockedPtr* operator->() const {
         return parent_;
       }
       ConstLockedPtr* operator->() const {
         return parent_;


@@ -542,8 +377,9 @@ struct Synchronized {
 
   private:
     void acquire() {
 
   private:
     void acquire() {

-      using namespace detail;
-      if (parent_) acquireRead(parent_->mutex_);
+      if (parent_) {
+        lock_shared_or_unique(parent_->mutex_);
+      }

     }
 
     const Synchronized* parent_;
     }
 
     const Synchronized* parent_;

diff --git a/folly/test/SynchronizedTest.cpp b/folly/test/SynchronizedTest.cpp
index 375b0c165cc09e533eaf76d00c918b1b487f7f59..3c80c192494bb9fe5c3b5a8961574a1e5b64865d 100644 (file)
--- a/folly/test/SynchronizedTest.cpp
+++ b/folly/test/SynchronizedTest.cpp
@@ -31,28 +31,27 @@ namespace {
 template <class Mutex>
 class SynchronizedTest : public testing::Test {};
 
 template <class Mutex>
 class SynchronizedTest : public testing::Test {};
 

-using SynchronizedTestTypes = testing::Types
-  < folly::SharedMutexReadPriority
-  , folly::SharedMutexWritePriority
-  , std::mutex
-  , std::recursive_mutex
-#if FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
-  , std::timed_mutex
-  , std::recursive_timed_mutex
+using SynchronizedTestTypes = testing::Types<
+    folly::SharedMutexReadPriority,
+    folly::SharedMutexWritePriority,
+    std::mutex,
+    std::recursive_mutex,
+#if FOLLY_LOCK_TRAITS_HAVE_TIMED_MUTEXES
+    std::timed_mutex,
+    std::recursive_timed_mutex,

 #endif
 #endif

-  , boost::mutex
-  , boost::recursive_mutex
-#if FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
-  , boost::timed_mutex
-  , boost::recursive_timed_mutex
+    boost::mutex,
+    boost::recursive_mutex,
+#if FOLLY_LOCK_TRAITS_HAVE_TIMED_MUTEXES
+    boost::timed_mutex,
+    boost::recursive_timed_mutex,

 #endif
 #endif

-  , boost::shared_mutex
-  , folly::SpinLock

 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_
 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_

-  , folly::RWTicketSpinLock32
-  , folly::RWTicketSpinLock64
+    folly::RWTicketSpinLock32,
+    folly::RWTicketSpinLock64,

 #endif
 #endif

-  >;
+    boost::shared_mutex,
+    folly::SpinLock>;

 TYPED_TEST_CASE(SynchronizedTest, SynchronizedTestTypes);
 
 TYPED_TEST(SynchronizedTest, Basic) {
 TYPED_TEST_CASE(SynchronizedTest, SynchronizedTestTypes);
 
 TYPED_TEST(SynchronizedTest, Basic) {


@@ -78,21 +77,20 @@ TYPED_TEST(SynchronizedTest, ConstCopy) {
 template <class Mutex>
 class SynchronizedTimedTest : public testing::Test {};
 
 template <class Mutex>
 class SynchronizedTimedTest : public testing::Test {};
 

-using SynchronizedTimedTestTypes = testing::Types
-  < folly::SharedMutexReadPriority
-  , folly::SharedMutexWritePriority
-#if FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
-  , std::timed_mutex
-  , std::recursive_timed_mutex
-  , boost::timed_mutex
-  , boost::recursive_timed_mutex
-  , boost::shared_mutex
+using SynchronizedTimedTestTypes = testing::Types<
+#if FOLLY_LOCK_TRAITS_HAVE_TIMED_MUTEXES
+    std::timed_mutex,
+    std::recursive_timed_mutex,
+    boost::timed_mutex,
+    boost::recursive_timed_mutex,
+    boost::shared_mutex,

 #endif
 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_
 #endif
 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_

-  , folly::RWTicketSpinLock32
-  , folly::RWTicketSpinLock64
+    folly::RWTicketSpinLock32,
+    folly::RWTicketSpinLock64,

 #endif
 #endif

-  >;
+    folly::SharedMutexReadPriority,
+    folly::SharedMutexWritePriority>;

 TYPED_TEST_CASE(SynchronizedTimedTest, SynchronizedTimedTestTypes);
 
 TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) {
 TYPED_TEST_CASE(SynchronizedTimedTest, SynchronizedTimedTestTypes);
 
 TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) {


@@ -102,17 +100,16 @@ TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) {
 template <class Mutex>
 class SynchronizedTimedWithConstTest : public testing::Test {};
 
 template <class Mutex>
 class SynchronizedTimedWithConstTest : public testing::Test {};
 

-using SynchronizedTimedWithConstTestTypes = testing::Types
-  < folly::SharedMutexReadPriority
-  , folly::SharedMutexWritePriority
-#if FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
-  , boost::shared_mutex
+using SynchronizedTimedWithConstTestTypes = testing::Types<
+#if FOLLY_LOCK_TRAITS_HAVE_TIMED_MUTEXES
+    boost::shared_mutex,

 #endif
 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_
 #endif
 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_

-  , folly::RWTicketSpinLock32
-  , folly::RWTicketSpinLock64
+    folly::RWTicketSpinLock32,
+    folly::RWTicketSpinLock64,

 #endif
 #endif

-  >;
+    folly::SharedMutexReadPriority,
+    folly::SharedMutexWritePriority>;

 TYPED_TEST_CASE(
     SynchronizedTimedWithConstTest, SynchronizedTimedWithConstTestTypes);
 
 TYPED_TEST_CASE(
     SynchronizedTimedWithConstTest, SynchronizedTimedWithConstTestTypes);
 


@@ -152,10 +149,6 @@ class FakeMutex {
   // process
   static FOLLY_TLS int lockCount_;
   static FOLLY_TLS int unlockCount_;
   // process
   static FOLLY_TLS int lockCount_;
   static FOLLY_TLS int unlockCount_;

-
-  // Adapters for Synchronized<>
-  friend void acquireReadWrite(FakeMutex& lock) { lock.lock(); }
-  friend void releaseReadWrite(FakeMutex& lock) { lock.unlock(); }

 };
 FOLLY_TLS int FakeMutex::lockCount_{0};
 FOLLY_TLS int FakeMutex::unlockCount_{0};
 };
 FOLLY_TLS int FakeMutex::lockCount_{0};
 FOLLY_TLS int FakeMutex::unlockCount_{0};