QReadWriteLock: add tryLockForXxx overloads taking QDeadlineTimer

This propagates inside the internals, ending up in wait_until calls in
the internal std::condition_variable. For systems with proper support
for monotonic waiting (Linux, FreeBSD), this should improve performance.

We could even remove the hasExpired() check and pass a time point in the
past too. Right now, there's a minor performance drawback for
tryLockForXxxx(), because we will make at least two system calls to get
the time.

Change-Id: I6f518d59e63249ddbf43fffd1759fc5b2e40256a
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/corelib/thread/qreadwritelock.cpp

@@ -31,7 +31,7 @@ QT_BEGIN_NAMESPACE
 using namespace QReadWriteLockStates;
 namespace {
 
-using ms = std::chrono::milliseconds;
+using steady_clock = std::chrono::steady_clock;
 
 const auto dummyLockedForRead = reinterpret_cast<QReadWriteLockPrivate *>(quintptr(StateLockedForRead));
 const auto dummyLockedForWrite = reinterpret_cast<QReadWriteLockPrivate *>(quintptr(StateLockedForWrite));
@@ -40,9 +40,9 @@ inline bool isUncontendedLocked(const QReadWriteLockPrivate *d)
 }
 
 static bool contendedTryLockForRead(QAtomicPointer<QReadWriteLockPrivate> &d_ptr,
-                                    int timeout, QReadWriteLockPrivate *d);
+                                    QDeadlineTimer timeout, QReadWriteLockPrivate *d);
 static bool contendedTryLockForWrite(QAtomicPointer<QReadWriteLockPrivate> &d_ptr,
-                                     int timeout, QReadWriteLockPrivate *d);
+                                     QDeadlineTimer timeout, QReadWriteLockPrivate *d);
 
 /*! \class QReadWriteLock
     \inmodule QtCore
@@ -143,6 +143,7 @@ QReadWriteLock::~QReadWriteLock()
 */
 
 /*!
+    \fn bool QReadWriteLock::tryLockForRead(int timeout)
 
     Attempts to lock for reading. This function returns \c true if the
     lock was obtained; otherwise it returns \c false. If another thread
@@ -161,7 +162,25 @@ QReadWriteLock::~QReadWriteLock()
 
     \sa unlock(), lockForRead()
 */
