Commit b179601d authored by Adam Simpkins's avatar Adam Simpkins Committed by Facebook Github Bot 5

add Synchronized::withLock() methods

Summary:
Add withLock() and related methods for invoking a lambda function while the
lock is held.  This is sometimes more convenient than opening a new scope and
using lock().

withLock() also retains some of the benefits of the SYNCHRONIZED macro in that
it forces users to put their critical section code in a new scope, making the
critical section more visibly distinct in the code.  This also encourages users
to only put necessary work inside the critical section, and do to other work
once the lock is released.

This also adds a LockedGuardPtr class, which is a slightly cheaper version of
LockedPtr.  The relationship between LockedGuardPtr and LockedPtr is very much
like that between std::lock_guard and std::unique_lock.  It saves a branch in
the destructor, and in the case of std::mutex it also saves a small amount of
storage space (since LockedPtr is specialized for std::mutex to also store a
std::unique_lock).

Reviewed By: yfeldblum, djwatson

Differential Revision: D3530368

fbshipit-source-id: 72a4f457b3f18e8e8f4cc6713218f6882bb89818
parent 50f4f92c
...@@ -39,6 +39,8 @@ template <class LockedType, class Mutex, class LockPolicy> ...@@ -39,6 +39,8 @@ template <class LockedType, class Mutex, class LockPolicy>
class LockedPtrBase; class LockedPtrBase;
template <class LockedType, class LockPolicy> template <class LockedType, class LockPolicy>
class LockedPtr; class LockedPtr;
template <class LockedType, class LockPolicy = LockPolicyExclusive>
class LockedGuardPtr;
/** /**
* SynchronizedBase is a helper parent class for Synchronized<T>. * SynchronizedBase is a helper parent class for Synchronized<T>.
...@@ -112,6 +114,75 @@ class SynchronizedBase<Subclass, true> { ...@@ -112,6 +114,75 @@ class SynchronizedBase<Subclass, true> {
const std::chrono::duration<Rep, Period>& timeout) const { const std::chrono::duration<Rep, Period>& timeout) const {
return ConstLockedPtr(static_cast<const Subclass*>(this), timeout); return ConstLockedPtr(static_cast<const Subclass*>(this), timeout);
} }
/*
* Note: C++ 17 adds guaranteed copy elision. (http://wg21.link/P0135)
* Once compilers support this, it would be nice to add wguard() and rguard()
* methods that return LockedGuardPtr objects.
*/
/**
* Invoke a function while holding the lock exclusively.
*
* A reference to the datum will be passed into the function as its only
* argument.
*
* This can be used with a lambda argument for easily defining small critical
* sections in the code. For example:
*
* auto value = obj.withWLock([](auto& data) {
* data.doStuff();
* return data.getValue();
* });
*/
template <class Function>
auto withWLock(Function&& function) {
LockedGuardPtr<Subclass, LockPolicyExclusive> guardPtr(
static_cast<Subclass*>(this));
return function(*guardPtr);
}
template <class Function>
auto withWLock(Function&& function) const {
LockedGuardPtr<const Subclass, LockPolicyExclusive> guardPtr(
static_cast<const Subclass*>(this));
return function(*guardPtr);
}
/**
* Invoke a function while holding the lock exclusively.
*
* This is similar to withWLock(), but the function will be passed a
* LockedPtr rather than a reference to the data itself.
*
* This allows scopedUnlock() to be called on the LockedPtr argument if
* desired.
*/
template <class Function>
auto withWLockPtr(Function&& function) {
return function(wlock());
}
template <class Function>
auto withWLockPtr(Function&& function) const {
return function(wlock());
}
/**
* Invoke a function while holding an the lock in shared mode.
*
* A const reference to the datum will be passed into the function as its
* only argument.
*/
template <class Function>
auto withRLock(Function&& function) const {
LockedGuardPtr<const Subclass, LockPolicyShared> guardPtr(
static_cast<const Subclass*>(this));
return function(*guardPtr);
}
template <class Function>
auto withRLockPtr(Function&& function) const {
return function(rlock());
}
}; };
/** /**
...@@ -160,6 +231,57 @@ class SynchronizedBase<Subclass, false> { ...@@ -160,6 +231,57 @@ class SynchronizedBase<Subclass, false> {
ConstLockedPtr lock(const std::chrono::duration<Rep, Period>& timeout) const { ConstLockedPtr lock(const std::chrono::duration<Rep, Period>& timeout) const {
return ConstLockedPtr(static_cast<const Subclass*>(this), timeout); return ConstLockedPtr(static_cast<const Subclass*>(this), timeout);
} }
/*
* Note: C++ 17 adds guaranteed copy elision. (http://wg21.link/P0135)
* Once compilers support this, it would be nice to add guard() methods that
* return LockedGuardPtr objects.
*/
/**
* Invoke a function while holding the lock.
*
* A reference to the datum will be passed into the function as its only
* argument.
*
* This can be used with a lambda argument for easily defining small critical
* sections in the code. For example:
*
* auto value = obj.withLock([](auto& data) {
* data.doStuff();
* return data.getValue();
* });
*/
template <class Function>
auto withLock(Function&& function) {
LockedGuardPtr<Subclass, LockPolicyExclusive> guardPtr(
static_cast<Subclass*>(this));
return function(*guardPtr);
}
template <class Function>
auto withLock(Function&& function) const {
LockedGuardPtr<const Subclass, LockPolicyExclusive> guardPtr(
static_cast<const Subclass*>(this));
return function(*guardPtr);
}
/**
* Invoke a function while holding the lock exclusively.
*
* This is similar to withWLock(), but the function will be passed a
* LockedPtr rather than a reference to the data itself.
*
* This allows scopedUnlock() and getUniqueLock() to be called on the
* LockedPtr argument.
*/
template <class Function>
auto withLockPtr(Function&& function) {
return function(lock());
}
template <class Function>
auto withLockPtr(Function&& function) const {
return function(lock());
}
}; };
/** /**
...@@ -457,6 +579,8 @@ struct Synchronized : public SynchronizedBase< ...@@ -457,6 +579,8 @@ struct Synchronized : public SynchronizedBase<
friend class folly::LockedPtrBase; friend class folly::LockedPtrBase;
template <class LockedType, class LockPolicy> template <class LockedType, class LockPolicy>
friend class folly::LockedPtr; friend class folly::LockedPtr;
template <class LockedType, class LockPolicy>
friend class folly::LockedGuardPtr;
/** /**
* Helper constructors to enable Synchronized for * Helper constructors to enable Synchronized for
...@@ -481,6 +605,27 @@ struct Synchronized : public SynchronizedBase< ...@@ -481,6 +605,27 @@ struct Synchronized : public SynchronizedBase<
template <class SynchronizedType, class LockPolicy> template <class SynchronizedType, class LockPolicy>
class ScopedUnlocker; class ScopedUnlocker;
namespace detail {
/*
* A helper alias that resolves to "const T" if the template parameter
* is a const Synchronized<T>, or "T" if the parameter is not const.
*/
template <class SynchronizedType>
using SynchronizedDataType = typename std::conditional<
std::is_const<SynchronizedType>::value,
typename SynchronizedType::DataType const,
typename SynchronizedType::DataType>::type;
/*
* A helper alias that resolves to a ConstLockedPtr if the template parameter
* is a const Synchronized<T>, or a LockedPtr if the parameter is not const.
*/
template <class SynchronizedType>
using LockedPtrType = typename std::conditional<
std::is_const<SynchronizedType>::value,
typename SynchronizedType::ConstLockedPtr,
typename SynchronizedType::LockedPtr>::type;
} // detail
/** /**
* A helper base class for implementing LockedPtr. * A helper base class for implementing LockedPtr.
* *
...@@ -701,10 +846,7 @@ class LockedPtr : public LockedPtrBase< ...@@ -701,10 +846,7 @@ class LockedPtr : public LockedPtrBase<
LockPolicy>; LockPolicy>;
using UnlockerData = typename Base::UnlockerData; using UnlockerData = typename Base::UnlockerData;
// CDataType is the DataType with the appropriate const-qualification // CDataType is the DataType with the appropriate const-qualification
using CDataType = typename std::conditional< using CDataType = detail::SynchronizedDataType<SynchronizedType>;
std::is_const<SynchronizedType>::value,
typename SynchronizedType::DataType const,
typename SynchronizedType::DataType>::type;
public: public:
using DataType = typename SynchronizedType::DataType; using DataType = typename SynchronizedType::DataType;
...@@ -812,17 +954,61 @@ class LockedPtr : public LockedPtrBase< ...@@ -812,17 +954,61 @@ class LockedPtr : public LockedPtrBase<
} }
}; };
namespace detail { /**
/* * LockedGuardPtr is a simplified version of LockedPtr.
* A helper alias to select a ConstLockedPtr if the template parameter is a *
* const Synchronized<T>, or a LockedPtr if the parameter is not const. * It is non-movable, and supports fewer features than LockedPtr. However, it
* is ever-so-slightly more performant than LockedPtr. (The destructor can
* unconditionally release the lock, without requiring a conditional branch.)
*
* The relationship between LockedGuardPtr and LockedPtr is similar to that
* between std::lock_guard and std::unique_lock.
*/ */
template <class SynchronizedType> template <class SynchronizedType, class LockPolicy>
using LockedPtrType = typename std::conditional< class LockedGuardPtr {
std::is_const<SynchronizedType>::value, private:
typename SynchronizedType::ConstLockedPtr, // CDataType is the DataType with the appropriate const-qualification
typename SynchronizedType::LockedPtr>::type; using CDataType = detail::SynchronizedDataType<SynchronizedType>;
} // detail
public:
using DataType = typename SynchronizedType::DataType;
using MutexType = typename SynchronizedType::MutexType;
using Synchronized = typename std::remove_const<SynchronizedType>::type;
LockedGuardPtr() = delete;
/**
* Takes a Synchronized<T> and locks it.
*/
explicit LockedGuardPtr(SynchronizedType* parent) : parent_(parent) {
LockPolicy::lock(parent_->mutex_);
}
/**
* Destructor releases.
*/
~LockedGuardPtr() {
LockPolicy::unlock(parent_->mutex_);
}
/**
* Access the locked data.
*/
CDataType* operator->() const {
return &parent_->datum_;
}
/**
* Access the locked data.
*/
CDataType& operator*() const {
return parent_->datum_;
}
private:
// This is the entire state of LockedGuardPtr.
SynchronizedType* const parent_{nullptr};
};
/** /**
* Acquire locks for multiple Synchronized<T> objects, in a deadlock-safe * Acquire locks for multiple Synchronized<T> objects, in a deadlock-safe
......
...@@ -59,6 +59,10 @@ TYPED_TEST(SynchronizedTest, Basic) { ...@@ -59,6 +59,10 @@ TYPED_TEST(SynchronizedTest, Basic) {
testBasic<TypeParam>(); testBasic<TypeParam>();
} }
TYPED_TEST(SynchronizedTest, WithLock) {
testWithLock<TypeParam>();
}
TYPED_TEST(SynchronizedTest, Deprecated) { TYPED_TEST(SynchronizedTest, Deprecated) {
testDeprecated<TypeParam>(); testDeprecated<TypeParam>();
} }
......
...@@ -108,6 +108,7 @@ template <class Mutex> ...@@ -108,6 +108,7 @@ template <class Mutex>
typename std::enable_if<folly::LockTraits<Mutex>::is_shared>::type typename std::enable_if<folly::LockTraits<Mutex>::is_shared>::type
testBasicImpl() { testBasicImpl() {
folly::Synchronized<std::vector<int>, Mutex> obj; folly::Synchronized<std::vector<int>, Mutex> obj;
const auto& constObj = obj;
obj.wlock()->resize(1000); obj.wlock()->resize(1000);
...@@ -141,7 +142,6 @@ testBasicImpl() { ...@@ -141,7 +142,6 @@ testBasicImpl() {
obj.wlock()->front() = 2; obj.wlock()->front() = 2;
{ {
const auto& constObj = obj;
// contextualLock() on a const reference should grab a shared lock // contextualLock() on a const reference should grab a shared lock
auto lockedObj = constObj.contextualLock(); auto lockedObj = constObj.contextualLock();
EXPECT_EQ(2, lockedObj->front()); EXPECT_EQ(2, lockedObj->front());
...@@ -160,6 +160,7 @@ template <class Mutex> ...@@ -160,6 +160,7 @@ template <class Mutex>
typename std::enable_if<!folly::LockTraits<Mutex>::is_shared>::type typename std::enable_if<!folly::LockTraits<Mutex>::is_shared>::type
testBasicImpl() { testBasicImpl() {
folly::Synchronized<std::vector<int>, Mutex> obj; folly::Synchronized<std::vector<int>, Mutex> obj;
const auto& constObj = obj;
obj.lock()->resize(1000); obj.lock()->resize(1000);
...@@ -178,6 +179,12 @@ testBasicImpl() { ...@@ -178,6 +179,12 @@ testBasicImpl() {
EXPECT_EQ(1001, obj.lock()->size()); EXPECT_EQ(1001, obj.lock()->size());
} }
} }
{
auto lockedObj = constObj.lock();
EXPECT_EQ(1001, lockedObj->size());
EXPECT_EQ(10, lockedObj->back());
EXPECT_EQ(1000, obj2.lock()->size());
}
obj.lock()->front() = 2; obj.lock()->front() = 2;
...@@ -193,6 +200,160 @@ void testBasic() { ...@@ -193,6 +200,160 @@ void testBasic() {
testBasicImpl<Mutex>(); testBasicImpl<Mutex>();
} }
// testWithLock() version for shared lock types
template <class Mutex>
typename std::enable_if<folly::LockTraits<Mutex>::is_shared>::type
testWithLock() {
folly::Synchronized<std::vector<int>, Mutex> obj;
const auto& constObj = obj;
// Test withWLock() and withRLock()
obj.withWLock([](std::vector<int>& lockedObj) {
lockedObj.resize(1000);
lockedObj.push_back(10);
lockedObj.push_back(11);
});
obj.withWLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size());
});
constObj.withWLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size());
EXPECT_EQ(11, lockedObj.back());
});
obj.withRLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size());
EXPECT_EQ(11, lockedObj.back());
});
constObj.withRLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size());
});
#if __cpp_generic_lambdas >= 201304
obj.withWLock([](auto& lockedObj) { lockedObj.push_back(12); });
obj.withWLock(
[](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); });
constObj.withWLock([](const auto& lockedObj) {
EXPECT_EQ(1003, lockedObj.size());
EXPECT_EQ(12, lockedObj.back());
});
obj.withRLock([](const auto& lockedObj) {
EXPECT_EQ(1003, lockedObj.size());
EXPECT_EQ(12, lockedObj.back());
});
constObj.withRLock(
[](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); });
obj.withWLock([](auto& lockedObj) { lockedObj.pop_back(); });
#endif
// Test withWLockPtr() and withRLockPtr()
using SynchType = folly::Synchronized<std::vector<int>, Mutex>;
#if __cpp_generic_lambdas >= 201304
obj.withWLockPtr([](auto&& lockedObj) { lockedObj->push_back(13); });
obj.withRLockPtr([](auto&& lockedObj) {
EXPECT_EQ(1003, lockedObj->size());
EXPECT_EQ(13, lockedObj->back());
});
constObj.withRLockPtr([](auto&& lockedObj) {
EXPECT_EQ(1003, lockedObj->size());
EXPECT_EQ(13, lockedObj->back());
});
obj.withWLockPtr([&](auto&& lockedObj) {
lockedObj->push_back(14);
{
auto unlocker = lockedObj.scopedUnlock();
obj.wlock()->push_back(15);
}
EXPECT_EQ(15, lockedObj->back());
});
constObj.withWLockPtr([](auto&& lockedObj) {
EXPECT_EQ(1005, lockedObj->size());
EXPECT_EQ(15, lockedObj->back());
});
#else
obj.withWLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
lockedObj->push_back(13);
lockedObj->push_back(14);
lockedObj->push_back(15);
});
#endif
obj.withWLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
lockedObj->push_back(16);
EXPECT_EQ(1006, lockedObj->size());
});
constObj.withWLockPtr([](typename SynchType::ConstWLockedPtr&& lockedObj) {
EXPECT_EQ(1006, lockedObj->size());
EXPECT_EQ(16, lockedObj->back());
});
obj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
EXPECT_EQ(1006, lockedObj->size());
EXPECT_EQ(16, lockedObj->back());
});
constObj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
EXPECT_EQ(1006, lockedObj->size());
EXPECT_EQ(16, lockedObj->back());
});
}
// testWithLock() version for non-shared lock types
template <class Mutex>
typename std::enable_if<!folly::LockTraits<Mutex>::is_shared>::type
testWithLock() {
folly::Synchronized<std::vector<int>, Mutex> obj;
// Test withLock()
obj.withLock([](std::vector<int>& lockedObj) {
lockedObj.resize(1000);
lockedObj.push_back(10);
lockedObj.push_back(11);
});
obj.withLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size());
});
#if __cpp_generic_lambdas >= 201304
obj.withLock([](auto& lockedObj) { lockedObj.push_back(12); });
obj.withLock(
[](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); });
obj.withLock([](auto& lockedObj) { lockedObj.pop_back(); });
#endif
// Test withLockPtr()
using SynchType = folly::Synchronized<std::vector<int>, Mutex>;
#if __cpp_generic_lambdas >= 201304
obj.withLockPtr([](auto&& lockedObj) { lockedObj->push_back(13); });
obj.withLockPtr([](auto&& lockedObj) {
EXPECT_EQ(1003, lockedObj->size());
EXPECT_EQ(13, lockedObj->back());
});
obj.withLockPtr([&](auto&& lockedObj) {
lockedObj->push_back(14);
{
auto unlocker = lockedObj.scopedUnlock();
obj.lock()->push_back(15);
}
EXPECT_EQ(1005, lockedObj->size());
EXPECT_EQ(15, lockedObj->back());
});
#else
obj.withLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
lockedObj->push_back(13);
lockedObj->push_back(14);
lockedObj->push_back(15);
});
#endif
obj.withLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
lockedObj->push_back(16);
EXPECT_EQ(1006, lockedObj->size());
});
const auto& constObj = obj;
constObj.withLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
EXPECT_EQ(1006, lockedObj->size());
EXPECT_EQ(16, lockedObj->back());
});
}
// Testing the deprecated SYNCHRONIZED and SYNCHRONIZED_CONST APIs // Testing the deprecated SYNCHRONIZED and SYNCHRONIZED_CONST APIs
template <class Mutex> template <class Mutex>
void testDeprecated() { void testDeprecated() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment