]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
libstdc++: Simplify std::try_lock and std::lock further
authorJonathan Wakely <jwakely@redhat.com>
Tue, 22 Jun 2021 12:35:19 +0000 (13:35 +0100)
committerJonathan Wakely <jwakely@redhat.com>
Tue, 22 Jun 2021 20:17:25 +0000 (21:17 +0100)
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<L> 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 <m.kretz@gsi.de>
Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
Co-authored-by: Matthias Kretz <m.kretz@gsi.de>
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.

libstdc++-v3/include/std/mutex
libstdc++-v3/testsuite/30_threads/lock/3.cc
libstdc++-v3/testsuite/30_threads/lock/4.cc
libstdc++-v3/testsuite/30_threads/try_lock/5.cc [new file with mode: 0644]
libstdc++-v3/testsuite/30_threads/unique_lock/cons/60497.cc

index 5f2d8f9ee7b6bb1283c8c5b10a1fdb74633b0340..c18ca1a195533058a5a3d70f418a6080a147dc6d 100644 (file)
@@ -514,39 +514,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   /// @cond undocumented
   namespace __detail
   {
+    // Lock the last lockable, after all previous ones are locked.
     template<typename _Lockable>
-      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<int _Idx, typename... _Lockables>
-      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<int _Idx, typename... _Lockables>
-      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<typename _L0, typename... _Lockables>
+      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<typename _Lock1, typename _Lock2, typename... _Lock3>
+  template<typename _L1, typename _L2, typename... _L3>
     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<typename _L1, typename _L2, typename... _L3>
     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
index 52136077be80ab5770aebf51b7d0c002144f3a6a..5fa118768bdf2a5d4ea83d55d94eaab2e49aa791 100644 (file)
@@ -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();
index 7ba15cba84b49e7d3c04be99eb94cd35d7582d07..130c1f62d7326f3864bce390559847b4c5030ea9 100644 (file)
@@ -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 (file)
index 0000000..a5574ff
--- /dev/null
@@ -0,0 +1,41 @@
+// { dg-do run { target c++11 } }
+
+#include <mutex>
+#include <testsuite_hooks.h>
+
+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();
+}
index 8603e56790e9663bc4b23cf3acddb6428145a84b..08698cea783eb674c2d26cd5ae20753bf578507d 100644 (file)
@@ -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);
+}