Use type-parameterized test cases in folly/test/SynchronizedTest.cpp

Summary: [Folly] Use type-parameterized test cases in `folly/test/SynchronizedTest.cpp`.

And some cleanups.

Reviewed By: @nbronson

Differential Revision: D2428287

diff --git a/folly/RWSpinLock.h b/folly/RWSpinLock.h
index 8e191d825dcfcacda807fe3279be771fc012dc80..09914e8a6dd75b7d6197479d59211743c0b10469 100644 (file)
--- a/folly/RWSpinLock.h
+++ b/folly/RWSpinLock.h
@@ -740,11 +740,6 @@ class RWTicketSpinLockT : boost::noncopyable {
   friend void acquireReadWrite(RWTicketSpinLockT& mutex) {
     mutex.lock();
   }
-  friend bool acquireReadWrite(RWTicketSpinLockT& mutex,
-                               unsigned int milliseconds) {
-    mutex.lock();
-    return true;
-  }
   friend void releaseRead(RWTicketSpinLockT& mutex) {
     mutex.unlock_shared();
   }

diff --git a/folly/SharedMutex.h b/folly/SharedMutex.h
index 05f317b26d4b767d85996b70770b6b2194738fe7..796323dd0f175d67a578cbe43fbd9367ae656c06 100644 (file)
--- a/folly/SharedMutex.h
+++ b/folly/SharedMutex.h
@@ -1379,6 +1379,12 @@ class SharedMutexImpl {
   friend void acquireReadWrite(SharedMutexImpl& lock) { lock.lock(); }
   friend void releaseRead(SharedMutexImpl& lock) { lock.unlock_shared(); }
   friend void releaseReadWrite(SharedMutexImpl& lock) { lock.unlock(); }
+  friend bool acquireRead(SharedMutexImpl& lock, unsigned int ms) {
+    return lock.try_lock_shared_for(std::chrono::milliseconds(ms));
+  }
+  friend bool acquireReadWrite(SharedMutexImpl& lock, unsigned int ms) {
+    return lock.try_lock_for(std::chrono::milliseconds(ms));
+  }
 };
 
 #define COMMON_CONCURRENCY_SHARED_MUTEX_DECLARE_STATIC_STORAGE(type) \

diff --git a/folly/Synchronized.h b/folly/Synchronized.h
index 4fc47afd5ed7e22cb4afb9a04453df09e2b1b8b8..312d33743fa6a37b12b3e1d8da750de92b6927d6 100644 (file)
--- a/folly/Synchronized.h
+++ b/folly/Synchronized.h
@@ -54,46 +54,53 @@ enum InternalDoNotUse {};
  */
 template <class T>
 struct HasLockUnlock {
-  enum { value = IsOneOf<T,
-         std::mutex, std::recursive_mutex,
-         boost::mutex, boost::recursive_mutex, boost::shared_mutex
+  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
+      , std::timed_mutex
+      , std::recursive_timed_mutex
+      , boost::timed_mutex
+      , boost::recursive_timed_mutex
 #endif
-         >::value };
+      >::value };
 };
 
 /**
- * Acquires a mutex for reading by calling .lock(). The exception is
- * boost::shared_mutex, which has a special read-lock primitive called
- * .lock_shared().
+ * 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>
-typename std::enable_if<
-  HasLockUnlock<T>::value && !std::is_same<T, boost::shared_mutex>::value>::type
-acquireRead(T& mutex) {
-  mutex.lock();
-}
+struct HasLockSharedUnlockShared {
+  enum { value = IsOneOf<T
+      , boost::shared_mutex
+      >::value };
+};
 
 /**
- * Special case for boost::shared_mutex.
+ * Acquires a mutex for reading by calling .lock().
+ *
+ * This variant is not appropriate for shared mutexes.
  */
 template <class T>
-typename std::enable_if<std::is_same<T, boost::shared_mutex>::value>::type
+typename std::enable_if<
+  HasLockUnlock<T>::value && !HasLockSharedUnlockShared<T>::value>::type
 acquireRead(T& mutex) {
-  mutex.lock_shared();
+  mutex.lock();
 }
 
 /**
- * Acquires a mutex for reading with timeout by calling .timed_lock(). This
- * applies to three of the boost mutex classes as enumerated below.
+ * Acquires a mutex for reading by calling .lock_shared().
+ *
+ * This variant is not appropriate for nonshared mutexes.
  */
 template <class T>
