From 71fea09e1a8d1fc5d9cca7c504f45b77725c0e21 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Fri, 25 Jun 2021 15:10:23 +0200
Subject: Avoid TLS access for groupUpdateData

By putting the groupUpdateData pointer into the same thread local as the
binding status, we avoid having to fetch two thread_local variables.
Moreover, we can reuse the caching mechanism which we have in place for
QBindingStatus to avoid costly TLS lookups.

Pick-to: 6.2
Change-Id: Iaea515763510daab83f89b8e74f35a80965d6965
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
---
 src/corelib/kernel/qproperty.cpp      | 18 ++++++++++++++----
 src/corelib/kernel/qproperty.h        |  7 +++++--
 src/corelib/kernel/qproperty_p.h      |  2 +-
 src/corelib/kernel/qpropertyprivate.h |  3 +++
 4 files changed, 23 insertions(+), 7 deletions(-)

(limited to 'src')

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index 50ec3807d3..db202ad725 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -198,7 +198,7 @@ struct QPropertyDelayedNotifications
     }
 };
 
-static thread_local QPropertyDelayedNotifications *groupUpdateData = nullptr;
+static thread_local QBindingStatus bindingStatus;
 
 /*!
     \since 6.2
@@ -222,6 +222,7 @@ static thread_local QPropertyDelayedNotifications *groupUpdateData = nullptr;
  */
 void Qt::beginPropertyUpdateGroup()
 {
+    QPropertyDelayedNotifications *& groupUpdateData = bindingStatus.groupUpdateData;
     if (!groupUpdateData)
         groupUpdateData = new QPropertyDelayedNotifications;
     ++groupUpdateData->ref;
@@ -241,6 +242,7 @@ void Qt::beginPropertyUpdateGroup()
  */
 void Qt::endPropertyUpdateGroup()
 {
+    QPropertyDelayedNotifications *& groupUpdateData = bindingStatus.groupUpdateData;
     auto *data = groupUpdateData;
     Q_ASSERT(data->ref);
     if (--data->ref)
@@ -467,8 +469,6 @@ QPropertyBindingData::QPropertyBindingData(QPropertyBindingData &&other) : d_ptr
     QPropertyBindingDataPointer::fixupAfterMove(this);
 }
 
-static thread_local QBindingStatus bindingStatus;
-
 BindingEvaluationState::BindingEvaluationState(QPropertyBindingPrivate *binding, QBindingStatus *status)
     : binding(binding)
 {
@@ -544,13 +544,23 @@ void QPropertyBindingData::registerWithCurrentlyEvaluatingBinding_helper(Binding
 }
 
 void QPropertyBindingData::notifyObservers(QUntypedPropertyData *propertyDataPtr) const
+{
+    notifyObservers(propertyDataPtr, nullptr);
+}
+
+void QPropertyBindingData::notifyObservers(QUntypedPropertyData *propertyDataPtr, QBindingStorage *storage) const
 {
     if (isNotificationDelayed())
         return;
     QPropertyBindingDataPointer d{this};
 
     if (QPropertyObserverPointer observer = d.firstObserver()) {
-        auto *delay = groupUpdateData;
+        auto status = storage ? storage->bindingStatus : nullptr;
+        QPropertyDelayedNotifications *delay;
+        if (status && status->threadId == QThread::currentThreadId())
+            delay = status->groupUpdateData;
+        else
+            delay = bindingStatus.groupUpdateData;
         if (delay) {
             delay->addProperty(this, propertyDataPtr);
             return;
diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h
index 008748840d..133c7a7e14 100644
--- a/src/corelib/kernel/qproperty.h
+++ b/src/corelib/kernel/qproperty.h
@@ -944,8 +944,10 @@ struct QBindingStatus
     QtPrivate::BindingEvaluationState *currentlyEvaluatingBinding = nullptr;
     QtPrivate::CompatPropertySafePoint *currentCompatProperty = nullptr;
     Qt::HANDLE threadId = nullptr;
+    QPropertyDelayedNotifications *groupUpdateData = nullptr;
 };
 
+
 struct QBindingStorageData;
 class Q_CORE_EXPORT QBindingStorage
 {
@@ -955,6 +957,7 @@ class Q_CORE_EXPORT QBindingStorage
     template<typename Class, typename T, auto Offset, auto Setter, auto Signal>
     friend class QObjectCompatProperty;
     friend class QObjectPrivate;
+    friend class QtPrivate::QPropertyBindingData;
 public:
     QBindingStorage();
     ~QBindingStorage();
@@ -1184,7 +1187,7 @@ private:
     void notify(const QtPrivate::QPropertyBindingData *binding)
     {
         if (binding)
-            binding->notifyObservers(this);
+            binding->notifyObservers(this, qGetBindingStorage(owner()));
         if constexpr (HasSignal) {
             if constexpr (SignalTakesValue::value)
                 (owner()->*Signal)(this->valueBypassingBindings());
@@ -1322,7 +1325,7 @@ public:
         auto *storage = const_cast<QBindingStorage *>(qGetBindingStorage(owner()));
         auto bd = storage->bindingData(const_cast<QObjectComputedProperty *>(this), false);
         if (bd)
-            bd->notifyObservers(this);
+            bd->notifyObservers(this, qGetBindingStorage(owner()));
     }
 };
 
diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index ba1eb7adf2..07f9099ee3 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -592,7 +592,7 @@ private:
     void notify(const QtPrivate::QPropertyBindingData *binding)
     {
         if (binding)
-            binding->notifyObservers(this);
+            binding->notifyObservers(this, qGetBindingStorage(owner()));
     }
 };
 
diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h
index 5b6a9557f9..2fa82f6342 100644
--- a/src/corelib/kernel/qpropertyprivate.h
+++ b/src/corelib/kernel/qpropertyprivate.h
@@ -59,6 +59,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class QBindingStorage;
+
 namespace QtPrivate {
 // QPropertyBindingPrivatePtr operates on a RefCountingMixin solely so that we can inline
 // the constructor and copy constructor
@@ -302,6 +304,7 @@ public:
     }
     void registerWithCurrentlyEvaluatingBinding() const;
     void notifyObservers(QUntypedPropertyData *propertyDataPtr) const;
+    void notifyObservers(QUntypedPropertyData *propertyDataPtr, QBindingStorage *storage) const;
 private:
     /*!
         \internal
-- 
cgit v1.2.3