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>
This commit is contained in:
Marc Mutz 2017-02-16 12:09:17 +01:00 committed by Marc Mutz
parent 98bb6d23a1
commit 0b3f4f8f5a
2 changed files with 24 additions and 7 deletions

View File

@ -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--;
}

View File

@ -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