QReadWriteLock: replace a QHash with a QVarLengthArray<., 16>

The QHash was used to track the recursion level of concurrent
readers. But it makes no sense to optimize this data structure
for O(1) lookup speed, since once you go beyond a few threads,
a mutex-based solution falls apart, anyway.

So use an unordered QVarLengthArray with preallocated capacity
16 instead. Lookup and erasure are now O(N), but tracking the
first 16 threads that concurrently lock this shared mutex for
reading no longer allocates memory (except for the Private
class that contains the data structure).

Results on my machine (recursive only):

  thread count: 16
  ********* Start testing of tst_QReadWriteLock *********
  Config: Using QtTest library 6.3.0, Qt 6.3.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 11.2.1 20211101), ubuntu 20.04
  [...]
   PASS   : tst_QReadWriteLock::uncontended(QReadWriteLock, read, recursive: 1)
   RESULT : tst_QReadWriteLock::uncontended():"QReadWriteLock, read, recursive: 1":
  -     51 msecs per iteration (total: 51, iterations: 1)
  +     38 msecs per iteration (total: 77, iterations: 2)
   PASS   : tst_QReadWriteLock::uncontended(QReadWriteLock, write, recursive: 1)
   RESULT : tst_QReadWriteLock::uncontended():"QReadWriteLock, write, recursive: 1":
  -     31 msecs per iteration (total: 62, iterations: 2)
  +     29 msecs per iteration (total: 58, iterations: 2)
   PASS   : tst_QReadWriteLock::uncontended(QReadWriteLock, read, recursive: 2)
   RESULT : tst_QReadWriteLock::uncontended():"QReadWriteLock, read, recursive: 2":
  -     89 msecs per iteration (total: 89, iterations: 1)
  +     75 msecs per iteration (total: 75, iterations: 1)
   PASS   : tst_QReadWriteLock::uncontended(QReadWriteLock, write, recursive: 2)
   RESULT : tst_QReadWriteLock::uncontended():"QReadWriteLock, write, recursive: 2":
  -     62 msecs per iteration (total: 62, iterations: 1)
  +     56 msecs per iteration (total: 56, iterations: 1)
   PASS   : tst_QReadWriteLock::uncontended(QReadWriteLock, read, recursive: 32)
   RESULT : tst_QReadWriteLock::uncontended():"QReadWriteLock, read, recursive: 32":
  -     1,357 msecs per iteration (total: 1,357, iterations: 1)
  +     1,154 msecs per iteration (total: 1,154, iterations: 1)
   PASS   : tst_QReadWriteLock::uncontended(QReadWriteLock, write, recursive: 32)
   RESULT : tst_QReadWriteLock::uncontended():"QReadWriteLock, write, recursive: 32":
  -     1,067 msecs per iteration (total: 1,067, iterations: 1)
  +     984 msecs per iteration (total: 984, iterations: 1)
  [...]
   PASS   : tst_QReadWriteLock::readOnly(QReadWriteLock, recursive: 1)
   RESULT : tst_QReadWriteLock::readOnly():"QReadWriteLock, recursive: 1":
  -     11,561 msecs per iteration (total: 11,561, iterations: 1)
  +     6,704 msecs per iteration (total: 6,704, iterations: 1)
   PASS   : tst_QReadWriteLock::readOnly(QReadWriteLock, recursive: 2)
   RESULT : tst_QReadWriteLock::readOnly():"QReadWriteLock, recursive: 2":
  -     16,173 msecs per iteration (total: 16,173, iterations: 1)
  +     13,053 msecs per iteration (total: 13,053, iterations: 1)
   PASS   : tst_QReadWriteLock::readOnly(QReadWriteLock, recursive: 32)
   RESULT : tst_QReadWriteLock::readOnly():"QReadWriteLock, recursive: 32":
  -     178,597 msecs per iteration (total: 178,597, iterations: 1)
  +     146,008 msecs per iteration (total: 146,008, iterations: 1)
  [...]
   PASS   : tst_QReadWriteLock::writeOnly(QReadWriteLock, recursive: 1)
   RESULT : tst_QReadWriteLock::writeOnly():"QReadWriteLock, recursive: 1":
  -     65,165 msecs per iteration (total: 65,165, iterations: 1)
  +     64,503 msecs per iteration (total: 64,503, iterations: 1)
   PASS   : tst_QReadWriteLock::writeOnly(QReadWriteLock, recursive: 2)
   RESULT : tst_QReadWriteLock::writeOnly():"QReadWriteLock, recursive: 2":
  -     70,665 msecs per iteration (total: 70,665, iterations: 1)
  +     69,812 msecs per iteration (total: 69,812, iterations: 1)
   PASS   : tst_QReadWriteLock::writeOnly(QReadWriteLock, recursive: 32)
   RESULT : tst_QReadWriteLock::writeOnly():"QReadWriteLock, recursive: 32":
  -     50,811 msecs per iteration (total: 50,811, iterations: 1)
  +     57,659 msecs per iteration (total: 57,659, iterations: 1)

