QML: Introduce write accessors to QQmlAccessors

Same idea as the read accessors, but with a slight difference: the
property setters do emit signals, so we can't do a direct property write
and have to call the setter. However, it does circumvent the meta-calls.

There is one gotcha: if a property is intercepted (e.g. by a Behavior),
we still have to do the meta-call in order to dispatch the write to the
interceptor.

According to valgrind, this saves 138 instructions on x86 for every
"accessible" property write.

Change-Id: I07dbac95613415559ffa1691734a5af7c84721fc
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

6 files changed, 94 insertions, 119 deletions

diff --git a/src/qml/qml/qqmlabstractbinding_p.h b/src/qml/qml/qqmlabstractbinding_p.h
index 45221a4bcd..d25c0c6288 100644
--- a/src/qml/qml/qqmlabstractbinding_p.h
+++ b/src/qml/qml/qqmlabstractbinding_p.h
@@ -91,6 +91,8 @@ public:
 
     inline QQmlAbstractBinding *nextBinding() const;
 
+    inline bool canUseAccessor() const
+    { return m_nextBinding.flag2(); }
 
     struct RefCount {
         RefCount() : refCount(0) {}
@@ -112,8 +114,15 @@ protected:
     inline void setNextBinding(QQmlAbstractBinding *);
 
     int m_targetIndex;
+
+    // Pointer is the target object to which the binding binds
+    // flag1 is the updating flag
+    // flag2 is the enabled flag
     QFlagPointer<QObject> m_target;
+
     // Pointer to the next binding in the linked list of bindings.
+    // flag1 is used for addedToObject
+    // flag2 indicates if an accessor is can be used (i.e. there is no interceptor on the target)
     QFlagPointer<QQmlAbstractBinding> m_nextBinding;
 };
 
diff --git a/src/qml/qml/qqmlaccessors_p.h b/src/qml/qml/qqmlaccessors_p.h
index 55562a5307..e98663adfe 100644
--- a/src/qml/qml/qqmlaccessors_p.h
+++ b/src/qml/qml/qqmlaccessors_p.h
@@ -102,11 +102,15 @@ class QQmlNotifier;
         } \
     } while (false);
 
-#define QML_PRIVATE_ACCESSOR(clazz, cpptype, name, variable) \
+#define QML_PRIVATE_ACCESSOR(clazz, cpptype, name, variable, setter) \
     static void clazz ## _ ## name ## Read(QObject *o, void *rv) \
     { \
         clazz ## Private *d = clazz ## Private::get(static_cast<clazz *>(o)); \
         *static_cast<cpptype *>(rv) = d->variable; \
+    } \
+    static void clazz ## _ ## name ## Write(QObject *o, void *rv) \
+    { \
+        static_cast<clazz *>(o)->setter(*static_cast<cpptype *>(rv)); \
     }
 
 #define QML_PROPERTY_NAME(name) #name, sizeof #name - 1
@@ -115,6 +119,7 @@ class QQmlAccessors
 {
 public:
     void (*read)(QObject *object, void *output);
+    void (*write)(QObject *object, void *output);
     void (*notifier)(QObject *object, QQmlNotifier **notifier);
 };
 
diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp
index 9f86e448ef..78d0a4386f 100644
--- a/src/qml/qml/qqmlbinding.cpp
+++ b/src/qml/qml/qqmlbinding.cpp
@@ -43,6 +43,7 @@
 #include "qqmlcontext.h"
 #include "qqmlinfo.h"
 #include "qqmldata_p.h"
+#include "qqmlaccessors_p.h"
 #include <private/qqmlprofiler_p.h>
 #include <private/qqmlexpression_p.h>
 #include <private/qqmlscriptstring_p.h>
@@ -210,21 +211,10 @@ protected:
     }
 };
 
