Significantly improve performance of binding evaluation

Avoid any QVariant or type dependent code in the cpp files.
Instead, let the binding wrapper determine if the value
has changed and return true/false accordingly.

This required also some reworking of the guard mechanism
for notified properties, where the guard function wrapper
now calls first the binding evaluation function and then
passes the result to the guard.

Change-Id: I350d07a508ccc0c5db7054a0efa4f270b6a78ec3
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

5 files changed, 75 insertions, 52 deletions

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index f7039cc62a..9374ce3a74 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -95,8 +95,8 @@ QPropertyBase::~QPropertyBase()
 QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding &binding,
                                                   void *propertyDataPtr,
                                                   void *staticObserver,
-                                                  void (*staticObserverCallback)(void*, void*),
-                                                  bool (*guardCallback)(void *, void*))
+                                                  QPropertyObserverCallback staticObserverCallback,
+                                                  QtPrivate::QPropertyGuardFunction guardCallback)
 {
     QPropertyBindingPrivatePtr oldBinding;
     QPropertyBindingPrivatePtr newBinding = binding.d;
diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h
index 8a712d395e..5d26c1c0f4 100644
--- a/src/corelib/kernel/qproperty.h
+++ b/src/corelib/kernel/qproperty.h
@@ -120,7 +120,7 @@ class Q_CORE_EXPORT QUntypedPropertyBinding
 {
 public:
     // writes binding result into dataPtr
-    using BindingEvaluationFunction = std::function<void(const QMetaType &metaType, void *dataPtr)>;
+    using BindingEvaluationFunction = QtPrivate::QPropertyBindingFunction;
 
     QUntypedPropertyBinding();
     QUntypedPropertyBinding(const QMetaType &metaType, BindingEvaluationFunction function, const QPropertyBindingSourceLocation &location);
@@ -151,10 +151,14 @@ class QPropertyBinding : public QUntypedPropertyBinding
     struct BindingAdaptor
     {
         Functor impl;
-        void operator()(const QMetaType &/*metaType*/, void *dataPtr)
+        bool operator()(const QMetaType &/*metaType*/, void *dataPtr)
         {
             PropertyType *propertyPtr = static_cast<PropertyType *>(dataPtr);
-            *propertyPtr = impl();
+            PropertyType newValue = impl();
+            if (newValue == *propertyPtr)
+                return false;
+            *propertyPtr = std::move(newValue);
+            return true;
         }
     };
 
@@ -352,20 +356,12 @@ namespace Qt {
     }
 }
 
-namespace detail {
-    template <typename F>
-    struct ExtractClassFromFunctionPointer;
-
-    template<typename T, typename C>
-    struct ExtractClassFromFunctionPointer<T C::*> { using Class = C; };
-}
-
 template <typename T, auto Callback, auto ValueGuard=nullptr>
 class QNotifiedProperty
 {
 public:
     using value_type = T;
-    using Class = typename detail::ExtractClassFromFunctionPointer<decltype(Callback)>::Class;
+    using Class = typename QtPrivate::detail::ExtractClassFromFunctionPointer<decltype(Callback)>::Class;
 private:
     static bool constexpr ValueGuardModifiesArgument = std::is_invocable_r_v<bool, decltype(ValueGuard), Class, T&>;
     static bool constexpr CallbackAcceptsOldValue = std::is_invocable_v<decltype(Callback), Class, T>;
@@ -378,17 +374,8 @@ public:
             !HasValueGuard,
         "Guard has wrong signature");
 private:
-    // type erased guard functions, casts its arguments to the correct types
-    static constexpr bool (*GuardTEHelper)(void *, void*) = [](void *o, void *newValue){
-        if constexpr (HasValueGuard) { // Guard->* is invalid if Guard == nullptr
-            return (reinterpret_cast<Class *>(o)->*(ValueGuard))(*static_cast<T *>(newValue));
-        } else {
-            Q_UNUSED(o); // some compilers complain about unused variables
-            Q_UNUSED(newValue);
-            return true;
-        }
-    };
-    static constexpr bool(*GuardTE)(void *, void*)  = HasValueGuard ? GuardTEHelper : nullptr;
+    static constexpr QtPrivate::QPropertyGuardFunction GuardTE =
+            QtPrivate::QPropertyGuardFunctionHelper<T, Class, ValueGuard>::guard;
 public:
 
     QNotifiedProperty() = default;
diff --git a/src/corelib/kernel/qpropertybinding.cpp b/src/corelib/kernel/qpropertybinding.cpp
index 85dc59902a..99d17757de 100644
--- a/src/corelib/kernel/qpropertybinding.cpp
+++ b/src/corelib/kernel/qpropertybinding.cpp
@@ -104,31 +104,26 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
     BindingEvaluationState evaluationFrame(this);
 
     bool changed = false;
-    if (metaType.id() == QMetaType::Bool) {
-        auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
-        bool newValue = false;
-        evaluationFunction(metaType, &newValue);
-        if (!error.hasError()) {
-            bool updateAllowed = true;
-            if (hasStaticObserver && staticGuardCallback)
-                updateAllowed = staticGuardCallback(staticObserver, &newValue);
-            if (updateAllowed && propertyPtr->extraBit() != newValue) {
+
+    if (hasStaticObserver && staticGuardCallback) {
+        if (metaType.id() == QMetaType::Bool) {
+            auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
+            bool newValue = propertyPtr->extraBit();
+            changed = staticGuardCallback(metaType, &newValue, evaluationFunction, staticObserver);
+            if (changed && !error.hasError())
                 propertyPtr->setExtraBit(newValue);
-                changed = true;
-            }
+        } else {
+            changed = staticGuardCallback(metaType, propertyDataPtr, evaluationFunction, staticObserver);
         }
     } else {
-        QVariant resultVariant(metaType.id(), nullptr);
-        evaluationFunction(metaType, resultVariant.data());
-        if (!error.hasError()) {
-            bool updateAllowed = true;
-            if (hasStaticObserver && staticGuardCallback)
-                updateAllowed = staticGuardCallback(staticObserver, resultVariant.data());
-            if (updateAllowed && !metaType.equals(propertyDataPtr, resultVariant.constData())) {
-                changed = true;
-                metaType.destruct(propertyDataPtr);
-                metaType.construct(propertyDataPtr, resultVariant.constData());
-            }
+        if (metaType.id() == QMetaType::Bool) {
+            auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
+            bool newValue = propertyPtr->extraBit();
+            changed = evaluationFunction(metaType, &newValue);
+            if (changed && !error.hasError())
+                propertyPtr->setExtraBit(newValue);
+        } else {
+            changed = evaluationFunction(metaType, propertyDataPtr);
         }
     }
 
diff --git a/src/corelib/kernel/qpropertybinding_p.h b/src/corelib/kernel/qpropertybinding_p.h
index 7722a4b7c5..9a18ff9015 100644
--- a/src/corelib/kernel/qpropertybinding_p.h
+++ b/src/corelib/kernel/qpropertybinding_p.h
@@ -80,8 +80,8 @@ private:
         ObserverArray inlineDependencyObservers;
         struct {
             void *staticObserver;
-            void (*staticObserverCallback)(void*, void*);
-            bool (*staticGuardCallback)(void*, void*);
+            QtPrivate::QPropertyObserverCallback staticObserverCallback;
+            QtPrivate::QPropertyGuardFunction staticGuardCallback;
         };
     };
     QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
@@ -108,7 +108,8 @@ public:
 
     void setDirty(bool d) { dirty = d; }
     void setProperty(void *propertyPtr) { propertyDataPtr = propertyPtr; }
-    void setStaticObserver(void *observer, void (*callback)(void*, void*), bool (*guardCallback)(void *, void*))
+    void setStaticObserver(void *observer, QtPrivate::QPropertyObserverCallback callback,
+                           QtPrivate::QPropertyGuardFunction guardCallback)
     {
         if (observer) {
             if (!hasStaticObserver) {
diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h
index 145f2c66b9..91d9204996 100644
--- a/src/corelib/kernel/qpropertyprivate.h
+++ b/src/corelib/kernel/qpropertyprivate.h
@@ -61,9 +61,17 @@ class QUntypedPropertyBinding;
 class QPropertyBindingPrivate;
 using QPropertyBindingPrivatePtr = QExplicitlySharedDataPointer<QPropertyBindingPrivate>;
 struct QPropertyBasePointer;
+class QMetaType;
 
 namespace QtPrivate {
 
+// writes binding result into dataPtr
+using QPropertyBindingFunction = std::function<bool(const QMetaType &metaType, void *dataPtr)>;
+
+using QPropertyGuardFunction = bool(*)(const QMetaType &, void *dataPtr,
+                                       QPropertyBindingFunction, void *owner);
+using QPropertyObserverCallback = void (*)(void *, void *);
+
 class Q_CORE_EXPORT QPropertyBase
 {
     // Mutable because the address of the observer of the currently evaluating binding is stored here, for
@@ -84,8 +92,8 @@ public:
 
     QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
                                        void *propertyDataPtr, void *staticObserver = nullptr,
-                                       void (*staticObserverCallback)(void *, void *) = nullptr,
-                                       bool (*guardCallback)(void *, void *) = nullptr);
+                                       QPropertyObserverCallback staticObserverCallback = nullptr,
+                                       QPropertyGuardFunction guardCallback = nullptr);
     QPropertyBindingPrivate *binding();
 
     void evaluateIfDirty();
@@ -211,6 +219,38 @@ private:
     quintptr *d = nullptr;
 };
 
+namespace detail {
+    template <typename F>
+    struct ExtractClassFromFunctionPointer;
+
+    template<typename T, typename C>
+    struct ExtractClassFromFunctionPointer<T C::*> { using Class = C; };
+}
+
+// type erased guard functions, casts its arguments to the correct types
+template<typename T, typename Class, auto Guard, bool = std::is_same_v<decltype(Guard), std::nullptr_t>>
+struct QPropertyGuardFunctionHelper
+{
+    static constexpr QPropertyGuardFunction guard = nullptr;
+};
+template<typename T, typename Class, auto Guard>
+struct QPropertyGuardFunctionHelper<T, Class, Guard, false>
+{
+    static auto guard(const QMetaType &metaType, void *dataPtr,
+                      QPropertyBindingFunction eval, void *owner) -> bool
+    {
+        T t;
+        eval(metaType, &t);
+        if (!(static_cast<Class *>(owner)->*Guard)(t))
+            return false;
+        T *data = static_cast<T *>(dataPtr);
+        if (*data == t)
+            return false;
+        *data = std::move(t);
+        return true;
+    };
+};
+
 } // namespace QtPrivate
 
 QT_END_NAMESPACE