-typename std::enable_if<std::is_same<T, boost::shared_mutex>::value, bool>::type
-acquireRead(T& mutex,
-            unsigned int milliseconds) {
-  return mutex.timed_lock_shared(boost::posix_time::milliseconds(milliseconds));
+typename std::enable_if<HasLockSharedUnlockShared<T>::value>::type
+acquireRead(T& mutex) {
+  mutex.lock_shared();
 }
 
 /**
@@ -106,6 +113,20 @@ acquireReadWrite(T& mutex) {
 }
 
 #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
@@ -113,11 +134,15 @@ acquireReadWrite(T& mutex) {
  */
 template <class T>
 typename std::enable_if<
-  IsOneOf<T, std::timed_mutex, std::recursive_timed_mutex>::value, bool>::type
+  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() +
@@ -126,16 +151,19 @@ acquireReadWrite(T& mutex,
 
 /**
  * Acquires a mutex for reading and writing with timeout by calling
- * .timed_lock(). This applies to three of the boost mutex classes as
+ * .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
+  IsOneOf<T
+      , boost::shared_mutex
+      , boost::timed_mutex
+      , boost::recursive_timed_mutex
+      >::value, bool>::type
 acquireReadWrite(T& mutex,
                  unsigned int milliseconds) {
-  return mutex.timed_lock(boost::posix_time::milliseconds(milliseconds));
+  return mutex.try_lock_for(boost::chrono::milliseconds(milliseconds));
 }
 #endif // FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
 
@@ -146,7 +174,7 @@ acquireReadWrite(T& mutex,
  */
 template <class T>
 typename std::enable_if<
-  HasLockUnlock<T>::value && !std::is_same<T, boost::shared_mutex>::value>::type
+  HasLockUnlock<T>::value && !HasLockSharedUnlockShared<T>::value>::type
 releaseRead(T& mutex) {
   mutex.unlock();
 }
@@ -155,7 +183,7 @@ releaseRead(T& mutex) {
  * Special case for boost::shared_mutex.
  */
 template <class T>
-typename std::enable_if<std::is_same<T, boost::shared_mutex>::value>::type
+typename std::enable_if<HasLockSharedUnlockShared<T>::value>::type
 releaseRead(T& mutex) {
   mutex.unlock_shared();
 }
@@ -454,8 +482,10 @@ struct Synchronized {
       acquire();
     }
     ConstLockedPtr(const Synchronized* parent, unsigned int milliseconds) {
-      if (parent->mutex_.timed_lock_shared(
-            boost::posix_time::milliseconds(milliseconds))) {
+      using namespace detail;
+      if (acquireRead(
+            parent->mutex_,
+            milliseconds)) {
         parent_ = parent;
         return;
       }

diff --git a/folly/test/SynchronizedTest.cpp b/folly/test/SynchronizedTest.cpp
index 04422a9fb60851e6ba90d65375fd1ccb823ef9fc..841f2b25febb76b90b5554f17fcfc05a33a27dad 100644 (file)
--- a/folly/test/SynchronizedTest.cpp
+++ b/folly/test/SynchronizedTest.cpp
@@ -20,117 +20,101 @@
 
 #include <folly/Synchronized.h>
 #include <folly/RWSpinLock.h>
+#include <folly/SharedMutex.h>
 #include <folly/test/SynchronizedTestLib.h>
 #include <gtest/gtest.h>
 
+namespace {
 
-TEST(Synchronized, Basic) {
-  testBasic<std::mutex>();
-  testBasic<std::recursive_mutex>();
-#ifndef __APPLE__
-  testBasic<std::timed_mutex>();
-  testBasic<std::recursive_timed_mutex>();
-#endif
+template <class Mutex>
+class SynchronizedTest : public testing::Test {};
 
+using SynchronizedTestTypes = testing::Types
+  < folly::SharedMutexReadPriority
+  , folly::SharedMutexWritePriority
+  , std::mutex
+  , std::recursive_mutex
+#ifdef FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
+  , std::timed_mutex
+  , std::recursive_timed_mutex
+#endif
+  , boost::mutex
+  , boost::recursive_mutex
+#ifdef FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
+  , boost::timed_mutex
+  , boost::recursive_timed_mutex
+#endif
+  , boost::shared_mutex
 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_
-  testBasic<folly::RWTicketSpinLock32>();
+  , folly::RWTicketSpinLock32
+  , folly::RWTicketSpinLock64
 #endif
+  >;
+TYPED_TEST_CASE(SynchronizedTest, SynchronizedTestTypes);
 
-  testBasic<boost::mutex>();
-  testBasic<boost::recursive_mutex>();
-  testBasic<boost::shared_mutex>();
-#ifndef __APPLE__
-  testBasic<boost::timed_mutex>();
-  testBasic<boost::recursive_timed_mutex>();
-#endif
+TYPED_TEST(SynchronizedTest, Basic) {
+  testBasic<TypeParam>();
 }
 
-TEST(Synchronized, Concurrency) {
-  testConcurrency<std::mutex>();
-  testConcurrency<std::recursive_mutex>();
-#ifndef __APPLE__
-  testConcurrency<std::timed_mutex>();
-  testConcurrency<std::recursive_timed_mutex>();
-#endif
+TYPED_TEST(SynchronizedTest, Concurrency) {
+  testConcurrency<TypeParam>();
+}
 
-#ifdef RW_SPINLOCK_USE_X86_INTRINSIC_
-  testConcurrency<folly::RWTicketSpinLock32>();
-#endif
+TYPED_TEST(SynchronizedTest, DualLocking) {
+  testDualLocking<TypeParam>();
+}
 
-  testConcurrency<boost::mutex>();
-  testConcurrency<boost::recursive_mutex>();
-  testConcurrency<boost::shared_mutex>();
-#ifndef __APPLE__
-  testConcurrency<boost::timed_mutex>();
-  testConcurrency<boost::recursive_timed_mutex>();
-#endif
+TYPED_TEST(SynchronizedTest, DualLockingWithConst) {
+  testDualLockingWithConst<TypeParam>();
 }
 
+TYPED_TEST(SynchronizedTest, ConstCopy) {
+  testConstCopy<TypeParam>();
+}
 
-TEST(Synchronized, DualLocking) {
-  testDualLocking<std::mutex>();
-  testDualLocking<std::recursive_mutex>();
-#ifndef __APPLE__
-  testDualLocking<std::timed_mutex>();
-  testDualLocking<std::recursive_timed_mutex>();
+template <class Mutex>
+class SynchronizedTimedTest : public testing::Test {};
+
+using SynchronizedTimedTestTypes = testing::Types
+  < folly::SharedMutexReadPriority
+  , folly::SharedMutexWritePriority
+#ifdef FOLLY_SYNCHRONIZED_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_
-  testDualLocking<folly::RWTicketSpinLock32>();
+  , folly::RWTicketSpinLock32
+  , folly::RWTicketSpinLock64
 #endif
+  >;
+TYPED_TEST_CASE(SynchronizedTimedTest, SynchronizedTimedTestTypes);
 
-  testDualLocking<boost::mutex>();
-  testDualLocking<boost::recursive_mutex>();
-  testDualLocking<boost::shared_mutex>();
-#ifndef __APPLE__
-  testDualLocking<boost::timed_mutex>();
-  testDualLocking<boost::recursive_timed_mutex>();
-#endif
+TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) {
+  testTimedSynchronized<TypeParam>();
 }
 
+template <class Mutex>
+class SynchronizedTimedWithConstTest : public testing::Test {};
 
-TEST(Synchronized, DualLockingWithConst) {
-  testDualLockingWithConst<std::mutex>();
-  testDualLockingWithConst<std::recursive_mutex>();
-#ifndef __APPLE__
-  testDualLockingWithConst<std::timed_mutex>();
-  testDualLockingWithConst<std::recursive_timed_mutex>();
+using SynchronizedTimedWithConstTestTypes = testing::Types
+  < folly::SharedMutexReadPriority
+  , folly::SharedMutexWritePriority
+#ifdef FOLLY_SYNCHRONIZED_HAVE_TIMED_MUTEXES
+  , boost::shared_mutex
 #endif
-
 #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_
-  testDualLockingWithConst<folly::RWTicketSpinLock32>();
+  , folly::RWTicketSpinLock32
+  , folly::RWTicketSpinLock64
 #endif
+  >;
+TYPED_TEST_CASE(
+    SynchronizedTimedWithConstTest, SynchronizedTimedWithConstTestTypes);
 
-  testDualLockingWithConst<boost::mutex>();
-  testDualLockingWithConst<boost::recursive_mutex>();
-  testDualLockingWithConst<boost::shared_mutex>();
-#ifndef __APPLE__
-  testDualLockingWithConst<boost::timed_mutex>();
-  testDualLockingWithConst<boost::recursive_timed_mutex>();
-#endif
+TYPED_TEST(SynchronizedTimedWithConstTest, TimedSynchronizeWithConst) {
+  testTimedSynchronizedWithConst<TypeParam>();
 }
 
-
-#ifndef __APPLE__
-TEST(Synchronized, TimedSynchronized) {
-  testTimedSynchronized<std::timed_mutex>();
-  testTimedSynchronized<std::recursive_timed_mutex>();
-
-  testTimedSynchronized<boost::timed_mutex>();
-  testTimedSynchronized<boost::recursive_timed_mutex>();
-  testTimedSynchronized<boost::shared_mutex>();
-
-  testTimedSynchronizedWithConst<boost::shared_mutex>();
-}
-#endif
-
-TEST(Synchronized, ConstCopy) {
-#ifndef __APPLE__
-  testConstCopy<std::timed_mutex>();
-  testConstCopy<std::recursive_timed_mutex>();
-
-  testConstCopy<boost::timed_mutex>();
-  testConstCopy<boost::recursive_timed_mutex>();
-#endif
-  testConstCopy<boost::shared_mutex>();
 }