Hoist the recursive mutex check out of the inner loop

A non-recursive mutex doesn't suddenly become recursive, so we don't
need to check it multiple times.

Change-Id: Id040254b6142d320a7bd3111491082ad09968404
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>

diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp
index 1a064a1..fa4fddc 100644 (file)
--- a/src/corelib/thread/qmutex.cpp
+++ b/src/corelib/thread/qmutex.cpp
@@ -152,7 +152,7 @@ QMutex::QMutex(RecursionMode mode)
 QMutex::~QMutex()
 {
     QMutexData *d = d_ptr.load();
-    if (quintptr(d) > 0x3 && d->recursive) {
+    if (isRecursive()) {
         delete static_cast<QRecursiveMutexPrivate *>(d);
     } else if (d) {
 #ifndef QT_LINUX_FUTEX
@@ -234,7 +234,12 @@ bool QBasicMutex::isRecursive() {
     QMutexData *d = d_ptr.load();
     if (quintptr(d) <= 0x3)
         return false;
+#ifdef QT_LINUX_FUTEX
+    Q_ASSERT(d->recursive);
+    return true;
+#else
     return d->recursive;
+#endif
 }
 
 
@@ -333,6 +338,9 @@ bool QBasicMutex::isRecursive() {
  */
 bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 {
+    if (isRecursive())
+        return static_cast<QRecursiveMutexPrivate *>(d_ptr.load())->lock(timeout);
+
     while (!fastTryLock()) {
         QMutexData *copy = d_ptr.loadAcquire();
         if (!copy) // if d is 0, the mutex is unlocked
@@ -349,8 +357,6 @@ bool QBasicMutex::lockInternal(int timeout) QT_MUTEX_LOCK_NOEXCEPT
             }
             copy = newD;
             //the d->refCount is already 1 the deref will occurs when we unlock
-        } else if (copy->recursive) {
-             return static_cast<QRecursiveMutexPrivate *>(copy)->lock(timeout);
         }
 
         QMutexPrivate *d = static_cast<QMutexPrivate *>(copy);

diff --git a/src/corelib/thread/qmutex_linux.cpp b/src/corelib/thread/qmutex_linux.cpp
index 3fb3db2..e14660c 100644 (file)
--- a/src/corelib/thread/qmutex_linux.cpp
+++ b/src/corelib/thread/qmutex_linux.cpp
@@ -116,40 +116,46 @@ static inline QMutexData *dummyFutexValue()
 
 bool QBasicMutex::lockInternal(int timeout) Q_DECL_NOTHROW
 {
+    // we're here because fastTryLock() has just failed
+    QMutexData *d = d_ptr.load();
+    if (quintptr(d) > 0x3) { //d == dummyLocked() || d == dummyFutexValue()
+        Q_ASSERT(d->recursive);
+        return static_cast<QRecursiveMutexPrivate *>(d)->lock(timeout);
+    }
+
     QElapsedTimer elapsedTimer;
     if (timeout >= 1)
         elapsedTimer.start();
 
     while (!fastTryLock()) {
-        QMutexData *d = d_ptr.load();
+        d = d_ptr.load();
         if (!d) // if d is 0, the mutex is unlocked
             continue;
-
-        if (quintptr(d) <= 0x3) { //d == dummyLocked() || d == dummyFutexValue()
-            if (timeout == 0)
-                return false;
-            while (d_ptr.fetchAndStoreAcquire(dummyFutexValue()) != 0) {
-                struct timespec ts, *pts = 0;
-                if (timeout >= 1) {
-                    // recalculate the timeout
-                    qint64 xtimeout = qint64(timeout) * 1000 * 1000;
-                    xtimeout -= elapsedTimer.nsecsElapsed();
-                    if (xtimeout <= 0) {
-                        // timer expired after we returned
-                        return false;
-                    }
-                    ts.tv_sec = xtimeout / Q_INT64_C(1000) / 1000 / 1000;
-                    ts.tv_nsec = xtimeout % (Q_INT64_C(1000) * 1000 * 1000);
-                    pts = &ts;
-                }
-                int r = _q_futex(&d_ptr, FUTEX_WAIT, quintptr(dummyFutexValue()), pts);
-                if (r != 0 && errno == ETIMEDOUT)
+        if (timeout == 0)
+            return false;
+
+        // the mutex is locked already, set a bit indicating we're waiting
+        while (d_ptr.fetchAndStoreAcquire(dummyFutexValue()) != 0) {
+            struct timespec ts, *pts = 0;
+            if (timeout >= 1) {
+                // recalculate the timeout
+                qint64 xtimeout = qint64(timeout) * 1000 * 1000;
+                xtimeout -= elapsedTimer.nsecsElapsed();
+                if (xtimeout <= 0) {
+                    // timer expired after we returned
                     return false;
+                }
+                ts.tv_sec = xtimeout / Q_INT64_C(1000) / 1000 / 1000;
+                ts.tv_nsec = xtimeout % (Q_INT64_C(1000) * 1000 * 1000);
+                pts = &ts;
             }
-            return true;
+
+            // successfully set the waiting bit, now sleep
+            int r = _q_futex(&d_ptr, FUTEX_WAIT, quintptr(dummyFutexValue()), pts);
+            if (r != 0 && errno == ETIMEDOUT)
+                return false;
         }
-        Q_ASSERT(d->recursive);
-        return static_cast<QRecursiveMutexPrivate *>(d)->lock(timeout);
+        return true;
     }
     Q_ASSERT(d_ptr.load());
     return true;