Recursive mode is really, really expensive, even with this patch applied.

Change-Id: I36a164cf09462b69dce7e553f96afcebb49e3dbf
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

2 files changed, 24 insertions, 7 deletions

diff --git a/src/corelib/thread/qreadwritelock.cpp b/src/corelib/thread/qreadwritelock.cpp
index 894e852494..f22f3dab40 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 <algorithm>
+
 QT_BEGIN_NAMESPACE
 
 /*
@@ -534,6 +536,11 @@ void QReadWriteLockPrivate::unlock()
         readerCond.wakeAll();
 }
 
+static auto handleEquals(Qt::HANDLE handle)
+{
+    return [handle](QReadWriteLockPrivate::Reader reader) { return reader.handle == handle; };
+}
+
 bool QReadWriteLockPrivate::recursiveLockForRead(int timeout)
 {
     Q_ASSERT(recursive);
@@ -541,16 +548,18 @@ bool QReadWriteLockPrivate::recursiveLockForRead(int timeout)
 
     Qt::HANDLE self = QThread::currentThreadId();
 
-    auto it = currentReaders.find(self);
+    auto it = std::find_if(currentReaders.begin(), currentReaders.end(),
+                           handleEquals(self));
     if (it != currentReaders.end()) {
-        ++it.value();
+        ++it->recursionLevel;
         return true;
     }
 
     if (!lockForRead(timeout))
         return false;
 
-    currentReaders.insert(self, 1);
+    Reader r = {self, 1};
+    currentReaders.append(std::move(r));
     return true;
 }
 
@@ -583,12 +592,13 @@ void QReadWriteLockPrivate::recursiveUnlock()
             return;
         currentWriter = nullptr;
     } else {
-        auto it = currentReaders.find(self);
+        auto it = std::find_if(currentReaders.begin(), currentReaders.end(),
+                               handleEquals(self));
         if (it == currentReaders.end()) {
             qWarning("QReadWriteLock::unlock: unlocking from a thread that did not lock");
             return;
         } else {
-            if (--it.value() <= 0) {
+            if (--it->recursionLevel <= 0) {
                 currentReaders.erase(it);
                 readerCount--;
             }
diff --git a/src/corelib/thread/qreadwritelock_p.h b/src/corelib/thread/qreadwritelock_p.h
index a4d002b7f2..1adbce6658 100644
--- a/src/corelib/thread/qreadwritelock_p.h
+++ b/src/corelib/thread/qreadwritelock_p.h
@@ -53,7 +53,7 @@
 //
 
 #include <QtCore/private/qglobal_p.h>
-#include <QtCore/qhash.h>
+#include <QtCore/qvarlengtharray.h>
 #include <QtCore/qwaitcondition.h>
 
 QT_REQUIRE_CONFIG(thread);
@@ -87,13 +87,20 @@ public:
 
     // Recusive mutex handling
     Qt::HANDLE currentWriter = {};
-    QHash<Qt::HANDLE, int> currentReaders;
+
+    struct Reader {
+        Qt::HANDLE handle;
+        int recursionLevel;
+    };
+
+    QVarLengthArray<Reader, 16> currentReaders;
 
     // called with the mutex unlocked
     bool recursiveLockForWrite(int timeout);
     bool recursiveLockForRead(int timeout);
     void recursiveUnlock();
 };
+Q_DECLARE_TYPEINFO(QReadWriteLockPrivate::Reader, Q_PRIMITIVE_TYPE);
 
 QT_END_NAMESPACE