From: Adam Simpkins Date: Wed, 6 Jul 2016 01:26:33 +0000 (-0700) Subject: update Synchronized to use LockTraits X-Git-Tag: 2016.07.26~76 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=c9fd5aed37d8e116d4f3eed21b65a77e5d170037;p=folly.git 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 5bbdca6f..eea433cf 100644 --- a/folly/RWSpinLock.h +++ b/folly/RWSpinLock.h @@ -457,12 +457,6 @@ class RWSpinLock { 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 bits_; }; @@ -760,20 +754,6 @@ class RWTicketSpinLockT { 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; diff --git a/folly/SpinLock.h b/folly/SpinLock.h index 5ab44605..48d20f1a 100644 --- a/folly/SpinLock.h +++ b/folly/SpinLock.h @@ -64,11 +64,4 @@ class SpinLockGuardImpl : private boost::noncopyable { typedef SpinLockGuardImpl SpinLockGuard; -namespace detail { -template -struct HasLockUnlock; - -template <> -struct HasLockUnlock : public std::true_type {}; -} } diff --git a/folly/Synchronized.h b/folly/Synchronized.h index f9fee2ab..3d40280b 100644 --- a/folly/Synchronized.h +++ b/folly/Synchronized.h @@ -25,179 +25,17 @@ #include #include +#include #include #include #include #include #include -// 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 {}; - -/** - * 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 -struct HasLockUnlock { - enum { value = IsOneOf::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 -struct HasLockSharedUnlockShared { - enum { value = IsOneOf::value }; -}; - -/** - * Acquires a mutex for reading by calling .lock(). - * - * This variant is not appropriate for shared mutexes. - */ -template -typename std::enable_if< - HasLockUnlock::value && !HasLockSharedUnlockShared::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 -typename std::enable_if::value>::type -acquireRead(T& mutex) { - mutex.lock_shared(); -} - -/** - * Acquires a mutex for reading and writing by calling .lock(). - */ -template -typename std::enable_if::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 -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 - * enumerated below. - */ -template -typename std::enable_if< - 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() + - 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 -typename std::enable_if< - IsOneOf::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 -typename std::enable_if< - HasLockUnlock::value && !HasLockSharedUnlockShared::value>::type -releaseRead(T& mutex) { - mutex.unlock(); -} - -/** - * Special case for boost::shared_mutex. - */ -template -typename std::enable_if::value>::type -releaseRead(T& mutex) { - mutex.unlock_shared(); -} - -/** - * Releases a mutex previously acquired for reading-writing by calling - * .unlock(). - */ -template -typename std::enable_if::value>::type -releaseReadWrite(T& mutex) { - mutex.unlock(); -} - } // namespace detail /** @@ -209,20 +47,19 @@ releaseReadWrite(T& mutex) { * reviewer. In contrast, the code that uses Synchronized 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 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 struct Synchronized { @@ -372,8 +209,8 @@ struct Synchronized { * 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::try_lock_for(parent->mutex_, chronoMS)) { parent_ = parent; return; } @@ -415,8 +252,9 @@ struct Synchronized { * Destructor releases. */ ~LockedPtr() { - using namespace detail; - if (parent_) releaseReadWrite(parent_->mutex_); + if (parent_) { + LockTraits::unlock(parent_->mutex_); + } } /** @@ -435,8 +273,7 @@ struct Synchronized { */ struct Unsynchronizer { explicit Unsynchronizer(LockedPtr* p) : parent_(p) { - using namespace detail; - releaseReadWrite(parent_->parent_->mutex_); + LockTraits::unlock(parent_->parent_->mutex_); } Unsynchronizer(const Unsynchronizer&) = delete; Unsynchronizer& operator=(const Unsynchronizer&) = delete; @@ -457,8 +294,9 @@ struct Synchronized { private: void acquire() { - using namespace detail; - if (parent_) acquireReadWrite(parent_->mutex_); + if (parent_) { + LockTraits::lock(parent_->mutex_); + } } // This is the entire state of LockedPtr. @@ -490,10 +328,8 @@ struct Synchronized { 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; } @@ -509,8 +345,9 @@ struct Synchronized { } } ~ConstLockedPtr() { - using namespace detail; - if (parent_) releaseRead(parent_->mutex_); + if (parent_) { + unlock_shared_or_unique(parent_->mutex_); + } } const T* operator->() const { @@ -519,14 +356,12 @@ struct Synchronized { 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() { - using namespace detail; - acquireRead(parent_->parent_->mutex_); + lock_shared_or_unique(parent_->parent_->mutex_); } ConstLockedPtr* operator->() const { return parent_; @@ -542,8 +377,9 @@ struct Synchronized { private: void acquire() { - using namespace detail; - if (parent_) acquireRead(parent_->mutex_); + if (parent_) { + lock_shared_or_unique(parent_->mutex_); + } } const Synchronized* parent_; diff --git a/folly/test/SynchronizedTest.cpp b/folly/test/SynchronizedTest.cpp index 375b0c16..3c80c192 100644 --- a/folly/test/SynchronizedTest.cpp +++ b/folly/test/SynchronizedTest.cpp @@ -31,28 +31,27 @@ namespace { template 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 - , 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 - , boost::shared_mutex - , folly::SpinLock #ifdef RW_SPINLOCK_USE_X86_INTRINSIC_ - , folly::RWTicketSpinLock32 - , folly::RWTicketSpinLock64 + folly::RWTicketSpinLock32, + folly::RWTicketSpinLock64, #endif - >; + boost::shared_mutex, + folly::SpinLock>; TYPED_TEST_CASE(SynchronizedTest, SynchronizedTestTypes); TYPED_TEST(SynchronizedTest, Basic) { @@ -78,21 +77,20 @@ TYPED_TEST(SynchronizedTest, ConstCopy) { template 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_ - , folly::RWTicketSpinLock32 - , folly::RWTicketSpinLock64 + folly::RWTicketSpinLock32, + folly::RWTicketSpinLock64, #endif - >; + folly::SharedMutexReadPriority, + folly::SharedMutexWritePriority>; TYPED_TEST_CASE(SynchronizedTimedTest, SynchronizedTimedTestTypes); TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) { @@ -102,17 +100,16 @@ TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) { template 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_ - , folly::RWTicketSpinLock32 - , folly::RWTicketSpinLock64 + folly::RWTicketSpinLock32, + folly::RWTicketSpinLock64, #endif - >; + folly::SharedMutexReadPriority, + folly::SharedMutexWritePriority>; TYPED_TEST_CASE( SynchronizedTimedWithConstTest, SynchronizedTimedWithConstTestTypes); @@ -152,10 +149,6 @@ class FakeMutex { // 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};