-#define QUICK_STORE(cpptype, conversion) \
-        { \
-            cpptype o = (conversion); \
-            int status = -1; \
-            void *argv[] = { &o, 0, &status, &flags }; \
-            QMetaObject::metacall(m_target.data(), QMetaObject::WriteProperty, coreIndex, argv); \
-            return true; \
-        } \
-
-
 template<int StaticPropType>
 class GenericBinding: public QQmlBinding
 {
 protected:
-
     void doUpdate(QQmlBinding *binding, const DeleteWatcher &watcher,
                   QQmlPropertyPrivate::WriteFlags flags, QV4::Scope &scope,
                   const QV4::ScopedFunctionObject &f) Q_DECL_OVERRIDE Q_DECL_FINAL
@@ -238,13 +228,8 @@ protected:
         binding->QQmlJavaScriptExpression::evaluate(callData, &isUndefined, scope);
 
         bool error = false;
-        if (!watcher.wasDeleted() && isAddedToObject() && !hasError()) {
-            if (StaticPropType == QMetaType::UnknownType) {
-                error = !write(scope.result, isUndefined, flags);
-            } else {
-                error = !fastWrite(scope.result, isUndefined, flags);
-            }
-        }
+        if (!watcher.wasDeleted() && isAddedToObject() && !hasError())
+            error = !write(scope.result, isUndefined, flags);
 
         if (!watcher.wasDeleted()) {
 
@@ -264,99 +249,72 @@ protected:
         ep->dereferenceScarceResources();
     }
 
-private:
     // Returns true if successful, false if an error description was set on expression
-    Q_ALWAYS_INLINE bool fastWrite(const QV4::Value &result, bool isUndefined,
-                                   QQmlPropertyPrivate::WriteFlags flags)
+    Q_ALWAYS_INLINE bool write(const QV4::Value &result, bool isUndefined,
+                               QQmlPropertyPrivate::WriteFlags flags)
     {
-        int coreIndex = getPropertyCoreIndex();
-
-        Q_ASSERT(m_target.data());
-
-        if (Q_LIKELY(!isUndefined && coreIndex != -1 )) {
-            switch (StaticPropType) {
+        QQmlPropertyData pd = getPropertyData();
+        int propertyType = StaticPropType; // If the binding is specialized to a type, the if and switch below will be constant-folded.
+        if (propertyType == QMetaType::UnknownType)
+            propertyType = pd.propType;
+        Q_ASSERT(targetObject());
+
+        if (Q_LIKELY(!isUndefined && !pd.isValueTypeVirtual())) {
+            switch (propertyType) {
+            case QMetaType::Bool:
+                if (result.isBoolean())
+                    return doStore<bool>(result.booleanValue(), pd, flags);
+                else
+                    return doStore<bool>(result.toBoolean(), pd, flags);
             case QMetaType::Int:
                 if (result.isInteger())
-                    QUICK_STORE(int, result.integerValue())
+                    return doStore<int>(result.integerValue(), pd, flags);
                 else if (result.isNumber())
-                    QUICK_STORE(int, result.doubleValue())
+                    return doStore<int>(result.doubleValue(), pd, flags);
                 break;
             case QMetaType::Double:
                 if (result.isNumber())
-                    QUICK_STORE(double, result.asDouble())
+                    return doStore<double>(result.asDouble(), pd, flags);
                 break;
             case QMetaType::Float:
                 if (result.isNumber())
-                    QUICK_STORE(float, result.asDouble())
+                    return doStore<float>(result.asDouble(), pd, flags);
                 break;
             case QMetaType::QString:
                 if (result.isString())
-                    QUICK_STORE(QString, result.toQStringNoThrow())
+                    return doStore<QString>(result.toQStringNoThrow(), pd, flags);
                 break;
             default:
                 if (const QV4::QQmlValueTypeWrapper *vtw = result.as<const QV4::QQmlValueTypeWrapper>()) {
-                    if (vtw->d()->valueType->typeId == StaticPropType) {
-                        return vtw->write(m_target.data(), coreIndex);
+                    if (vtw->d()->valueType->typeId == pd.propType) {
+                        return vtw->write(m_target.data(), pd.coreIndex);
                     }
                 }
                 break;
             }
         }
 
-        return slowWrite(result, isUndefined, flags);
+        return slowWrite(pd, result, isUndefined, flags);
     }
-};
 
-// Returns true if successful, false if an error description was set on expression
-Q_ALWAYS_INLINE bool QQmlBinding::write(const QV4::Value &result, bool isUndefined,
-                                        QQmlPropertyPrivate::WriteFlags flags)
-{
-    Q_ASSERT(m_target.data());
-
-    int coreIndex = getPropertyCoreIndex();
-    int propertyType = getPropertyType();
-
-    Q_ASSERT(m_target.data());
-
-    if (Q_LIKELY(!isUndefined && coreIndex != -1 )) {
-        switch (propertyType) {
-        case QMetaType::Int:
-            if (result.isInteger())
-                QUICK_STORE(int, result.integerValue())
-            else if (result.isNumber())
-                QUICK_STORE(int, result.doubleValue())
-            break;
-        case QMetaType::Double:
-            if (result.isNumber())
-                QUICK_STORE(double, result.asDouble())
-            break;
-        case QMetaType::Float:
-            if (result.isNumber())
-                QUICK_STORE(float, result.asDouble())
-            break;
-        case QMetaType::QString:
-            if (result.isString())
-                QUICK_STORE(QString, result.toQStringNoThrow())
-            break;
-        default:
-            if (const QV4::QQmlValueTypeWrapper *vtw = result.as<const QV4::QQmlValueTypeWrapper>()) {
-                if (vtw->d()->valueType->typeId == propertyType) {
-                    return vtw->write(m_target.data(), coreIndex);
-                }
-            }
-            break;
+    template <typename T>
+    Q_ALWAYS_INLINE bool doStore(T value, const QQmlPropertyData &pd, QQmlPropertyPrivate::WriteFlags flags) const
+    {
+        void *o = &value;
+        if (pd.hasAccessors() && canUseAccessor()) {
+            pd.accessors->write(m_target.data(), o);
+        } else {
+            int status = -1;
+            void *argv[] = { o, 0, &status, &flags };
+            QMetaObject::metacall(targetObject(), QMetaObject::WriteProperty, pd.coreIndex, argv);
         }
+        return true;
     }
+};
 
-    return slowWrite(result, isUndefined, flags);
-}
-#undef QUICK_STORE
-
-Q_NEVER_INLINE bool QQmlBinding::slowWrite(const QV4::Value &result, bool isUndefined,
-                                           QQmlPropertyPrivate::WriteFlags flags)
+Q_NEVER_INLINE bool QQmlBinding::slowWrite(const QQmlPropertyData &core, const QV4::Value &result,
+                                           bool isUndefined, QQmlPropertyPrivate::WriteFlags flags)
 {
-    QQmlPropertyData core = getPropertyData();
-
     QQmlEngine *engine = context()->engine;
     QV8Engine *v8engine = QQmlEnginePrivate::getV8Engine(engine);
 
@@ -505,6 +463,13 @@ void QQmlBinding::setEnabled(bool e, QQmlPropertyPrivate::WriteFlags flags)
     setEnabledFlag(e);
     setNotifyOnValueChanged(e);
 
+    m_nextBinding.clearFlag2();
+    if (auto interceptorMetaObject = QQmlInterceptorMetaObject::get(targetObject())) {
+        int coreIndex = getPropertyCoreIndex();
+        if (coreIndex != -1 && !interceptorMetaObject->intercepts(coreIndex))
+            m_nextBinding.setFlag2();
+    }
+
     if (e)
         update(flags);
 }
@@ -611,38 +576,26 @@ Q_ALWAYS_INLINE int QQmlBinding::getPropertyCoreIndex() const
     }
 }
 
-int QQmlBinding::getPropertyType() const
-{
-    int coreIndex;
-    int valueTypeIndex = QQmlPropertyData::decodeValueTypePropertyIndex(m_targetIndex, &coreIndex);
-
-    QQmlData *data = QQmlData::get(*m_target, false);
-    Q_ASSERT(data && data->propertyCache);
-
-    QQmlPropertyData *propertyData = data->propertyCache->property(coreIndex);
-    Q_ASSERT(propertyData);
-
-    if (valueTypeIndex == -1)
-        return propertyData->propType;
-    else
-        return QMetaType::UnknownType;
-}
-
 QQmlBinding *QQmlBinding::newBinding(const QQmlPropertyData *property)
 {
     const int type = (property && property->isFullyResolved()) ? property->propType : QMetaType::UnknownType;
 
     if (type == qMetaTypeId<QQmlBinding *>()) {
         return new QQmlBindingBinding;
-    } else if (type == QMetaType::Int) {
+    }
+
+    switch (type) {
+    case QMetaType::Bool:
+        return new GenericBinding<QMetaType::Bool>;
+    case QMetaType::Int:
         return new GenericBinding<QMetaType::Int>;
-    } else if (type == QMetaType::Double) {
+    case QMetaType::Double:
         return new GenericBinding<QMetaType::Double>;
-    } else if (type == QMetaType::Float) {
+    case QMetaType::Float:
         return new GenericBinding<QMetaType::Float>;
-    } else if (type == QMetaType::QString) {
+    case QMetaType::QString:
         return new GenericBinding<QMetaType::QString>;
-    } else {
+    default:
         return new GenericBinding<QMetaType::UnknownType>;
     }
 }
diff --git a/src/qml/qml/qqmlbinding_p.h b/src/qml/qml/qqmlbinding_p.h
index ad1b24f340..a7c90603f6 100644
--- a/src/qml/qml/qqmlbinding_p.h
+++ b/src/qml/qml/qqmlbinding_p.h
@@ -110,9 +110,7 @@ protected:
     int getPropertyCoreIndex() const;
     int getPropertyType() const;
 
-    bool write(const QV4::Value &result, bool isUndefined, QQmlPropertyPrivate::WriteFlags flags);
-
-    bool slowWrite(const QV4::Value &result, bool isUndefined,
+    bool slowWrite(const QQmlPropertyData &core, const QV4::Value &result, bool isUndefined,
                    QQmlPropertyPrivate::WriteFlags flags);
 
 private:
diff --git a/src/qml/qml/qqmlvmemetaobject_p.h b/src/qml/qml/qqmlvmemetaobject_p.h
index 5aa141d026..4a81fc50d2 100644
--- a/src/qml/qml/qqmlvmemetaobject_p.h
+++ b/src/qml/qml/qqmlvmemetaobject_p.h
@@ -72,6 +72,7 @@
 
 #include <private/qv4object_p.h>
 #include <private/qv4value_p.h>
+#include <private/qqmlpropertyvalueinterceptor_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -105,6 +106,15 @@ public:
     // Used by auto-tests for inspection
     QQmlPropertyCache *propertyCache() const { return cache; }
 
+    bool intercepts(int coreIndex) const
+    {
+        for (auto it = interceptors; it; it = it->m_next) {
+            if (it->m_coreIndex == coreIndex)
+                return true;
+        }
+        return false;
+    }
+
 protected:
     int metaCall(QObject *o, QMetaObject::Call c, int id, void **a) Q_DECL_OVERRIDE;
     bool intercept(QMetaObject::Call c, int id, void **a);
diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index dc0c9df132..391a785b8f 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -117,17 +117,17 @@ static void QQuickItem_parentNotifier(QObject *o, QQmlNotifier **n)
     *n = &d->parentNotifier;
 }
 
-QML_PRIVATE_ACCESSOR(QQuickItem, QQuickItem *, parent, parentItem)
-QML_PRIVATE_ACCESSOR(QQuickItem, qreal, x, x)
-QML_PRIVATE_ACCESSOR(QQuickItem, qreal, y, y)
-QML_PRIVATE_ACCESSOR(QQuickItem, qreal, width, width)
-QML_PRIVATE_ACCESSOR(QQuickItem, qreal, height, height)
-
-static QQmlAccessors QQuickItem_parent = { QQuickItem_parentRead, QQuickItem_parentNotifier };
-static QQmlAccessors QQuickItem_x = { QQuickItem_xRead, 0 };
-static QQmlAccessors QQuickItem_y = { QQuickItem_yRead, 0 };
-static QQmlAccessors QQuickItem_width = { QQuickItem_widthRead, 0 };
-static QQmlAccessors QQuickItem_height = { QQuickItem_heightRead, 0 };
+QML_PRIVATE_ACCESSOR(QQuickItem, QQuickItem *, parent, parentItem, setParentItem)
+QML_PRIVATE_ACCESSOR(QQuickItem, qreal, x, x, setX)
+QML_PRIVATE_ACCESSOR(QQuickItem, qreal, y, y, setY)
+QML_PRIVATE_ACCESSOR(QQuickItem, qreal, width, width, setWidth)
+QML_PRIVATE_ACCESSOR(QQuickItem, qreal, height, height, setHeight)
+
+static QQmlAccessors QQuickItem_parent = { QQuickItem_parentRead, QQuickItem_parentWrite, QQuickItem_parentNotifier };
+static QQmlAccessors QQuickItem_x = { QQuickItem_xRead, QQuickItem_xWrite, 0 };
+static QQmlAccessors QQuickItem_y = { QQuickItem_yRead, QQuickItem_yWrite, 0 };
+static QQmlAccessors QQuickItem_width = { QQuickItem_widthRead, QQuickItem_widthWrite, 0 };
+static QQmlAccessors QQuickItem_height = { QQuickItem_heightRead, QQuickItem_heightWrite, 0 };
 
 QML_DECLARE_PROPERTIES(QQuickItem) {
     { QML_PROPERTY_NAME(parent), 0, &QQuickItem_parent },