diff options
author | Marc Mutz <marc.mutz@qt.io> | 2022-05-25 21:32:53 +0200 |
---|---|---|
committer | Marc Mutz <marc.mutz@qt.io> | 2022-05-26 16:43:11 +0000 |
commit | fda8eb17417e92f8f80587937463be77899c7bed (patch) | |
tree | 8d546a021ccf3d74883b3b280eb08b08d579cca4 /src | |
parent | 09c7457f4ab8021e7637e60e6c3d1f644639aec1 (diff) |
Optimize QThreadPrivate::addObjectWithPendingBindingStatusChange()
... and bindingStatus()
QBindingStatus* is the final state of the value chain in
m_statusOrPendingObjects, so we can use the Double-Checked Locking
Pattern to avoid locking the mutex when we already have a status - it
won't go away again, unlike the vector in the List state.
To enable the change, make the data member an atomic<>. All loads and
stores can continue to use memory_order::relaxed, except the loads of
a potential status, which have to acquire, and the store of the
status, which has to release. This creates the necessary
synchronizes-with relation. So even though we synchronize out of
middle of the mutex critical section in QThread::exec() this way,
there's no data race between QThread::exec() and a potential
bindingStatus() call.
Change-Id: I0e0b7bd305649fa5f56a0f8723fb75f2577b90dd
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/corelib/thread/qthread.cpp | 8 | ||||
-rw-r--r-- | src/corelib/thread/qthread_p.h | 15 |
2 files changed, 14 insertions, 9 deletions
diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index d067bd06d3..208420b8fe 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -519,7 +519,8 @@ void QtPrivate::BindingStatusOrList::setStatusAndClearList(QBindingStatus *statu QObjectPrivate::get(obj)->reinitBindingStorageAfterThreadMove(); delete pendingObjects; } - data = encodeBindingStatus(status); + // synchronizes-with the load-acquire in bindingStatus(): + data.store(encodeBindingStatus(status), std::memory_order_release); } /*! @@ -568,14 +569,13 @@ int QThread::exec() */ QBindingStatus *QtPrivate::BindingStatusOrList::addObjectUnlessAlreadyStatus(QObject *object) { - if (auto status = bindingStatus()) return status; List *objectList = list(); if (!objectList) { objectList = new List(); objectList->reserve(8); - data = encodeList(objectList); + data.store(encodeList(objectList), std::memory_order_relaxed); } objectList->push_back(object); return nullptr; @@ -583,6 +583,8 @@ QBindingStatus *QtPrivate::BindingStatusOrList::addObjectUnlessAlreadyStatus(QOb QBindingStatus *QThreadPrivate::addObjectWithPendingBindingStatusChange(QObject *obj) { + if (auto status = m_statusOrPendingObjects.bindingStatus()) + return status; QMutexLocker lock(&mutex); return m_statusOrPendingObjects.addObjectUnlessAlreadyStatus(obj); } diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h index 79476a940f..7909f00e37 100644 --- a/src/corelib/thread/qthread_p.h +++ b/src/corelib/thread/qthread_p.h @@ -122,6 +122,7 @@ public: data(encodeBindingStatus(status)) {} explicit BindingStatusOrList(List *list) noexcept : data(encodeList(list)) {} + // requires external synchronization: QBindingStatus *addObjectUnlessAlreadyStatus(QObject *object); void setStatusAndClearList(QBindingStatus *status) noexcept; @@ -133,17 +134,21 @@ public: static bool isList(quintptr data) noexcept { return data & 1; } static bool isNull(quintptr data) noexcept { return data == 0; } + // thread-safe: QBindingStatus *bindingStatus() const noexcept { - if (isBindingStatus(data)) - return reinterpret_cast<QBindingStatus *>(data); + // synchronizes-with the store-release in setStatusAndClearList(): + const auto d = data.load(std::memory_order_acquire); + if (isBindingStatus(d)) + return reinterpret_cast<QBindingStatus *>(d); else return nullptr; } + // requires external synchronization: List *list() const noexcept { - return decodeList(data); + return decodeList(data.load(std::memory_order_relaxed)); } private: @@ -165,7 +170,7 @@ private: return quintptr(list) | 1; } - quintptr data; + std::atomic<quintptr> data; }; } // namespace QtPrivate @@ -241,8 +246,6 @@ public: QBindingStatus *bindingStatus() { - - QMutexLocker lock(&mutex); return m_statusOrPendingObjects.bindingStatus(); } |