From fbc491230fe62f739925e8113d05f4108e2738ef Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 7 Jun 2023 22:46:59 -0700 Subject: [PATCH] Q{Semaphore,ReadWriteLock}Private: reorganize the members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They are now ordered so that the mutex and the wait condition are in different cache lines, to avoid false sharing situations, if the types are holding the actual threading primitive structures, not mere pointers to some other structure elsewhere. For 64-bit systems: OS | mutex | cond | Remark --------------------+-------+-------+------------------ Darwin | 64 | 48 | FreeBSD | 8 | 8 | INTEGRITY | 8 | 8 | QMutex & QWaitCondition Linux | 24 | 48 | Always uses futex MinGW (Winpthreads) | 8 | 8 | Always uses futex MSVC (MS STL) | 32 | 16 | Always uses futex NetBSD | 48 | 40 | OpenBSD | 8 | 8 | QNX | ? | ? | Change-Id: I63b988479db546dabffcfffd176698e4f0097e90 Reviewed-by: Fabian Kosmale Reviewed-by: MÃ¥rten Nordheim --- src/corelib/thread/qreadwritelock_p.h | 5 +++-- src/corelib/thread/qsemaphore.cpp | 31 +++++++++++++++++++++------ src/corelib/thread/qwaitcondition_p.h | 8 +++++++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/corelib/thread/qreadwritelock_p.h b/src/corelib/thread/qreadwritelock_p.h index d1f887eb45b..596239e3256 100644 --- a/src/corelib/thread/qreadwritelock_p.h +++ b/src/corelib/thread/qreadwritelock_p.h @@ -45,9 +45,10 @@ public: explicit QReadWriteLockPrivate(bool isRecursive = false) : recursive(isRecursive) {} - QtPrivate::mutex mutex; - QtPrivate::condition_variable writerCond; + alignas(QtPrivate::IdealMutexAlignment) QtPrivate::condition_variable writerCond; QtPrivate::condition_variable readerCond; + + alignas(QtPrivate::IdealMutexAlignment) QtPrivate::mutex mutex; int readerCount = 0; int writerCount = 0; int waitingReaders = 0; diff --git a/src/corelib/thread/qsemaphore.cpp b/src/corelib/thread/qsemaphore.cpp index 73e88cf42c8..4915bde25a9 100644 --- a/src/corelib/thread/qsemaphore.cpp +++ b/src/corelib/thread/qsemaphore.cpp @@ -251,14 +251,31 @@ futexSemaphoreTryAcquire(QBasicAtomicInteger &u, int n, T timeout) return false; } -class QSemaphorePrivate { +namespace { namespace QtSemaphorePrivate { +using namespace QtPrivate; +struct Layout1 +{ + alignas(IdealMutexAlignment) QtPrivate::mutex mutex; + qsizetype avail = 0; + alignas(IdealMutexAlignment) QtPrivate::condition_variable cond; +}; + +struct Layout2 +{ + alignas(IdealMutexAlignment) QtPrivate::mutex mutex; + alignas(IdealMutexAlignment) QtPrivate::condition_variable cond; + qsizetype avail = 0; +}; + +// Choose Layout1 if it is smaller than Layout2. That happens for platforms +// where sizeof(mutex) is 64. +using Members = std::conditional_t; +} } // namespace QtSemaphorePrivate + +class QSemaphorePrivate : public QtSemaphorePrivate::Members +{ public: - explicit QSemaphorePrivate(int n) : avail(n) { } - - QtPrivate::mutex mutex; - QtPrivate::condition_variable cond; - - int avail; + explicit QSemaphorePrivate(qsizetype n) { avail = n; } }; /*! diff --git a/src/corelib/thread/qwaitcondition_p.h b/src/corelib/thread/qwaitcondition_p.h index cfb36ca30be..f3c931f3922 100644 --- a/src/corelib/thread/qwaitcondition_p.h +++ b/src/corelib/thread/qwaitcondition_p.h @@ -123,6 +123,14 @@ using condition_variable = std::condition_variable; #endif // C++11 threads +// Ideal alignment for mutex and condition_variable: it's the hardware +// interference size (size of a cache line) if the types are likely to contain +// the actual data structures, otherwise just that of a pointer. +static constexpr quintptr IdealMutexAlignment = + sizeof(QtPrivate::mutex) > sizeof(void *) && + sizeof(QtPrivate::condition_variable) > sizeof(void *) ? + 64 : alignof(void*); + } // namespace QtPrivate QT_END_NAMESPACE