QReadWriteLock: replace (QWaitCondition, QMutex) with std::(condition_variable, mutex)

It turns out that QWaitCondition is a std::condition_variable_any. The
_any variant works with any mutex type, but requires a native mutex
for the native condition variable. So, QWaitCondition and
std::condition_variable_any both require two different mutexes: the
one the user passes in, and an internal one.

std::condition_variable, however, only works with std::mutex, and
since both are backed by the native API, condition_variable can use
the mutex passed in by the user instead of having to use an internal
one.

So, port from 2 × QWaitCondition + QMutex (2 × native cond + 2 ×
native mutex + Qt mutex) to std::condition_variable + std::mutex (2 ×
native cond + native mutex), shaving the overhead of two additional
mutexes (one Qt, one native) as well as the memory allocation
performed by QWaitCondition (for its Private).

Speeds up the writeOnly case by ~1/8th:

  PASS   : tst_QReadWriteLock::writeOnly(QReadWriteLock)
  RESULT : tst_QReadWriteLock::writeOnly():"QReadWriteLock":
  -   39,703 msecs per iteration (total: 39,703, iterations: 1)
  +   34,950 msecs per iteration (total: 34,950, iterations: 1)

Change-Id: I196cb13a27242fc1cb99723dfab5b2e5f8522143
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

1 files changed, 21 insertions, 16 deletions

diff --git a/src/corelib/thread/qreadwritelock.cpp b/src/corelib/thread/qreadwritelock.cpp
index 14654986a0..5aba05c1b9 100644
--- a/src/corelib/thread/qreadwritelock.cpp
+++ b/src/corelib/thread/qreadwritelock.cpp
@@ -50,6 +50,8 @@
 #include "private/qfreelist_p.h"
 #include "private/qlocking_p.h"
 
+#include <chrono>
+
 QT_BEGIN_NAMESPACE
 
 /*
@@ -65,6 +67,9 @@ QT_BEGIN_NAMESPACE
  */
 
 namespace {
+
+using ms = std::chrono::milliseconds;
+
 enum {
     StateMask = 0x3,
     StateLockedForRead = 0x1,
@@ -274,7 +279,7 @@ bool QReadWriteLock::tryLockForRead(int timeout)
             d = d_ptr.loadAcquire();
             continue;
         }
-        return d->lockForRead(timeout);
+        return d->lockForRead(lock, timeout);
     }
 }
 
@@ -378,7 +383,7 @@ bool QReadWriteLock::tryLockForWrite(int timeout)
             d = d_ptr.loadAcquire();
             continue;
         }
-        return d->lockForWrite(timeout);
+        return d->lockForWrite(lock, timeout);
     }
 }
 
@@ -461,9 +466,9 @@ QReadWriteLock::StateForWaitCondition QReadWriteLock::stateForWaitCondition() co
 
 }
 
-bool QReadWriteLockPrivate::lockForRead(int timeout)
+bool QReadWriteLockPrivate::lockForRead(std::unique_lock<std::mutex> &lock, int timeout)
 {
-    Q_ASSERT(!mutex.tryLock()); // mutex must be locked when entering this function
+    Q_ASSERT(!mutex.try_lock()); // mutex must be locked when entering this function
 
     QElapsedTimer t;
     if (timeout > 0)
@@ -477,10 +482,10 @@ bool QReadWriteLockPrivate::lockForRead(int timeout)
             if (elapsed > timeout)
                 return false;
             waitingReaders++;
-            readerCond.wait(&mutex, timeout - elapsed);
+            readerCond.wait_for(lock, ms{timeout - elapsed});
         } else {
             waitingReaders++;
-            readerCond.wait(&mutex);
+            readerCond.wait(lock);
         }
         waitingReaders--;
     }
@@ -489,9 +494,9 @@ bool QReadWriteLockPrivate::lockForRead(int timeout)
     return true;
 }
 
-bool QReadWriteLockPrivate::lockForWrite(int timeout)
+bool QReadWriteLockPrivate::lockForWrite(std::unique_lock<std::mutex> &lock, int timeout)
 {
-    Q_ASSERT(!mutex.tryLock()); // mutex must be locked when entering this function
+    Q_ASSERT(!mutex.try_lock()); // mutex must be locked when entering this function
 
     QElapsedTimer t;
     if (timeout > 0)
@@ -506,15 +511,15 @@ bool QReadWriteLockPrivate::lockForWrite(int timeout)
                 if (waitingReaders && !waitingWriters && !writerCount) {
                     // We timed out and now there is no more writers or waiting writers, but some
                     // readers were queueud (probably because of us). Wake the waiting readers.
-                    readerCond.wakeAll();
+                    readerCond.notify_all();
                 }
                 return false;
             }
             waitingWriters++;
-            writerCond.wait(&mutex, timeout - elapsed);
+            writerCond.wait_for(lock, ms{timeout - elapsed});
         } else {
             waitingWriters++;
-            writerCond.wait(&mutex);
+            writerCond.wait(lock);
         }
         waitingWriters--;
     }
@@ -527,11 +532,11 @@ bool QReadWriteLockPrivate::lockForWrite(int timeout)
 
 void QReadWriteLockPrivate::unlock()
 {
-    Q_ASSERT(!mutex.tryLock()); // mutex must be locked when entering this function
+    Q_ASSERT(!mutex.try_lock()); // mutex must be locked when entering this function
     if (waitingWriters)
-        writerCond.wakeOne();
+        writerCond.notify_one();
     else if (waitingReaders)
-        readerCond.wakeAll();
+        readerCond.notify_all();
 }
 
 bool QReadWriteLockPrivate::recursiveLockForRead(int timeout)
@@ -547,7 +552,7 @@ bool QReadWriteLockPrivate::recursiveLockForRead(int timeout)
         return true;
     }
 
-    if (!lockForRead(timeout))
+    if (!lockForRead(lock, timeout))
         return false;
 
     currentReaders.insert(self, 1);
@@ -565,7 +570,7 @@ bool QReadWriteLockPrivate::recursiveLockForWrite(int timeout)
         return true;
     }
 
-    if (!lockForWrite(timeout))
+    if (!lockForWrite(lock, timeout))
         return false;
 
     currentWriter = self;