Inline the common case when there is no contention.

QMutex locking could be used in performence critical code (such
as QObject and co.) where the cost of a function call is
not neglectible. This would slow down the common case where
there is only one thread and no contention.

Since we need to keep the old symbol for binary compatibilty, introduce
new *Inline function.

Reviewed-by: Brad

7 files changed, 174 insertions, 67 deletions

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 8a4304e..f1262d6 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -893,7 +893,7 @@ QObject::~QObject()
                         if (c->next) c->next->prev = c->prev;
                     }
                     if (needToUnlock)
-                        m->unlock();
+                        m->unlockInline();
 
                     connectionList.first = c->nextConnectionList;
                     delete c;
@@ -917,7 +917,7 @@ QObject::~QObject()
             bool needToUnlock = QOrderedMutexLocker::relock(signalSlotMutex, m);
             //the node has maybe been removed while the mutex was unlocked in relock?
             if (!node || node->sender != sender) {
-                m->unlock();
+                m->unlockInline();
                 continue;
             }
             node->receiver = 0;
@@ -927,7 +927,7 @@ QObject::~QObject()
 
             node = node->next;
             if (needToUnlock)
-                m->unlock();
+                m->unlockInline();
         }
     }
 
@@ -2983,7 +2983,7 @@ bool QMetaObjectPrivate::disconnectHelper(QObjectPrivate::Connection *c,
             }
 
             if (needToUnlock)
-                receiverMutex->unlock();
+                receiverMutex->unlockInline();
 
             c->receiver = 0;
 
diff --git a/src/corelib/thread/qmutex.cpp b/src/corelib/thread/qmutex.cpp
index 43df13a..eb6f729 100644
--- a/src/corelib/thread/qmutex.cpp
+++ b/src/corelib/thread/qmutex.cpp
@@ -130,7 +130,7 @@ QMutex::QMutex(RecursionMode mode)
     \warning Destroying a locked mutex may result in undefined behavior.
 */
 QMutex::~QMutex()
