From c6bc549b6bec99241710f38fcc73adc7c1dde9a9 Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@qt.io>
Date: Sun, 17 Oct 2021 12:21:20 +0200
Subject: Optimize QObjectCompatProperty::notify

Do the check for inBindingWrapper() last.

Change-Id: I3d589c9fba524f465e35cd4cc0e65e3af376b419
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
---
 src/corelib/kernel/qproperty.cpp      | 38 ++++++++++++++++++++---------------
 src/corelib/kernel/qproperty_p.h      | 25 ++++++++++++++---------
 src/corelib/kernel/qpropertyprivate.h |  6 ++++++
 3 files changed, 44 insertions(+), 25 deletions(-)

(limited to 'src/corelib/kernel')

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index eb554eaeeb..9ca290d879 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -579,27 +579,33 @@ void QPropertyBindingData::notifyObservers(QUntypedPropertyData *propertyDataPtr
     QPropertyBindingDataPointer d{this};
 
     if (QPropertyObserverPointer observer = d.firstObserver()) {
-        auto status = storage ? storage->bindingStatus : nullptr;
-        QPropertyDelayedNotifications *delay;
-#ifndef QT_HAS_FAST_CURRENT_THREAD_ID
+        if (notifyObserver_helper(propertyDataPtr, observer, storage) == Evaluated) {
+            // evaluateBindings() can trash the observers. We need to re-fetch here.
+            if (QPropertyObserverPointer observer = d.firstObserver())
+                observer.notify(propertyDataPtr);
+        }
+    }
+}
+
+QPropertyBindingData::NotificationResult QPropertyBindingData::notifyObserver_helper(
+        QUntypedPropertyData *propertyDataPtr, QPropertyObserverPointer observer,
+        QBindingStorage *storage) const
+{
+#ifdef QT_HAS_FAST_CURRENT_THREAD_ID
+    QBindingStatus *status = storage ? storage->bindingStatus : nullptr;
+    if (!status || status->threadId != QThread::currentThreadId())
         status = &bindingStatus;
 #else
-        if (!status || status->threadId != QThread::currentThreadId())
-            status = &bindingStatus;
+    Q_UNUSED(storage);
+    QBindingStatus *status = &bindingStatus;
 #endif
-        delay = status->groupUpdateData;
-        if (delay) {
-            delay->addProperty(this, propertyDataPtr);
-            return;
-        }
-        observer.evaluateBindings(status);
-    } else {
-        return;
+    if (QPropertyDelayedNotifications *delay = status->groupUpdateData) {
+        delay->addProperty(this, propertyDataPtr);
+        return Delayed;
     }
 
-    // evaluateBindings() can trash the observers. We need to re-fetch here.
-    if (QPropertyObserverPointer observer = d.firstObserver())
-        observer.notify(propertyDataPtr);
+    observer.evaluateBindings(status);
+    return Evaluated;
 }
 
 int QPropertyBindingDataPointer::observerCount() const
diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index cc080ff82b..cafe211e71 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -600,8 +600,22 @@ public:
     {
         QBindingStorage *storage = qGetBindingStorage(owner());
         if (auto bd = storage->bindingData(this, false)) {
-            if (!inBindingWrapper(storage))
-                notify(bd);
+            // This partly duplicates QPropertyBindingData::notifyObservers because we want to
+            // check for inBindingWrapper() after checking for isNotificationDelayed() and
+            // firstObserver. This is because inBindingWrapper() is the most expensive check.
+            if (!bd->isNotificationDelayed()) {
+                QPropertyBindingDataPointer d{bd};
+                if (QPropertyObserverPointer observer = d.firstObserver()) {
+                    if (!inBindingWrapper(storage)) {
+                        if (bd->notifyObserver_helper(this, observer, storage)
+                                == QtPrivate::QPropertyBindingData::Evaluated) {
+                            // evaluateBindings() can trash the observers. We need to re-fetch here.
+                            if (QPropertyObserverPointer observer = d.firstObserver())
+                                observer.notify(this);
+                        }
+                    }
+                }
+            }
         }
         if constexpr (!std::is_null_pointer_v<decltype(Signal)>) {
             if constexpr (SignalTakesValue::value)
@@ -649,13 +663,6 @@ public:
         auto *storage = const_cast<QBindingStorage *>(qGetBindingStorage(owner()));
         return *storage->bindingData(const_cast<QObjectCompatProperty *>(this), true);
     }
-
-private:
-    void notify(const QtPrivate::QPropertyBindingData *binding)
-    {
-        if (binding)
-            binding->notifyObservers(this, qGetBindingStorage(owner()));
-    }
 };
 
 namespace QtPrivate {
diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h
index fec69f3a72..1c7f13046f 100644
--- a/src/corelib/kernel/qpropertyprivate.h
+++ b/src/corelib/kernel/qpropertyprivate.h
@@ -151,6 +151,7 @@ private:
 class QUntypedPropertyBinding;
 class QPropertyBindingPrivate;
 struct QPropertyBindingDataPointer;
+struct QPropertyObserverPointer;
 
 class QUntypedPropertyData
 {
@@ -334,6 +335,11 @@ private:
     quintptr d() const { return d_ref(); }
     void registerWithCurrentlyEvaluatingBinding_helper(BindingEvaluationState *currentBinding) const;
     void removeBinding_helper();
+
+    enum NotificationResult { Delayed, Evaluated };
+    NotificationResult notifyObserver_helper(
+            QUntypedPropertyData *propertyDataPtr, QPropertyObserverPointer observer,
+            QBindingStorage *storage) const;
 };
 
 template <typename T, typename Tag>
-- 
cgit v1.2.3