diff options
author | Tor Egge <Tor.Egge@broadpark.no> | 2017-10-27 01:57:52 +0200 |
---|---|---|
committer | Tor Egge <Tor.Egge@oath.com> | 2017-10-27 08:50:17 +0000 |
commit | 2bf0eaac4188d9e6b0efc72599297b68f6e80486 (patch) | |
tree | 8026a53c1a980a4a758446ebd609473ab00556a8 /vespalib | |
parent | c1a847ff3e9cf48187131079c736e660474fe7d2 (diff) |
Reduce use of FastOS_Mutex and FastOS_Cond.
Remove handover tricks, use move constructor and move assignment instead.
Diffstat (limited to 'vespalib')
-rw-r--r-- | vespalib/src/tests/sync/sync_test.cpp | 14 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/util/sync.h | 339 |
2 files changed, 128 insertions, 225 deletions
diff --git a/vespalib/src/tests/sync/sync_test.cpp b/vespalib/src/tests/sync/sync_test.cpp index ba6be74be22..a17f9089877 100644 --- a/vespalib/src/tests/sync/sync_test.cpp +++ b/vespalib/src/tests/sync/sync_test.cpp @@ -156,7 +156,7 @@ Test::Main() TryLock a(lock); CHECK_LOCKED(lock); if (a.hasLock()) { - LockGuard guard(a); + LockGuard guard(std::move(a)); CHECK_LOCKED(lock); } CHECK_UNLOCKED(lock); @@ -167,7 +167,7 @@ Test::Main() TryLock a(mon); CHECK_LOCKED(mon); if (a.hasLock()) { - LockGuard guard(a); + LockGuard guard(std::move(a)); CHECK_LOCKED(mon); } CHECK_UNLOCKED(mon); @@ -178,7 +178,7 @@ Test::Main() TryLock a(mon); CHECK_LOCKED(mon); if (a.hasLock()) { - MonitorGuard guard(a); + MonitorGuard guard(std::move(a)); CHECK_LOCKED(mon); } CHECK_UNLOCKED(mon); @@ -197,7 +197,7 @@ Test::Main() { CHECK_LOCKED(lock); EXPECT_TRUE(a.hasLock()); - LockGuard guard(a); + LockGuard guard(std::move(a)); EXPECT_TRUE(!a.hasLock()); CHECK_LOCKED(lock); } @@ -240,7 +240,7 @@ Test::Main() tl.unlock(); EXPECT_FALSE(tl.hasLock()); } - // LockGuard/MonitorGuard have destructive copy + // LockGuard/MonitorGuard have destructive move { Lock lock; CHECK_UNLOCKED(lock); @@ -248,7 +248,7 @@ Test::Main() CHECK_LOCKED(lock); { CHECK_LOCKED(lock); - LockGuard b(a); + LockGuard b(std::move(a)); CHECK_LOCKED(lock); } CHECK_UNLOCKED(lock); @@ -260,7 +260,7 @@ Test::Main() CHECK_LOCKED(mon); { CHECK_LOCKED(mon); - MonitorGuard b(a); + MonitorGuard b(std::move(a)); CHECK_LOCKED(mon); } CHECK_UNLOCKED(mon); diff --git a/vespalib/src/vespa/vespalib/util/sync.h b/vespalib/src/vespa/vespalib/util/sync.h index 86e0a227c72..c3d8ea86aec 100644 --- a/vespalib/src/vespa/vespalib/util/sync.h +++ b/vespalib/src/vespa/vespalib/util/sync.h @@ -2,39 +2,15 @@ #pragma once -#include <vespa/fastos/mutex.h> -#include <vespa/fastos/cond.h> #include <vespa/fastos/time.h> #include <cassert> +#include <mutex> +#include <condition_variable> +#include <chrono> namespace vespalib { -#ifndef IAM_DOXYGEN -class LockGuardHandover -{ -private: - friend class LockGuard; - FastOS_MutexInterface *_mutex; - LockGuardHandover(const LockGuardHandover &); - LockGuardHandover &operator=(const LockGuardHandover &); - LockGuardHandover(FastOS_MutexInterface *m) : _mutex(m) {} -public: -}; - - -class MonitorGuardHandover -{ -private: - friend class MonitorGuard; - FastOS_CondInterface *_cond; - MonitorGuardHandover(const MonitorGuardHandover &); - MonitorGuardHandover &operator=(const MonitorGuardHandover &); - MonitorGuardHandover(FastOS_CondInterface *c) : _cond(c) {} -public: -}; -#endif - /** * @brief A Lock is a synchronization primitive used to ensure mutual * exclusion. @@ -47,11 +23,11 @@ public: **/ class Lock { -private: +protected: friend class LockGuard; friend class TryLock; - mutable FastOS_Mutex _mutex; + mutable std::mutex _mutex; public: /** * @brief Create a new Lock. @@ -59,42 +35,10 @@ public: * Creates a Lock that has mutex instrumentation disabled. **/ Lock() : _mutex() {} - /** - * @brief Create a new Lock. - * - * Creates a Lock with support for mutex instrumentation. - * - * @param name mutex category (for instrumentation) - * @param leaf false if you will lock other locks while holding this one, - * true if you only lock this as the last (leaf) lock - * (for instrumentation). - **/ - Lock(const char *name, bool leaf) : _mutex() {(void) name; (void) leaf; } - /** - * @brief Copy a Lock. - * - * Create a new Lock with mutex instrumentation settings obtained - * from the given lock. No other information is copied. Normally - * only used when copy-constructing a bigger object containing a - * Lock. - * - * @param rhs other Lock - **/ - Lock(const Lock &rhs) : _mutex() { (void) rhs; } - /** - * @brief No-op assignment operator. - * - * Assignment operator ignoring the right hand side. It makes no - * sense to assign the state of one Lock to another, but we want - * to allow assignment of bigger objects that contain Lock - * objects. - * - * @param rhs other Lock (right hand side) - **/ - Lock &operator=(const Lock &rhs) { - (void) rhs; - return *this; - } + Lock(const Lock &) : Lock() { } + Lock(Lock &&) : Lock() { } + Lock &operator=(const Lock &) { return *this; } + Lock &operator=(Lock &&) { return *this; } }; @@ -110,56 +54,25 @@ public: * * @see TryLock **/ -class Monitor +class Monitor : public Lock { private: friend class LockGuard; friend class MonitorGuard; friend class TryLock; - mutable FastOS_Cond _cond; + mutable std::condition_variable _cond; public: /** * @brief Create a new Monitor. * * Creates a Monitor that has mutex instrumentation disabled. **/ - Monitor() : _cond() {} - /** - * @brief Create a new Monitor. - * - * Creates a Monitor with support for mutex instrumentation. - * - * @param name mutex category (for instrumentation) - * @param leaf false if you will lock other locks while holding this one, - * true if you only lock this as the last (leaf) lock - * (for instrumentation). - **/ - Monitor(const char *name, bool leaf) : _cond() { (void) name; (void) leaf; } - /** - * @brief Copy a Monitor. - * - * Creates a new Monitor with mutex instrumentation settings - * obtained from the given monitor. No other information is - * copied. Normally only used when copy-constructing a bigger - * object containing a Lock. - * - * @param rhs other Monitor - **/ - Monitor(const Monitor &rhs) : _cond() { (void) rhs; } - /** - * @brief No-op assignment operator. - * - * Assignment operator ignoring the right hand side. It makes no - * sense to assign the state of one Monitor to another, but we want - * to allow assigning objects that contain Monitor objects. - * - * @param rhs other Monitor (right hand side) - **/ - Monitor &operator=(const Monitor &rhs) { - (void) rhs; - return *this; - } + Monitor() : Lock(), _cond() {} + Monitor(const Monitor &) : Monitor() { } + Monitor(Monitor &&) : Monitor() { } + Monitor &operator=(const Monitor &) { return *this; } + Monitor &operator=(Monitor &&) { return *this; } }; @@ -198,29 +111,11 @@ private: friend class LockGuard; friend class MonitorGuard; - FastOS_MutexInterface *_mutex; - FastOS_CondInterface *_cond; - - TryLock(const TryLock &); - TryLock &operator=(const TryLock &); + std::unique_lock<std::mutex> _guard; + std::condition_variable *_cond; - FastOS_MutexInterface *stealMutex() { - FastOS_MutexInterface *ret = _mutex; - if (ret != NULL) { - _mutex = NULL; - return ret; - } - ret = _cond; - assert(ret != NULL); - _cond = NULL; - return ret; - } - FastOS_CondInterface *stealCond() { - FastOS_CondInterface *ret = _cond; - assert(ret != NULL); - _cond = NULL; - return ret; - } + TryLock(const TryLock &) = delete; + TryLock &operator=(const TryLock &) = delete; public: /** @@ -228,38 +123,50 @@ public: * * @param lock the lock to obtain **/ - TryLock(const Lock &lock) : _mutex(&lock._mutex), _cond(NULL) { - if (!_mutex->TryLock()) { - _mutex = NULL; - } + TryLock(const Lock &lock) + : _guard(lock._mutex, std::try_to_lock), + _cond(nullptr) + { } + /** * @brief Try to lock the given Monitor * * @param mon the monitor to lock **/ - TryLock(const Monitor &mon) : _mutex(NULL), _cond(&mon._cond) { - if (!_cond->TryLock()) { - _cond = NULL; - } + TryLock(const Monitor &mon) + : _guard(mon._mutex, std::try_to_lock), + _cond(_guard ? &mon._cond : nullptr) + { + } + + TryLock(TryLock &&rhs) + : _guard(std::move(rhs._guard)), + _cond(rhs._cond) + { + rhs._cond = nullptr; } + /** * @brief Release the lock held by this object, if any **/ - ~TryLock() { - if (_mutex != NULL) { - _mutex->Unlock(); - } - if (_cond != NULL) { - _cond->Unlock(); + ~TryLock() = default; + + TryLock &operator=(TryLock &&rhs) { + if (this != &rhs) { + _guard = std::move(rhs._guard); + _cond = rhs._cond; + rhs._cond = nullptr; } + return *this; } + /** * @brief Check whether this object holds a lock * * @return true if this object holds a lock **/ - bool hasLock() { return (_mutex != NULL || _cond != NULL); } + bool hasLock() { return static_cast<bool>(_guard); } /** * @brief Release the lock held by this object. * @@ -269,13 +176,9 @@ public: * the destructor will release the lock. **/ void unlock() { - if (_mutex != NULL) { - _mutex->Unlock(); - _mutex = NULL; - } - if (_cond != NULL) { - _cond->Unlock(); - _cond = NULL; + if (_guard) { + _guard.unlock(); + _cond = nullptr; } } }; @@ -296,25 +199,20 @@ public: class LockGuard { private: - FastOS_MutexInterface *_mutex; - LockGuard &operator=(const LockGuard &); - - FastOS_MutexInterface *stealMutex() { - FastOS_MutexInterface *ret = _mutex; - _mutex = NULL; - return ret; - } + std::unique_lock<std::mutex> _guard; + LockGuard &operator=(const LockGuard &) = delete; public: /** * @brief A noop guard without any mutex. **/ - LockGuard() : _mutex(NULL) {} + LockGuard() : _guard() {} + LockGuard(const LockGuard &rhs) = delete; /** * @brief Steal the lock from the given LockGuard * * @param rhs steal the lock from this one **/ - LockGuard(LockGuard &rhs) : _mutex(rhs.stealMutex()) {} + LockGuard(LockGuard &&rhs) : _guard(std::move(rhs._guard)) { } /** * @brief Obtain the lock represented by the given Lock object. * @@ -322,19 +220,8 @@ public: * * @param lock take it **/ - LockGuard(const Lock &lock) : _mutex(&lock._mutex) { - _mutex->Lock(); - } - /** - * @brief Obtain the lock on the given Monitor object. - * - * The method will block until the lock can be obtained. - * - * @param monitor take the lock on it - **/ - LockGuard(const Monitor &monitor) : _mutex(&monitor._cond) { - _mutex->Lock(); - } + LockGuard(const Lock &lock) : _guard(lock._mutex) { } + /** * @brief Create a LockGuard from a TryLock. * @@ -344,11 +231,18 @@ public: * * @param tlock take the lock from this one **/ - LockGuard(TryLock &tlock) : _mutex(tlock.stealMutex()) {} -#ifndef IAM_DOXYGEN - LockGuard(const LockGuardHandover &rhs) : _mutex(rhs._mutex) {} - operator LockGuardHandover() { return LockGuardHandover(stealMutex()); } -#endif + LockGuard(TryLock &&tlock) : _guard(std::move(tlock._guard)) + { + tlock._cond = nullptr; + } + + LockGuard &operator=(LockGuard &&rhs) { + if (this != &rhs) { + _guard = std::move(rhs._guard); + } + return *this; + } + /** * @brief Release the lock held by this object. * @@ -358,27 +252,22 @@ public: * the destructor will release the lock. **/ void unlock() { - if (_mutex != NULL) { - _mutex->Unlock(); - _mutex = NULL; + if (_guard) { + _guard.unlock(); } } /** * @brief Release the lock held by this object if unlock has not * been called. **/ - ~LockGuard() { - if (_mutex != NULL) { - _mutex->Unlock(); - } - } + ~LockGuard() = default; /** * Allow code to match guard with lock. This allows functions to take a * guard ref as input, ensuring that the caller have grabbed a lock. */ bool locks(const Lock& lock) const { - return (_mutex != NULL && _mutex == &lock._mutex); + return (_guard && _guard.mutex() == &lock._mutex); } }; @@ -399,25 +288,27 @@ public: class MonitorGuard { private: - FastOS_CondInterface *_cond; - MonitorGuard &operator=(const MonitorGuard &); + std::unique_lock<std::mutex> _guard; + std::condition_variable *_cond; + MonitorGuard &operator=(const MonitorGuard &) = delete; - FastOS_CondInterface *stealCond() { - FastOS_CondInterface *ret = _cond; - _cond = NULL; - return ret; - } public: /** * @brief A noop guard without any condition. **/ - MonitorGuard() : _cond(NULL) {} + MonitorGuard() : _guard(), _cond(nullptr) {} + MonitorGuard(const MonitorGuard &rhs) = delete; /** * @brief Steal the lock from the given MonitorGuard * * @param rhs steal the lock from this one **/ - MonitorGuard(MonitorGuard &rhs) : _cond(rhs.stealCond()) {} + MonitorGuard(MonitorGuard &&rhs) + : _guard(std::move(rhs._guard)), + _cond(rhs._cond) + { + rhs._cond = nullptr; + } /** * @brief Obtain the lock on the given Monitor object. * @@ -425,8 +316,10 @@ public: * * @param monitor take the lock on it **/ - MonitorGuard(const Monitor &monitor) : _cond(&monitor._cond) { - _cond->Lock(); + MonitorGuard(const Monitor &monitor) + : _guard(monitor._mutex), + _cond(&monitor._cond) + { } /** * @brief Create a MonitorGuard from a TryLock. @@ -437,13 +330,27 @@ public: * * @param tlock take the lock from this one **/ - MonitorGuard(TryLock &tlock) : _cond(tlock.stealCond()) {} -#ifndef IAM_DOXYGEN - MonitorGuard(const MonitorGuardHandover &rhs) : _cond(rhs._cond) {} - operator MonitorGuardHandover() { - return MonitorGuardHandover(stealCond()); + MonitorGuard(TryLock &&tlock) + : _guard(), + _cond(nullptr) + { + if (tlock._guard && tlock._cond != nullptr) { + _guard = std::move(tlock._guard); + _cond = tlock._cond; + tlock._cond = nullptr; + } + } + + MonitorGuard &operator=(MonitorGuard &&rhs) { + if (this != &rhs) { + _guard = std::move(rhs._guard); + _cond = rhs._cond; + rhs._cond = nullptr; + } + return *this; } -#endif + + /** * @brief Release the lock held by this object. * @@ -453,8 +360,8 @@ public: * the destructor will release the lock. **/ void unlock() { - assert(_cond != NULL); - _cond->Unlock(); + assert(_guard); + _guard.unlock(); _cond = NULL; } /** @@ -462,7 +369,7 @@ public: **/ void wait() { assert(_cond != NULL); - _cond->Wait(); + _cond->wait(_guard); } /** * @brief Wait for a signal on the underlying Monitor with the @@ -473,7 +380,7 @@ public: **/ bool wait(int msTimeout) { assert(_cond != NULL); - return _cond->TimedWait(msTimeout); + return _cond->wait_for(_guard, std::chrono::milliseconds(msTimeout)) == std::cv_status::no_timeout; } /** * @brief Send a signal to a single waiter on the underlying @@ -481,14 +388,14 @@ public: **/ void signal() { assert(_cond != NULL); - _cond->Signal(); + _cond->notify_one(); } /** * @brief Send a signal to all waiters on the underlying Monitor. **/ void broadcast() { assert(_cond != NULL); - _cond->Broadcast(); + _cond->notify_all(); } /** * @brief Send a signal to a single waiter on the underlying @@ -500,8 +407,8 @@ public: **/ void unsafeSignalUnlock() { assert(_cond != NULL); - _cond->Unlock(); - _cond->Signal(); + _guard.unlock(); + _cond->notify_one(); _cond = NULL; } /** @@ -514,19 +421,15 @@ public: **/ void unsafeBroadcastUnlock() { assert(_cond != NULL); - _cond->Unlock(); - _cond->Broadcast(); + _guard.unlock(); + _cond->notify_all(); _cond = NULL; } /** * @brief Release the lock held by this object if unlock has not * been called. **/ - ~MonitorGuard() { - if (_cond != NULL) { - _cond->Unlock(); - } - } + ~MonitorGuard() = default; /** * Allow code to match guard with lock. This allows functions to take a @@ -642,8 +545,8 @@ private: Monitor _monitor; uint32_t _count; - CountDownLatch(const CountDownLatch &rhs); - CountDownLatch &operator=(const CountDownLatch &rhs); + CountDownLatch(const CountDownLatch &rhs) = delete; + CountDownLatch &operator=(const CountDownLatch &rhs) = delete; public: /** |