From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2181) id 9DEB03848418; Tue, 22 Jun 2021 20:29:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9DEB03848418 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jonathan Wakely To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r12-1736] libstdc++: Simplify std::try_lock and std::lock further X-Act-Checkin: gcc X-Git-Author: Jonathan Wakely X-Git-Refname: refs/heads/master X-Git-Oldrev: b5a29741db11007e37d8d4ff977b89a8314abfda X-Git-Newrev: c556596119307f9ef1c9079ef2bd3a035dea355d Message-Id: <20210622202956.9DEB03848418@sourceware.org> Date: Tue, 22 Jun 2021 20:29:56 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Jun 2021 20:29:56 -0000 https://gcc.gnu.org/g:c556596119307f9ef1c9079ef2bd3a035dea355d commit r12-1736-gc556596119307f9ef1c9079ef2bd3a035dea355d Author: Jonathan Wakely Date: Tue Jun 22 13:35:19 2021 +0100 libstdc++: Simplify std::try_lock and std::lock further The std::try_lock and std::lock algorithms can use iteration instead of recursion when all lockables have the same type and can be held by an array of unique_lock objects. By making this change to __detail::__try_lock_impl it also benefits __detail::__lock_impl, which uses it. For std::lock we can just put the iterative version directly in std::lock, to avoid making any call to __detail::__lock_impl. Signed-off-by: Matthias Kretz Signed-off-by: Jonathan Wakely Co-authored-by: Matthias Kretz libstdc++-v3/ChangeLog: * include/std/mutex (lock): Replace recursion with iteration when lockables all have the same type. (__detail::__try_lock_impl): Likewise. Pass lockables as parameters, instead of a tuple. Always lock the first one, and recurse for the rest. (__detail::__lock_impl): Adjust call to __try_lock_impl. (__detail::__try_to_lock): Remove. * testsuite/30_threads/lock/3.cc: Check that mutexes are locked. * testsuite/30_threads/lock/4.cc: Also test non-heterogeneous arguments. * testsuite/30_threads/unique_lock/cons/60497.cc: Also check std::try_lock. * testsuite/30_threads/try_lock/5.cc: New test. Diff: --- libstdc++-v3/include/std/mutex | 108 +++++++++++++----- libstdc++-v3/testsuite/30_threads/lock/3.cc | 4 +- libstdc++-v3/testsuite/30_threads/lock/4.cc | 124 +++++++++++++++++++++ libstdc++-v3/testsuite/30_threads/try_lock/5.cc | 41 +++++++ .../testsuite/30_threads/unique_lock/cons/60497.cc | 6 + 5 files changed, 253 insertions(+), 30 deletions(-) diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index 5f2d8f9ee7b..c18ca1a1955 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -514,39 +514,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// @cond undocumented namespace __detail { + // Lock the last lockable, after all previous ones are locked. template - inline unique_lock<_Lockable> - __try_to_lock(_Lockable& __l) - { return unique_lock<_Lockable>{__l, try_to_lock}; } - - // Lock the last element of the tuple, after all previous ones are locked. - template - inline __enable_if_t<_Idx + 1 == sizeof...(_Lockables), int> - __try_lock_impl(tuple<_Lockables&...>& __lockables) + inline int + __try_lock_impl(_Lockable& __lockable) { - if (auto __lock = __detail::__try_to_lock(std::get<_Idx>(__lockables))) + if (unique_lock<_Lockable> __lock{__lockable, try_to_lock}) { __lock.release(); return -1; } else - return _Idx; + return 0; } - // Lock tuple elements starting from _Idx. - template - inline __enable_if_t<_Idx + 1 != sizeof...(_Lockables), int> - __try_lock_impl(tuple<_Lockables&...>& __lockables) + // Lock each lockable in turn. + // Use iteration if all lockables are the same type, recursion otherwise. + template + inline int + __try_lock_impl(_L0& __l0, _Lockables&... __lockables) { - if (auto __lock = __detail::__try_to_lock(std::get<_Idx>(__lockables))) +#if __cplusplus >= 201703L + if constexpr ((is_same_v<_L0, _Lockables> && ...)) { - int __idx = __detail::__try_lock_impl<_Idx + 1>(__lockables); + constexpr int _Np = 1 + sizeof...(_Lockables); + unique_lock<_L0> __locks[_Np] = { + {__l0, defer_lock}, {__lockables, defer_lock}... + }; + for (int __i = 0; __i < _Np; ++__i) + { + if (!__locks[__i].try_lock()) + { + const int __failed = __i; + while (__i--) + __locks[__i].unlock(); + return __failed; + } + } + for (auto& __l : __locks) + __l.release(); + return -1; + } + else +#endif + if (unique_lock<_L0> __lock{__l0, try_to_lock}) + { + int __idx = __detail::__try_lock_impl(__lockables...); if (__idx == -1) - __lock.release(); - return __idx; + { + __lock.release(); + return -1; + } + return __idx + 1; } else - return _Idx; + return 0; } } // namespace __detail @@ -562,12 +584,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * * Sequentially calls try_lock() on each argument. */ - template + template int - try_lock(_Lock1& __l1, _Lock2& __l2, _Lock3&... __l3) + try_lock(_L1& __l1, _L2& __l2, _L3&... __l3) { - auto __lockables = std::tie(__l1, __l2, __l3...); - return __detail::__try_lock_impl<0>(__lockables); + return __detail::__try_lock_impl(__l1, __l2, __l3...); } /// @cond undocumented @@ -589,8 +610,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION int __failed = 1; // index that couldn't be locked { unique_lock<_L0> __first(__l0); - auto __rest = std::tie(__l1...); - __failed += __detail::__try_lock_impl<0>(__rest); + __failed += __detail::__try_lock_impl(__l1...); if (!__failed) { __i = -1; // finished @@ -620,15 +640,45 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION * @post All arguments are locked. * * All arguments are locked via a sequence of calls to lock(), try_lock() - * and unlock(). If the call exits via an exception any locks that were - * obtained will be released. + * and unlock(). If this function exits via an exception any locks that + * were obtained will be released. */ template void lock(_L1& __l1, _L2& __l2, _L3&... __l3) { - int __i = 0; - __detail::__lock_impl(__i, 0, __l1, __l2, __l3...); +#if __cplusplus >= 201703L + if constexpr (is_same_v<_L1, _L2> && (is_same_v<_L1, _L3> && ...)) + { + constexpr int _Np = 2 + sizeof...(_L3); + unique_lock<_L1> __locks[] = { + {__l1, defer_lock}, {__l2, defer_lock}, {__l3, defer_lock}... + }; + int __first = 0; + do { + __locks[__first].lock(); + for (int __j = 1; __j < _Np; ++__j) + { + const int __idx = (__first + __j) % _Np; + if (!__locks[__idx].try_lock()) + { + for (int __k = __j; __k != 0; --__k) + __locks[(__first + __k - 1) % _Np].unlock(); + __first = __idx; + break; + } + } + } while (!__locks[__first].owns_lock()); + + for (auto& __l : __locks) + __l.release(); + } + else +#endif + { + int __i = 0; + __detail::__lock_impl(__i, 0, __l1, __l2, __l3...); + } } #if __cplusplus >= 201703L diff --git a/libstdc++-v3/testsuite/30_threads/lock/3.cc b/libstdc++-v3/testsuite/30_threads/lock/3.cc index 52136077be8..5fa118768bd 100644 --- a/libstdc++-v3/testsuite/30_threads/lock/3.cc +++ b/libstdc++-v3/testsuite/30_threads/lock/3.cc @@ -37,7 +37,7 @@ struct user_lock is_locked = true; } - bool try_lock() + bool try_lock() { return is_locked ? false : (is_locked = true); } void unlock() @@ -62,6 +62,8 @@ int main() { //heterogeneous types std::lock(m1, m2, m3); + VERIFY( !m1.try_lock() ); + VERIFY( !m3.try_lock() ); m1.unlock(); m2.unlock(); m3.unlock(); diff --git a/libstdc++-v3/testsuite/30_threads/lock/4.cc b/libstdc++-v3/testsuite/30_threads/lock/4.cc index 7ba15cba84b..130c1f62d73 100644 --- a/libstdc++-v3/testsuite/30_threads/lock/4.cc +++ b/libstdc++-v3/testsuite/30_threads/lock/4.cc @@ -73,6 +73,7 @@ int unreliable_lock::lock_on = -1; void test01() { unreliable_lock l1, l2, l3; + std::mutex m1, m2, m3; try { @@ -87,6 +88,60 @@ void test01() { VERIFY( false ); } + + // Repeat with non-heterogeneous arguments + + try + { + unreliable_lock::count = 0; + std::lock(l1, l2, l3, m1); + VERIFY( unreliable_lock::count == 3 ); + l1.unlock(); + l2.unlock(); + l3.unlock(); + VERIFY( !m1.try_lock() ); // already locked + m1.unlock(); + } + catch (...) + { + VERIFY( false ); + } + + try + { + unreliable_lock::count = 0; + std::lock(m1, l1, l2, l3); + VERIFY( unreliable_lock::count == 3 ); + VERIFY( !m1.try_lock() ); // already locked + m1.unlock(); + l1.unlock(); + l2.unlock(); + l3.unlock(); + } + catch (...) + { + VERIFY( false ); + } + + try + { + unreliable_lock::count = 0; + std::lock(l1, m1, l2, m2, l3, m3); + VERIFY( unreliable_lock::count == 3 ); + l1.unlock(); + l2.unlock(); + l3.unlock(); + VERIFY( !m1.try_lock() ); // already locked + VERIFY( !m2.try_lock() ); // already locked + VERIFY( !m3.try_lock() ); // already locked + m1.unlock(); + m2.unlock(); + m3.unlock(); + } + catch (...) + { + VERIFY( false ); + } } void test02() @@ -111,6 +166,31 @@ void test02() { VERIFY( false ); } + + // Repeat with non-heterogeneous arguments + + try + { + unreliable_lock::lock_on = 1; + while (unreliable_lock::lock_on < 3) + { + unreliable_lock::count = 0; + unreliable_lock l1, l2, l3; + std::mutex m1; + std::lock(l1, l2, l3, m1); + VERIFY( unreliable_lock::count > 3 ); + l1.unlock(); + l2.unlock(); + l3.unlock(); + VERIFY( !m1.try_lock() ); // already locked + m1.unlock(); + ++unreliable_lock::lock_on; + } + } + catch (...) + { + VERIFY( false ); + } } void test03() @@ -133,6 +213,50 @@ void test03() VERIFY( test ); ++unreliable_lock::throw_on; } + + // Repeat with non-heterogeneous arguments + + unreliable_lock::throw_on = 0; + while (unreliable_lock::throw_on < 3) + { + unreliable_lock::count = 0; + unreliable_lock l1, l2, l3; + std::mutex m1; + bool test = false; + try + { + std::lock(l1, l2, l3, m1); + } + catch (...) + { + test = true; + } + VERIFY( test ); + VERIFY( m1.try_lock() ); // m1 was not left locked by failed std::lock + m1.unlock(); + ++unreliable_lock::throw_on; + } + + unreliable_lock::throw_on = 0; + while (unreliable_lock::throw_on < 3) + { + unreliable_lock::count = 0; + unreliable_lock l1, l2, l3; + std::mutex m1; + bool test = false; + try + { + std::lock(m1, l1, l2, l3); + } + catch (...) + { + test = true; + } + VERIFY( test ); + VERIFY( m1.try_lock() ); // m1 was not left locked by failed std::lock + m1.unlock(); + ++unreliable_lock::throw_on; + } } int main() diff --git a/libstdc++-v3/testsuite/30_threads/try_lock/5.cc b/libstdc++-v3/testsuite/30_threads/try_lock/5.cc new file mode 100644 index 00000000000..a5574ff01fb --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/try_lock/5.cc @@ -0,0 +1,41 @@ +// { dg-do run { target c++11 } } + +#include +#include + +struct Lockable +{ + static int tries; + + void lock() { } + void unlock() { } + bool try_lock() { return ++tries != 2; } +}; + +int Lockable::tries = 0; + +void test01() +{ + Lockable l1, l2, l3; + std::mutex m1, m2; + + VERIFY( std::try_lock(l1, l2, l3) == 1 ); + VERIFY( Lockable::tries == 2 ); + + Lockable::tries = 0; + VERIFY( std::try_lock(m1, l1, l2, l3) == 2 ); + VERIFY( Lockable::tries == 2 ); + + Lockable::tries = 0; + VERIFY( std::try_lock(l1, l2, l3, m1) == 1 ); + VERIFY( Lockable::tries == 2 ); + + Lockable::tries = 0; + VERIFY( std::try_lock(m1, l1, l2, l3, m2) == 2 ); + VERIFY( Lockable::tries == 2 ); +} + +int main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/30_threads/unique_lock/cons/60497.cc b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/60497.cc index 8603e56790e..08698cea783 100644 --- a/libstdc++-v3/testsuite/30_threads/unique_lock/cons/60497.cc +++ b/libstdc++-v3/testsuite/30_threads/unique_lock/cons/60497.cc @@ -46,3 +46,9 @@ void test02() test_type l1, l2, l3; std::lock(l1, l2, l3); } + +void test03() +{ + test_type l1, l2, l3; + std::try_lock(l1, l2, l3); +}