-{ delete d; }
+{ delete static_cast<QMutexPrivate *>(d); }
 
 /*!
     Locks the mutex. If another thread has locked the mutex then this
@@ -146,6 +146,7 @@ QMutex::~QMutex()
 */
 void QMutex::lock()
 {
+    QMutexPrivate *d = static_cast<QMutexPrivate *>(this->d);
     Qt::HANDLE self;
 
     if (d->recursive) {
@@ -184,43 +185,7 @@ void QMutex::lock()
 
     bool isLocked = d->contenders == 0 && d->contenders.testAndSetAcquire(0, 1);
     if (!isLocked) {
-        int spinCount = 0;
-        int lastSpinCount = d->lastSpinCount;
-
-        enum { AdditionalSpins = 20, SpinCountPenalizationDivisor = 4 };
-        const int maximumSpinCount = lastSpinCount + AdditionalSpins;
-
-        do {
-            if (spinCount++ > maximumSpinCount) {
-                // puts("spinning useless, sleeping");
-                isLocked = d->contenders.fetchAndAddAcquire(1) == 0;
-                if (!isLocked) {
-#ifndef QT_NO_DEBUG
-                    if (d->owner == self)
-                        qWarning() << "QMutex::lock: Deadlock detected in thread" << d->owner;
-#endif
-
-                    // didn't get the lock, wait for it
-                    isLocked = d->wait();
-                    Q_ASSERT_X(isLocked, "QMutex::lock",
-                               "Internal error, infinite wait has timed out.");
-
-                    // don't need to wait for the lock anymore
-                    d->contenders.deref();
-                }
-                // decrease the lastSpinCount since we didn't actually get the lock by spinning
-                spinCount = -d->lastSpinCount / SpinCountPenalizationDivisor;
-                break;
-            }
-
-            isLocked = d->contenders == 0 && d->contenders.testAndSetAcquire(0, 1);
-        } while (!isLocked);
-
-        // adjust the last spin lock count
-        lastSpinCount = d->lastSpinCount;
-        d->lastSpinCount = spinCount >= 0
-                           ? qMax(lastSpinCount, spinCount)
-                           : lastSpinCount + spinCount;
+        lockInternal();
     }
 
 #ifndef QT_NO_DEBUG
@@ -247,6 +212,7 @@ void QMutex::lock()
 */
 bool QMutex::tryLock()
 {
+    QMutexPrivate *d = static_cast<QMutexPrivate *>(this->d);
     Qt::HANDLE self;
 
     if (d->recursive) {
@@ -310,6 +276,7 @@ bool QMutex::tryLock()
 */
 bool QMutex::tryLock(int timeout)
 {
+    QMutexPrivate *d = static_cast<QMutexPrivate *>(this->d);
     Qt::HANDLE self;
 
     if (d->recursive) {
@@ -366,8 +333,13 @@ bool QMutex::tryLock(int timeout)
 */
 void QMutex::unlock()
 {
-    Q_ASSERT_X(d->owner == QThread::currentThreadId(), "QMutex::unlock()",
-               "A mutex must be unlocked in the same thread that locked it.");
+    QMutexPrivate *d = static_cast<QMutexPrivate *>(this->d);
+#ifndef QT_NO_DEBUG
+    //note: if the mutex has been locked with (try)lockInline, d->owner could have not been set, and this would be a false warning
+    if ((d->owner || d->recursive) && d->owner != QThread::currentThreadId())
+        qWarning("QMutex::unlock(): A mutex must be unlocked in the same thread that locked it.");
+#endif
+
 
     if (d->recursive) {
         if (!--d->count) {
@@ -506,6 +478,87 @@ void QMutex::unlock()
     Use the constructor that takes a RecursionMode parameter instead.
 */
 
+/*!
+    \internal helper for lockInline()
+ */
+void QMutex::lockInternal()
+{
+    QMutexPrivate *d = static_cast<QMutexPrivate *>(this->d);
+    int spinCount = 0;
+    int lastSpinCount = d->lastSpinCount;
+
+    enum { AdditionalSpins = 20, SpinCountPenalizationDivisor = 4 };
+    const int maximumSpinCount = lastSpinCount + AdditionalSpins;
+
+#ifndef QT_NO_DEBUG
+    Qt::HANDLE self = QThread::currentThreadId();
+#endif
+
+    do {
+        if (spinCount++ > maximumSpinCount) {
+            // puts("spinning useless, sleeping");
+            bool isLocked = d->contenders.fetchAndAddAcquire(1) == 0;
+            if (!isLocked) {
+#ifndef QT_NO_DEBUG
+                if (d->owner == self)
+                    qWarning() << "QMutex::lock: Deadlock detected in thread" << d->owner;
+#endif
+
+                // didn't get the lock, wait for it
+                isLocked = d->wait();
+                Q_ASSERT_X(isLocked, "QMutex::lock",
+                            "Internal error, infinite wait has timed out.");
+
+                // don't need to wait for the lock anymore
+                d->contenders.deref();
+            }
+            // decrease the lastSpinCount since we didn't actually get the lock by spinning
+            spinCount = -d->lastSpinCount / SpinCountPenalizationDivisor;
+            break;
+        }
+    } while (d->contenders != 0 || !d->contenders.testAndSetAcquire(0, 1));
+
+    // adjust the last spin lock count
+    lastSpinCount = d->lastSpinCount;
+    d->lastSpinCount = spinCount >= 0
+                        ? qMax(lastSpinCount, spinCount)
+                        : lastSpinCount + spinCount;
+
+#ifndef QT_NO_DEBUG
+    d->owner = self;
+#endif
+}
+
+/*!
+    \internal
+*/
+void QMutex::unlockInternal()
+{
+#ifndef QT_NO_DEBUG
+    static_cast<QMutexPrivate *>(d)->owner = 0;
+#endif
+    static_cast<QMutexPrivate *>(d)->wakeUp();
+}
+
+/*!
+   \fn QMutex::lockInline()
+   \internal
+   inline version of QMutex::lock()
+*/
+
+/*!
+   \fn QMutex::unlockInline()
+   \internal
+   inline version of QMutex::unlock()
+*/
+
+/*!
+   \fn QMutex::tryLockInline()
+   \internal
+   inline version of QMutex::tryLock()
+*/
+
+
 QT_END_NAMESPACE
 
 #endif // QT_NO_THREAD
diff --git a/src/corelib/thread/qmutex.h b/src/corelib/thread/qmutex.h
index 509f300..710b794 100644
--- a/src/corelib/thread/qmutex.h
+++ b/src/corelib/thread/qmutex.h
@@ -43,6 +43,7 @@
 #define QMUTEX_H
 
 #include <QtCore/qglobal.h>
+#include <QtCore/qatomic.h>
 #include <new>
 
 QT_BEGIN_HEADER
@@ -53,7 +54,8 @@ QT_MODULE(Core)
 
 #ifndef QT_NO_THREAD
 
-class QMutexPrivate;
+class QAtomicInt;
+class QMutexData;
 
 class Q_CORE_EXPORT QMutex
 {
@@ -66,10 +68,13 @@ public:
     explicit QMutex(RecursionMode mode = NonRecursive);
     ~QMutex();
 
-    void lock();
-    bool tryLock();
+    void lock();     //### Qt5: make inline;
+    inline void lockInline();
+    bool tryLock();  //### Qt5: make inline;
     bool tryLock(int timeout);
-    void unlock();
+    inline bool tryLockInline();
+    void unlock();     //### Qt5: make inline;
+    inline void unlockInline();
 
 #if defined(QT3_SUPPORT)
     inline QT3_SUPPORT bool locked()
@@ -86,9 +91,11 @@ public:
 #endif
 
 private:
+    void lockInternal();
+    void unlockInternal();
     Q_DISABLE_COPY(QMutex)
 
-    QMutexPrivate *d;
+    QMutexData *d;
 };
 
 class Q_CORE_EXPORT QMutexLocker
@@ -99,7 +106,7 @@ public:
         Q_ASSERT_X((reinterpret_cast<quintptr>(m) & quintptr(1u)) == quintptr(0),
                    "QMutexLocker", "QMutex pointer is misaligned");
         if (m) {
-            m->lock();
+            m->lockInline();
             val = reinterpret_cast<quintptr>(m) | quintptr(1u);
         } else {
             val = 0;
@@ -111,7 +118,7 @@ public:
     {
         if ((val & quintptr(1u)) == quintptr(1u)) {
             val &= ~quintptr(1u);
-            mutex()->unlock();
+            mutex()->unlockInline();
         }
     }
 
@@ -119,7 +126,7 @@ public:
     {
         if (val) {
             if ((val & quintptr(1u)) == quintptr(0u)) {
-                mutex()->lock();
+                mutex()->lockInline();
                 val |= quintptr(1u);
             }
         }
@@ -145,6 +152,46 @@ private:
     quintptr val;
 };
 
+class QMutexData
+{
+    public:
+        QAtomicInt contenders;
+        const uint recursive : 1;
+        uint reserved : 31;
+    protected:
+        QMutexData(QMutex::RecursionMode mode);
+        ~QMutexData();
+};
+
+inline void QMutex::unlockInline()
+{
+    if (d->recursive) {
+        unlock();
+    } else if (!d->contenders.testAndSetRelease(1, 0)) {
+        unlockInternal();
+    }
+}
+
+inline bool QMutex::tryLockInline()
+{
+    if (d->recursive) {
+        return tryLock();
+    } else {
+        return d->contenders.testAndSetAcquire(0, 1);
+    }
+}
+
+inline void QMutex::lockInline()
+{
+    if (d->recursive) {
+        lock();
+    } else if(!tryLockInline()) {
+        lockInternal();
+    }
+}
+
+
+
 #else // QT_NO_THREAD
 
 
@@ -157,9 +204,11 @@ public:
     inline ~QMutex() {}
 
     static inline void lock() {}
-    static inline bool tryLock() { return true; }
-    static inline bool tryLock(int timeout) { Q_UNUSED(timeout); return true; }
-    static void unlock() {}
+    static inline void lockInline() {}
+    static inline bool tryLock(int timeout = 0) { Q_UNUSED(timeout); return true; }
+    static inline bool tryLockInline() { return true; }
+    static inline void unlock() {}
+    static inline void unlockInline() {}
 
 #if defined(QT3_SUPPORT)
     static inline QT3_SUPPORT bool locked() { return false; }
diff --git a/src/corelib/thread/qmutex_p.h b/src/corelib/thread/qmutex_p.h
index dce162a..6126423 100644
--- a/src/corelib/thread/qmutex_p.h
+++ b/src/corelib/thread/qmutex_p.h
@@ -56,10 +56,11 @@
 
 #include <QtCore/qglobal.h>
 #include <QtCore/qnamespace.h>
+#include <QtCore/qmutex.h>
 
 QT_BEGIN_NAMESPACE
 
-class QMutexPrivate {
+class QMutexPrivate : public QMutexData {
 public:
     QMutexPrivate(QMutex::RecursionMode mode);
     ~QMutexPrivate();
@@ -68,8 +69,6 @@ public:
     bool wait(int timeout = -1);
     void wakeUp();
 
-    const bool recursive;
-    QAtomicInt contenders;
     volatile int lastSpinCount;
     Qt::HANDLE owner;
     uint count;
@@ -83,6 +82,12 @@ public:
 #endif
 };
 
+inline QMutexData::QMutexData(QMutex::RecursionMode mode)
+    : recursive(mode == QMutex::Recursive)
+{}
+
+inline QMutexData::~QMutexData() {}
+
 QT_END_NAMESPACE
 
 #endif // QMUTEX_P_H
diff --git a/src/corelib/thread/qmutex_unix.cpp b/src/corelib/thread/qmutex_unix.cpp
index a58368c..7e7ef22 100644
--- a/src/corelib/thread/qmutex_unix.cpp
+++ b/src/corelib/thread/qmutex_unix.cpp
@@ -63,7 +63,7 @@ static void report_error(int code, const char *where, const char *what)
 
 
 QMutexPrivate::QMutexPrivate(QMutex::RecursionMode mode)
-    : recursive(mode == QMutex::Recursive), contenders(0), lastSpinCount(0), owner(0), count(0), wakeup(false)
+    : QMutexData(mode), lastSpinCount(0), owner(0), count(0), wakeup(false)
 {
     report_error(pthread_mutex_init(&mutex, NULL), "QMutex", "mutex init");
     report_error(pthread_cond_init(&cond, NULL), "QMutex", "cv init");
diff --git a/src/corelib/thread/qmutex_win.cpp b/src/corelib/thread/qmutex_win.cpp
index 9d58953..a810000 100644
--- a/src/corelib/thread/qmutex_win.cpp
+++ b/src/corelib/thread/qmutex_win.cpp
@@ -48,7 +48,7 @@
 QT_BEGIN_NAMESPACE
 
 QMutexPrivate::QMutexPrivate(QMutex::RecursionMode mode)
-    : recursive(mode == QMutex::Recursive), contenders(0), lastSpinCount(0), owner(0), count(0)
+    : QMutexData(mode), lastSpinCount(0), owner(0), count(0)
 {
     event = CreateEvent(0, FALSE, FALSE, 0);
     if (!event)
diff --git a/src/corelib/thread/qorderedmutexlocker_p.h b/src/corelib/thread/qorderedmutexlocker_p.h
index 702125c..375ded1 100644
--- a/src/corelib/thread/qorderedmutexlocker_p.h
+++ b/src/corelib/thread/qorderedmutexlocker_p.h
@@ -55,7 +55,7 @@
 
 QT_BEGIN_NAMESPACE
 
-class QMutex;
+#include <QtCore/qmutex.h>
 
 /*
   Locks 2 mutexes in a defined order, avoiding a recursive lock if
@@ -79,8 +79,8 @@ public:
     void relock()
     {
         if (!locked) {
-            if (mtx1) mtx1->lock();
-            if (mtx2) mtx2->lock();
+            if (mtx1) mtx1->lockInline();
+            if (mtx2) mtx2->lockInline();
             locked = true;
         }
     }
@@ -88,8 +88,8 @@ public:
     void unlock()
     {
         if (locked) {
-            if (mtx1) mtx1->unlock();
-            if (mtx2) mtx2->unlock();
+            if (mtx1) mtx1->unlockInline();
+            if (mtx2) mtx2->unlockInline();
             locked = false;
         }
     }
@@ -100,10 +100,10 @@ public:
         if (mtx1 == mtx2)
             return false;
         if (mtx1 < mtx2) {
-            mtx2->lock();
+            mtx2->lockInline();
             return true;
         }
-        if (!mtx2->tryLock()) {
+        if (!mtx2->tryLockInline()) {
             mtx1->unlock();
             mtx2->lock();
             mtx1->lock();