-bool QReadWriteLock::tryLockForRead(int timeout)
+
+/*!
+    \overload
+    \since 6.6
+
+    Attempts to lock for reading. This function returns \c true if the lock was
+    obtained; otherwise it returns \c false. If another thread has locked for
+    writing, this function will wait until \a timeout expires for the lock to
+    become available.
+
+    If the lock was obtained, the lock must be unlocked with unlock()
+    before another thread can successfully lock it for writing.
+
+    It is not possible to lock for read if the thread already has
+    locked for write.
+
+    \sa unlock(), lockForRead()
+*/
+bool QReadWriteLock::tryLockForRead(QDeadlineTimer timeout)
 {
     // Fast case: non contended:
     QReadWriteLockPrivate *d = d_ptr.loadRelaxed();
@@ -171,7 +190,7 @@ bool QReadWriteLock::tryLockForRead(int timeout)
 }
 
 Q_NEVER_INLINE static bool contendedTryLockForRead(QAtomicPointer<QReadWriteLockPrivate> &d_ptr,
-                                                   int timeout, QReadWriteLockPrivate *d)
+                                                   QDeadlineTimer timeout, QReadWriteLockPrivate *d)
 {
     while (true) {
         if (d == nullptr) {
@@ -191,7 +210,7 @@ Q_NEVER_INLINE static bool contendedTryLockForRead(QAtomicPointer<QReadWriteLock
         }
 
         if (d == dummyLockedForWrite) {
-            if (!timeout)
+            if (timeout.hasExpired())
                 return false;
 
             // locked for write, assign a d_ptr and wait.
@@ -239,6 +258,8 @@ Q_NEVER_INLINE static bool contendedTryLockForRead(QAtomicPointer<QReadWriteLock
 */
 
 /*!
+    \fn QReadWriteLock::tryLockForWrite(int timeout)
+
     Attempts to lock for writing. This function returns \c true if the
     lock was obtained; otherwise it returns \c false. If another thread
     has locked for reading or writing, this function will wait for at
@@ -256,7 +277,25 @@ Q_NEVER_INLINE static bool contendedTryLockForRead(QAtomicPointer<QReadWriteLock
 
     \sa unlock(), lockForWrite()
 */
-bool QReadWriteLock::tryLockForWrite(int timeout)
+
+/*!
+    \overload
+    \since 6.6
+
+    Attempts to lock for writing. This function returns \c true if the lock was
+    obtained; otherwise it returns \c false. If another thread has locked for
+    reading or writing, this function will wait until \a timeout expires for
+    the lock to become available.
+
+    If the lock was obtained, the lock must be unlocked with unlock()
+    before another thread can successfully lock it.
+
+    It is not possible to lock for write if the thread already has
+    locked for read.
+
+    \sa unlock(), lockForWrite()
+*/
+bool QReadWriteLock::tryLockForWrite(QDeadlineTimer timeout)
 {
     // Fast case: non contended:
     QReadWriteLockPrivate *d = d_ptr.loadRelaxed();
@@ -266,7 +305,7 @@ bool QReadWriteLock::tryLockForWrite(int timeout)
 }
 
 Q_NEVER_INLINE static bool contendedTryLockForWrite(QAtomicPointer<QReadWriteLockPrivate> &d_ptr,
-                                                    int timeout, QReadWriteLockPrivate *d)
+                                                    QDeadlineTimer timeout, QReadWriteLockPrivate *d)
 {
     while (true) {
         if (d == nullptr) {
@@ -276,7 +315,7 @@ Q_NEVER_INLINE static bool contendedTryLockForWrite(QAtomicPointer<QReadWriteLoc
         }
 
         if (isUncontendedLocked(d)) {
-            if (!timeout)
+            if (timeout.hasExpired())
                 return false;
 
             // locked for either read or write, assign a d_ptr and wait.
@@ -370,23 +409,16 @@ void QReadWriteLock::unlock()
     }
 }
 
-bool QReadWriteLockPrivate::lockForRead(std::unique_lock<QtPrivate::mutex> &lock, int timeout)
+bool QReadWriteLockPrivate::lockForRead(std::unique_lock<QtPrivate::mutex> &lock, QDeadlineTimer timeout)
 {
     Q_ASSERT(!mutex.try_lock()); // mutex must be locked when entering this function
 
-    QElapsedTimer t;
-    if (timeout > 0)
-        t.start();
-
     while (waitingWriters || writerCount) {
-        if (timeout == 0)
+        if (timeout.hasExpired())
-            return false;
-        if (timeout > 0) {
-            auto elapsed = t.elapsed();
-            if (elapsed > timeout)
             return false;
+        if (!timeout.isForever()) {
             waitingReaders++;
-            readerCond.wait_for(lock, ms{timeout - elapsed});
+            readerCond.wait_until(lock, timeout.deadline<steady_clock>());
         } else {
             waitingReaders++;
             readerCond.wait(lock);
@@ -398,20 +430,12 @@ bool QReadWriteLockPrivate::lockForRead(std::unique_lock<QtPrivate::mutex> &lock
     return true;
 }
 
-bool QReadWriteLockPrivate::lockForWrite(std::unique_lock<QtPrivate::mutex> &lock, int timeout)
+bool QReadWriteLockPrivate::lockForWrite(std::unique_lock<QtPrivate::mutex> &lock, QDeadlineTimer timeout)
 {
     Q_ASSERT(!mutex.try_lock()); // mutex must be locked when entering this function
 
-    QElapsedTimer t;
-    if (timeout > 0)
-        t.start();
-
     while (readerCount || writerCount) {
-        if (timeout == 0)
+        if (timeout.hasExpired()) {
-            return false;
-        if (timeout > 0) {
-            auto elapsed = t.elapsed();
-            if (elapsed > timeout) {
             if (waitingReaders && !waitingWriters && !writerCount) {
                 // We timed out and now there is no more writers or waiting writers, but some
                 // readers were queued (probably because of us). Wake the waiting readers.
@@ -419,8 +443,9 @@ bool QReadWriteLockPrivate::lockForWrite(std::unique_lock<QtPrivate::mutex> &loc
             }
             return false;
         }
+        if (!timeout.isForever()) {
             waitingWriters++;
-            writerCond.wait_for(lock, ms{timeout - elapsed});
+            writerCond.wait_until(lock, timeout.deadline<steady_clock>());
         } else {
             waitingWriters++;
             writerCond.wait(lock);
@@ -448,7 +473,7 @@ static auto handleEquals(Qt::HANDLE handle)
     return [handle](QReadWriteLockPrivate::Reader reader) { return reader.handle == handle; };
 }
 
-bool QReadWriteLockPrivate::recursiveLockForRead(int timeout)
+bool QReadWriteLockPrivate::recursiveLockForRead(QDeadlineTimer timeout)
 {
     Q_ASSERT(recursive);
     auto lock = qt_unique_lock(mutex);
@@ -470,7 +495,7 @@ bool QReadWriteLockPrivate::recursiveLockForRead(int timeout)
     return true;
 }
 
-bool QReadWriteLockPrivate::recursiveLockForWrite(int timeout)
+bool QReadWriteLockPrivate::recursiveLockForWrite(QDeadlineTimer timeout)
 {
     Q_ASSERT(recursive);
     auto lock = qt_unique_lock(mutex);

src/corelib/thread/qreadwritelock.h

@@ -5,10 +5,10 @@
 #define QREADWRITELOCK_H
 
 #include <QtCore/qglobal.h>
+#include <QtCore/qdeadlinetimer.h>
 
 QT_BEGIN_NAMESPACE
 
-
 #if QT_CONFIG(thread)
 
 class QReadWriteLockPrivate;
@@ -26,17 +26,30 @@ public:
 #if QT_CORE_REMOVED_SINCE(6, 6)
     bool tryLockForRead();
 #endif
-    bool tryLockForRead(int timeout = 0);
+    QT_CORE_INLINE_SINCE(6, 6)
+    bool tryLockForRead(int timeout);
+    bool tryLockForRead(QDeadlineTimer timeout = {});
 
     QT_CORE_INLINE_SINCE(6, 6)
     void lockForWrite();
 #if QT_CORE_REMOVED_SINCE(6, 6)
     bool tryLockForWrite();
 #endif
-    bool tryLockForWrite(int timeout = 0);
+    QT_CORE_INLINE_SINCE(6, 6)
+    bool tryLockForWrite(int timeout);
+    bool tryLockForWrite(QDeadlineTimer timeout = {});
 
     void unlock();
 
+#ifndef Q_QDOC
+    // because tryLockForXxx(QDeadlineTimer::Forever) is the same
+    // as tryLockForXxx(0), which is not forever
+    bool tryLockForRead(QDeadlineTimer::ForeverConstant)
+    { lockForRead(); return true; }
+    bool tryLockForWrite(QDeadlineTimer::ForeverConstant)
+    { lockForWrite(); return true; }
+#endif
+
 private:
     Q_DISABLE_COPY(QReadWriteLock)
     QAtomicPointer<QReadWriteLockPrivate> d_ptr;
@@ -46,12 +59,22 @@ private:
 #if QT_CORE_INLINE_IMPL_SINCE(6, 6)
 void QReadWriteLock::lockForRead()
 {
-    tryLockForRead(-1);
+    tryLockForRead(QDeadlineTimer(QDeadlineTimer::Forever));
+}
+
+bool QReadWriteLock::tryLockForRead(int timeout)
+{
+    return tryLockForRead(QDeadlineTimer(timeout));
 }
 
 void QReadWriteLock::lockForWrite()
 {
-    tryLockForWrite(-1);
+    tryLockForWrite(QDeadlineTimer(QDeadlineTimer::Forever));
+}
+
+bool QReadWriteLock::tryLockForWrite(int timeout)
+{
+    return tryLockForWrite(QDeadlineTimer(timeout));
 }
 #endif // inline since 6.6
 
@@ -164,10 +187,12 @@ public:
 
     void lockForRead() noexcept { }
     bool tryLockForRead() noexcept { return true; }
+    bool tryLockForRead(QDeadlineTimer) noexcept { return true; }
     bool tryLockForRead(int timeout) noexcept { Q_UNUSED(timeout); return true; }
 
     void lockForWrite() noexcept { }
     bool tryLockForWrite() noexcept { return true; }
+    bool tryLockForWrite(QDeadlineTimer) noexcept { return true; }
     bool tryLockForWrite(int timeout) noexcept { Q_UNUSED(timeout); return true; }
 
     void unlock() noexcept { }

src/corelib/thread/qreadwritelock_p.h

@@ -55,8 +55,8 @@ public:
     const bool recursive;
 
     //Called with the mutex locked
-    bool lockForWrite(std::unique_lock<QtPrivate::mutex> &lock, int timeout);
+    bool lockForWrite(std::unique_lock<QtPrivate::mutex> &lock, QDeadlineTimer timeout);
-    bool lockForRead(std::unique_lock<QtPrivate::mutex> &lock, int timeout);
+    bool lockForRead(std::unique_lock<QtPrivate::mutex> &lock, QDeadlineTimer timeout);
     void unlock();
 
     //memory management
@@ -75,8 +75,8 @@ public:
     QVarLengthArray<Reader, 16> currentReaders;
 
     // called with the mutex unlocked
-    bool recursiveLockForWrite(int timeout);
+    bool recursiveLockForWrite(QDeadlineTimer timeout);
-    bool recursiveLockForRead(int timeout);
+    bool recursiveLockForRead(QDeadlineTimer timeout);
     void recursiveUnlock();
 
     static QReadWriteLockStates::StateForWaitCondition