From ae099ba2e39ce43cbddc4a41b7a8c611cc515541 Mon Sep 17 00:00:00 2001 From: Yedidya Feldblum Date: Thu, 10 Sep 2015 20:22:21 -0700 Subject: [PATCH] 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 --- folly/RWSpinLock.h | 5 -- folly/SharedMutex.h | 6 ++ folly/Synchronized.h | 94 ++++++++++++------- folly/test/SynchronizedTest.cpp | 154 ++++++++++++++------------------ 4 files changed, 137 insertions(+), 122 deletions(-) diff --git a/folly/RWSpinLock.h b/folly/RWSpinLock.h index 8e191d82..09914e8a 100644 --- 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 05f317b2..796323dd 100644 --- 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 4fc47afd..312d3374 100644 --- a/folly/Synchronized.h +++ b/folly/Synchronized.h @@ -54,46 +54,53 @@ enum InternalDoNotUse {}; */ template struct HasLockUnlock { - enum { value = IsOneOf::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 -typename std::enable_if< - HasLockUnlock::value && !std::is_same::value>::type -acquireRead(T& mutex) { - mutex.lock(); -} +struct HasLockSharedUnlockShared { + enum { value = IsOneOf::value }; +}; /** - * Special case for boost::shared_mutex. + * Acquires a mutex for reading by calling .lock(). + * + * This variant is not appropriate for shared mutexes. */ template -typename std::enable_if::value>::type +typename std::enable_if< + HasLockUnlock::value && !HasLockSharedUnlockShared::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 -typename std::enable_if::value, bool>::type -acquireRead(T& mutex, - unsigned int milliseconds) { - return mutex.timed_lock_shared(boost::posix_time::milliseconds(milliseconds)); +typename std::enable_if::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 +typename std::enable_if< + IsOneOf::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 typename std::enable_if< - IsOneOf::value, bool>::type + IsOneOf::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 typename std::enable_if< - IsOneOf::value, bool>::type + IsOneOf::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 typename std::enable_if< - HasLockUnlock::value && !std::is_same::value>::type + HasLockUnlock::value && !HasLockSharedUnlockShared::value>::type releaseRead(T& mutex) { mutex.unlock(); } @@ -155,7 +183,7 @@ releaseRead(T& mutex) { * Special case for boost::shared_mutex. */ template -typename std::enable_if::value>::type +typename std::enable_if::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 04422a9f..841f2b25 100644 --- a/folly/test/SynchronizedTest.cpp +++ b/folly/test/SynchronizedTest.cpp @@ -20,117 +20,101 @@ #include #include +#include #include #include +namespace { -TEST(Synchronized, Basic) { - testBasic(); - testBasic(); -#ifndef __APPLE__ - testBasic(); - testBasic(); -#endif +template +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::RWTicketSpinLock64 #endif + >; +TYPED_TEST_CASE(SynchronizedTest, SynchronizedTestTypes); - testBasic(); - testBasic(); - testBasic(); -#ifndef __APPLE__ - testBasic(); - testBasic(); -#endif +TYPED_TEST(SynchronizedTest, Basic) { + testBasic(); } -TEST(Synchronized, Concurrency) { - testConcurrency(); - testConcurrency(); -#ifndef __APPLE__ - testConcurrency(); - testConcurrency(); -#endif +TYPED_TEST(SynchronizedTest, Concurrency) { + testConcurrency(); +} -#ifdef RW_SPINLOCK_USE_X86_INTRINSIC_ - testConcurrency(); -#endif +TYPED_TEST(SynchronizedTest, DualLocking) { + testDualLocking(); +} - testConcurrency(); - testConcurrency(); - testConcurrency(); -#ifndef __APPLE__ - testConcurrency(); - testConcurrency(); -#endif +TYPED_TEST(SynchronizedTest, DualLockingWithConst) { + testDualLockingWithConst(); } +TYPED_TEST(SynchronizedTest, ConstCopy) { + testConstCopy(); +} -TEST(Synchronized, DualLocking) { - testDualLocking(); - testDualLocking(); -#ifndef __APPLE__ - testDualLocking(); - testDualLocking(); +template +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::RWTicketSpinLock64 #endif + >; +TYPED_TEST_CASE(SynchronizedTimedTest, SynchronizedTimedTestTypes); - testDualLocking(); - testDualLocking(); - testDualLocking(); -#ifndef __APPLE__ - testDualLocking(); - testDualLocking(); -#endif +TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) { + testTimedSynchronized(); } +template +class SynchronizedTimedWithConstTest : public testing::Test {}; -TEST(Synchronized, DualLockingWithConst) { - testDualLockingWithConst(); - testDualLockingWithConst(); -#ifndef __APPLE__ - testDualLockingWithConst(); - testDualLockingWithConst(); +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::RWTicketSpinLock64 #endif + >; +TYPED_TEST_CASE( + SynchronizedTimedWithConstTest, SynchronizedTimedWithConstTestTypes); - testDualLockingWithConst(); - testDualLockingWithConst(); - testDualLockingWithConst(); -#ifndef __APPLE__ - testDualLockingWithConst(); - testDualLockingWithConst(); -#endif +TYPED_TEST(SynchronizedTimedWithConstTest, TimedSynchronizeWithConst) { + testTimedSynchronizedWithConst(); } - -#ifndef __APPLE__ -TEST(Synchronized, TimedSynchronized) { - testTimedSynchronized(); - testTimedSynchronized(); - - testTimedSynchronized(); - testTimedSynchronized(); - testTimedSynchronized(); - - testTimedSynchronizedWithConst(); -} -#endif - -TEST(Synchronized, ConstCopy) { -#ifndef __APPLE__ - testConstCopy(); - testConstCopy(); - - testConstCopy(); - testConstCopy(); -#endif - testConstCopy(); } -- 2.34.1