Fix performance of ListModel::get()

When called, the function would return a full-fledged QObject that maps the
list element addressed. It would contain a _copy_ of all values in the list
item and it would create a new meta-object for each list element.

This function exists for the JavaScript API, and therefore we now return a much
more lightweight object. For compatbility reasons it still has to be a QObject,
but the meta-object of it is created on-demand, i.e. only when accessing
properties from the C++ side or when connecting to the changed signal of a
property. Otherwise the JavaScript wrapper will return the live values from the
model without copying them.

Change-Id: Iabf3ca22192d2aee06ae9d4b4cfb2fcde2a021b1
Reviewed-by: Lars Knoll <lars.knoll@theqtcompany.com>
Reviewed-by: Michael Brasser <michael.brasser@live.com>
Reviewed-by: Spencer Schumann <spencer.schumann@echostar.com>

7 files changed, 213 insertions, 63 deletions

diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index 2d10006d97..542d908194 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -123,7 +123,6 @@ struct Q_QML_EXPORT QObjectWrapper : public Object
 protected:
     static bool isEqualTo(Managed *that, Managed *o);
 
-private:
     static ReturnedValue getProperty(ExecutionEngine *engine, QObject *object, QQmlPropertyData *property, bool captureRequired = true);
     static void setProperty(ExecutionEngine *engine, QObject *object, QQmlPropertyData *property, const Value &value);
 
diff --git a/src/qml/memory/qv4mm.cpp b/src/qml/memory/qv4mm.cpp
index 38b4e1eaf1..b880c9c8d5 100644
--- a/src/qml/memory/qv4mm.cpp
+++ b/src/qml/memory/qv4mm.cpp
@@ -373,7 +373,7 @@ void MemoryManager::mark()
     for (PersistentValueStorage::Iterator it = m_weakValues->begin(); it != m_weakValues->end(); ++it) {
         if (!(*it).isManaged())
             continue;
-        if ((*it).managed()->d()->vtable() != QObjectWrapper::staticVTable())
+        if (!(*it).as<QObjectWrapper>())
             continue;
         QObjectWrapper *qobjectWrapper = static_cast<QObjectWrapper*>((*it).managed());
         QObject *qobject = qobjectWrapper->object();
@@ -410,10 +410,8 @@ void MemoryManager::sweep(bool lastSweep)
             continue;
         // we need to call detroyObject on qobjectwrappers now, so that they can emit the destroyed
         // signal before we start sweeping the heap
-        if ((*it).managed()->d()->vtable() == QObjectWrapper::staticVTable()) {
-            QObjectWrapper *qobjectWrapper = static_cast<QObjectWrapper*>((*it).managed());
+        if (QObjectWrapper *qobjectWrapper = (*it).as<QObjectWrapper>())
             qobjectWrapper->destroyObject(lastSweep);
-        }
 
         (*it) = Primitive::undefinedValue();
     }
diff --git a/src/qml/qml/qqmlopenmetaobject.cpp b/src/qml/qml/qqmlopenmetaobject.cpp
index c6d2d44ee1..777bb0c5e1 100644
--- a/src/qml/qml/qqmlopenmetaobject.cpp
+++ b/src/qml/qml/qqmlopenmetaobject.cpp
@@ -106,6 +106,28 @@ QMetaObject *QQmlOpenMetaObjectType::metaObject() const
     return d->mem;
 }
 
+void QQmlOpenMetaObjectType::createProperties(const QVector<QByteArray> &names)
+{
+    for (int i = 0; i < names.count(); ++i) {
+        const QByteArray &name = names.at(i);
+        const int id = d->mob.propertyCount();
+        d->mob.addSignal("__" + QByteArray::number(id) + "()");
+        QMetaPropertyBuilder build = d->mob.addProperty(name, "QVariant", id);
+        propertyCreated(id, build);
+        d->names.insert(name, id);
+    }
+    free(d->mem);
+    d->mem = d->mob.toMetaObject();
+    QSet<QQmlOpenMetaObject*>::iterator it = d->referers.begin();
+    while (it != d->referers.end()) {
+        QQmlOpenMetaObject *omo = *it;
+        *static_cast<QMetaObject *>(omo) = *d->mem;
+        if (d->cache)
+            d->cache->update(omo);
+        ++it;
+    }
+}
+
 int QQmlOpenMetaObjectType::createProperty(const QByteArray &name)
 {
     int id = d->mob.propertyCount();
@@ -230,6 +252,14 @@ QQmlOpenMetaObjectType *QQmlOpenMetaObject::type() const
     return d->type;
 }
 
+void QQmlOpenMetaObject::emitPropertyNotification(const QByteArray &propertyName)
+{
+    QHash<QByteArray, int>::ConstIterator iter = d->type->d->names.constFind(propertyName);
+    if (iter == d->type->d->names.constEnd())
+        return;
+    activate(d->object, *iter + d->type->d->signalOffset, 0);
+}
+
 int QQmlOpenMetaObject::metaCall(QMetaObject::Call c, int id, void **a)
 {
     if (( c == QMetaObject::ReadProperty || c == QMetaObject::WriteProperty)
diff --git a/src/qml/qml/qqmlopenmetaobject_p.h b/src/qml/qml/qqmlopenmetaobject_p.h
index 6a29d08d4e..75ce9addd6 100644
--- a/src/qml/qml/qqmlopenmetaobject_p.h
+++ b/src/qml/qml/qqmlopenmetaobject_p.h
@@ -54,6 +54,7 @@ public:
     QQmlOpenMetaObjectType(const QMetaObject *base, QQmlEngine *engine);
     ~QQmlOpenMetaObjectType();
 
+    void createProperties(const QVector<QByteArray> &names);
     int createProperty(const QByteArray &name);
 
     int propertyOffset() const;
@@ -101,6 +102,8 @@ public:
 
     QQmlOpenMetaObjectType *type() const;
 
+    void emitPropertyNotification(const QByteArray &propertyName);
+
 protected:
     virtual int metaCall(QMetaObject::Call _c, int _id, void **_a);
     virtual int createProperty(const char *, const char *);
diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp
index de46020ad8..799f7a0b8a 100644
--- a/src/qml/types/qqmllistmodel.cpp
+++ b/src/qml/types/qqmllistmodel.cpp
@@ -242,11 +242,12 @@ const ListLayout::Role *ListLayout::getExistingRole(QV4::String *key)
     return r;
 }
 
-ModelObject *ListModel::getOrCreateModelObject(QQmlListModel *model, int elementIndex)
+QObject *ListModel::getOrCreateModelObject(QQmlListModel *model, int elementIndex)
 {
     ListElement *e = elements[elementIndex];
     if (e->m_objectCache == 0) {
-        e->m_objectCache = new ModelObject(model, elementIndex);
+        e->m_objectCache = new QObject;
+        (void)new ModelNodeMetaObject(e->m_objectCache, model, elementIndex);
     }
     return e->m_objectCache;
 }
@@ -317,8 +318,8 @@ void ListModel::sync(ListModel *src, ListModel *target, QHash<int, ListModel *>
     // Update values stored in target meta objects
     for (int i=0 ; i < target->elements.count() ; ++i) {
         ListElement *e = target->elements[i];
-        if (e->m_objectCache)
-            e->m_objectCache->updateValues();
+        if (ModelNodeMetaObject *mo = e->objectCache())
+            mo->updateValues();
     }
 }
 
@@ -384,9 +385,8 @@ void ListModel::updateCacheIndices()
 {
     for (int i=0 ; i < elements.count() ; ++i) {
         ListElement *e = elements.at(i);
-        if (e->m_objectCache) {
-            e->m_objectCache->m_elementIndex = i;
-        }
+        if (ModelNodeMetaObject *mo = e->objectCache())
+            mo->m_elementIndex = i;
     }
 }
 
@@ -470,9 +470,8 @@ void ListModel::set(int elementIndex, QV4::Object *object, QVector<int> *roles)
             roles->append(roleIndex);
     }
 
-    if (e->m_objectCache) {
-        e->m_objectCache->updateValues(*roles);
-    }
+    if (ModelNodeMetaObject *mo = e->objectCache())
+        mo->updateValues(*roles);
 }
 
 void ListModel::set(int elementIndex, QV4::Object *object)
@@ -591,10 +590,12 @@ int ListModel::setOrCreateProperty(int elementIndex, const QString &key, const Q
         if (r) {
             roleIndex = e->setVariantProperty(*r, data);
 
-            if (roleIndex != -1 && e->m_objectCache) {
+            ModelNodeMetaObject *cache = e->objectCache();
+
+            if (roleIndex != -1 && cache) {
                 QVector<int> roles;
                 roles << roleIndex;
-                e->m_objectCache->updateValues(roles);
+                cache->updateValues(roles);
             }
         }
     }
@@ -633,6 +634,13 @@ inline char *ListElement::getPropertyMemory(const ListLayout::Role &role)
     return mem;
 }
 
+ModelNodeMetaObject *ListElement::objectCache()
+{
+    if (!m_objectCache)
+        return 0;
+    return ModelNodeMetaObject::get(m_objectCache);
+}
+
 QString *ListElement::getStringProperty(const ListLayout::Role &role)
 {
     char *mem = getPropertyMemory(role);
@@ -1209,15 +1217,48 @@ int ListElement::setJsProperty(const ListLayout::Role &role, const QV4::Value &d
     return roleIndex;
 }
 
-ModelObject::ModelObject(QQmlListModel *model, int elementIndex)
-: m_model(model), m_elementIndex(elementIndex), m_meta(new ModelNodeMetaObject(this))
+ModelNodeMetaObject::ModelNodeMetaObject(QObject *object, QQmlListModel *model, int elementIndex)
+: QQmlOpenMetaObject(object), m_enabled(false), m_model(model), m_elementIndex(elementIndex), m_initialized(false)
+{}
+
+void ModelNodeMetaObject::initialize()
 {
+    const int roleCount = m_model->m_listModel->roleCount();
+    QVector<QByteArray> properties;
+    properties.reserve(roleCount);
+    for (int i = 0 ; i < roleCount ; ++i) {
+        const ListLayout::Role &role = m_model->m_listModel->getExistingRole(i);
+        QByteArray name = role.name.toUtf8();
+        properties << name;
+    }
+    type()->createProperties(properties);
     updateValues();
-    setNodeUpdatesEnabled(true);
+    m_enabled = true;
+}
+
+ModelNodeMetaObject::~ModelNodeMetaObject()
+{
+}
+
+QAbstractDynamicMetaObject *ModelNodeMetaObject::toDynamicMetaObject(QObject *object)
+{
+    if (!m_initialized) {
+        m_initialized = true;
+        initialize();
+    }
+    return QQmlOpenMetaObject::toDynamicMetaObject(object);
+}
+
+ModelNodeMetaObject *ModelNodeMetaObject::get(QObject *obj)
+{
+    QObjectPrivate *op = QObjectPrivate::get(obj);
+    return static_cast<ModelNodeMetaObject*>(op->metaObject);
 }
 
-void ModelObject::updateValues()
+void ModelNodeMetaObject::updateValues()
 {
+    if (!m_initialized)
+        return;
     int roleCount = m_model->m_listModel->roleCount();
     for (int i=0 ; i < roleCount ; ++i) {
         const ListLayout::Role &role = m_model->m_listModel->getExistingRole(i);
@@ -1227,8 +1268,10 @@ void ModelObject::updateValues()
     }
 }
 
-void ModelObject::updateValues(const QVector<int> &roles)
+void ModelNodeMetaObject::updateValues(const QVector<int> &roles)
 {
+    if (!m_initialized)
+        return;
     int roleCount = roles.count();
     for (int i=0 ; i < roleCount ; ++i) {
         int roleIndex = roles.at(i);
@@ -1239,15 +1282,6 @@ void ModelObject::updateValues(const QVector<int> &roles)
     }
 }
 
-ModelNodeMetaObject::ModelNodeMetaObject(ModelObject *object)
-: QQmlOpenMetaObject(object), m_enabled(false), m_obj(object)
-{
-}
-
-ModelNodeMetaObject::~ModelNodeMetaObject()
-{
-}
-
 void ModelNodeMetaObject::propertyWritten(int index)
 {
     if (!m_enabled)
@@ -1256,17 +1290,75 @@ void ModelNodeMetaObject::propertyWritten(int index)
     QString propName = QString::fromUtf8(name(index));
     QVariant value = operator[](index);
 
-    QV4::Scope scope(m_obj->m_model->engine());
+    QV4::Scope scope(m_model->engine());
     QV4::ScopedValue v(scope, scope.engine->fromVariant(value));
 
-    int roleIndex = m_obj->m_model->m_listModel->setExistingProperty(m_obj->m_elementIndex, propName, v, scope.engine);
+    int roleIndex = m_model->m_listModel->setExistingProperty(m_elementIndex, propName, v, scope.engine);
+    if (roleIndex != -1) {
+        QVector<int> roles;
+        roles << roleIndex;
+        m_model->emitItemsChanged(m_elementIndex, 1, roles);
+    }
+}
+
+namespace QV4 {
+
+void ModelObject::put(Managed *m, String *name, const Value &value)
+{
+    ModelObject *that = static_cast<ModelObject*>(m);
+
+    ExecutionEngine *eng = that->engine();
+    const int elementIndex = that->d()->m_elementIndex;
+    const QString propName = name->toQString();
+    int roleIndex = that->d()->m_model->m_listModel->setExistingProperty(elementIndex, propName, value, eng);
     if (roleIndex != -1) {
         QVector<int> roles;
         roles << roleIndex;
-        m_obj->m_model->emitItemsChanged(m_obj->m_elementIndex, 1, roles);
+        that->d()->m_model->emitItemsChanged(elementIndex, 1, roles);
+    }
+
+    ModelNodeMetaObject *mo = ModelNodeMetaObject::get(that->object());
+    if (mo->initialized())
+        mo->emitPropertyNotification(name->toQString().toUtf8());
+}
+
+ReturnedValue ModelObject::get(const Managed *m, String *name, bool *hasProperty)
+{
+    const ModelObject *that = static_cast<const ModelObject*>(m);
+    const ListLayout::Role *role = that->d()->m_model->m_listModel->getExistingRole(name);
+    if (!role)
+        return QObjectWrapper::get(m, name, hasProperty);
+    if (hasProperty)
+        *hasProperty = true;
+    const int elementIndex = that->d()->m_elementIndex;
+    QVariant value = that->d()->m_model->data(elementIndex, role->index);
+    return that->engine()->fromVariant(value);
+}
+
+void ModelObject::advanceIterator(Managed *m, ObjectIterator *it, Value *name, uint *index, Property *p, PropertyAttributes *attributes)
+{
+    ModelObject *that = static_cast<ModelObject*>(m);
+    ExecutionEngine *v4 = that->engine();
+    name->setM(0);
+    *index = UINT_MAX;
+    if (it->arrayIndex < uint(that->d()->m_model->m_listModel->roleCount())) {
+        Scope scope(that->engine());
+        const ListLayout::Role &role = that->d()->m_model->m_listModel->getExistingRole(it->arrayIndex);
+        ++it->arrayIndex;
+        ScopedString roleName(scope, v4->newString(role.name));
+        name->setM(roleName->d());
+        *attributes = QV4::Attr_Data;
+        QVariant value = that->d()->m_model->data(that->d()->m_elementIndex, role.index);
+        p->value = v4->fromVariant(value);
+        return;
     }
+    QV4::QObjectWrapper::advanceIterator(m, it, name, index, p, attributes);
 }
 
+DEFINE_OBJECT_VTABLE(ModelObject);
+
+} // namespace QV4
+
 DynamicRoleModelNode::DynamicRoleModelNode(QQmlListModel *owner, int uid) : m_owner(owner), m_uid(uid), m_meta(new DynamicRoleModelNodeMetaObject(this))
 {
     setNodeUpdatesEnabled(true);
@@ -2159,8 +2251,8 @@ QQmlV4Handle QQmlListModel::get(int index) const
             DynamicRoleModelNode *object = m_modelObjects[index];
             result = QV4::QObjectWrapper::wrap(scope.engine, object);
         } else {
-            ModelObject *object = m_listModel->getOrCreateModelObject(const_cast<QQmlListModel *>(this), index);
-            result = QV4::QObjectWrapper::wrap(scope.engine, object);
+            QObject *object = m_listModel->getOrCreateModelObject(const_cast<QQmlListModel *>(this), index);
+            result = scope.engine->memoryManager->alloc<QV4::ModelObject>(scope.engine, object, const_cast<QQmlListModel *>(this), index);
         }
     }
 
diff --git a/src/qml/types/qqmllistmodel_p.h b/src/qml/types/qqmllistmodel_p.h
index 75373be47c..b5a5ef3265 100644
--- a/src/qml/types/qqmllistmodel_p.h
+++ b/src/qml/types/qqmllistmodel_p.h
@@ -54,6 +54,10 @@ class QQmlListModelWorkerAgent;
 class ListModel;
 class ListLayout;
 
+namespace QV4 {
+struct ModelObject;
+}
+
 class Q_QML_PRIVATE_EXPORT QQmlListModel : public QAbstractListModel
 {
     Q_OBJECT
@@ -94,6 +98,7 @@ private:
     friend class QQmlListModelParser;
     friend class QQmlListModelWorkerAgent;
     friend class ModelObject;
+    friend struct QV4::ModelObject;
     friend class ModelNodeMetaObject;
     friend class ListModel;
     friend class ListElement;
diff --git a/src/qml/types/qqmllistmodel_p_p.h b/src/qml/types/qqmllistmodel_p_p.h
index 4e3132b860..bd0f028e7a 100644
--- a/src/qml/types/qqmllistmodel_p_p.h
+++ b/src/qml/types/qqmllistmodel_p_p.h
@@ -111,56 +111,72 @@ private:
     friend class DynamicRoleModelNodeMetaObject;
 };
 
-class ModelObject;
-
 class ModelNodeMetaObject : public QQmlOpenMetaObject
 {
 public:
-    ModelNodeMetaObject(ModelObject *object);
+    ModelNodeMetaObject(QObject *object, QQmlListModel *model, int elementIndex);
     ~ModelNodeMetaObject();
 
+    virtual QAbstractDynamicMetaObject *toDynamicMetaObject(QObject *object);
+
+    static ModelNodeMetaObject *get(QObject *obj);
+
     bool m_enabled;
+    QQmlListModel *m_model;
+    int m_elementIndex;
+
+    void updateValues();
+    void updateValues(const QVector<int> &roles);
+
+    bool initialized() const { return m_initialized; }
 
 protected:
     void propertyWritten(int index);
 
 private:
-
-    ModelObject *m_obj;
-};
-
-class ModelObject : public QObject
-{
-    Q_OBJECT
-public:
-    ModelObject(QQmlListModel *model, int elementIndex);
-
+    using QQmlOpenMetaObject::setValue;
     void setValue(const QByteArray &name, const QVariant &val, bool force)
     {
         if (force) {
-            QVariant existingValue = m_meta->value(name);
+            QVariant existingValue = value(name);
             if (existingValue.isValid()) {
-                (*m_meta)[name] = QVariant();
+                (*this)[name] = QVariant();
             }
         }
-        m_meta->setValue(name, val);
+        setValue(name, val);
     }
 
-    void setNodeUpdatesEnabled(bool enable)
-    {
-        m_meta->m_enabled = enable;
-    }
+    void initialize();
+    bool m_initialized;
+};
 
-    void updateValues();
-    void updateValues(const QVector<int> &roles);
+namespace QV4 {
+
+namespace Heap {
 
+struct ModelObject : public QObjectWrapper {
+    ModelObject(QV4::ExecutionEngine *engine, QObject *object, QQmlListModel *model, int elementIndex)
+        : QObjectWrapper(engine, object)
+        , m_model(model)
+        , m_elementIndex(elementIndex)
+    {}
     QQmlListModel *m_model;
     int m_elementIndex;
+};
 
-private:
-    ModelNodeMetaObject *m_meta;
+}
+
+struct ModelObject : public QObjectWrapper
+{
+    static void put(Managed *m, String *name, const Value& value);
+    static ReturnedValue get(const Managed *m, String *name, bool *hasProperty);
+    static void advanceIterator(Managed *m, ObjectIterator *it, Value *name, uint *index, Property *p, PropertyAttributes *attributes);
+
+    V4_OBJECT2(ModelObject, QObjectWrapper)
 };
 
+} // namespace QV4
+
 class ListLayout
 {
 public:
@@ -236,7 +252,7 @@ public:
 
     enum
     {
-        BLOCK_SIZE = 64 - sizeof(int) - sizeof(ListElement *) - sizeof(ModelObject *)
+        BLOCK_SIZE = 64 - sizeof(int) - sizeof(ListElement *) - sizeof(ModelNodeMetaObject *)
     };
 
 private:
@@ -278,11 +294,13 @@ private:
 
     int getUid() const { return uid; }
 
+    ModelNodeMetaObject *objectCache();
+
     char data[BLOCK_SIZE];
     ListElement *next;
 
     int uid;
-    ModelObject *m_objectCache;
+    QObject *m_objectCache;
 
     friend class ListModel;
 };
@@ -315,6 +333,11 @@ public:
         return m_layout->getExistingRole(index);
     }
 
+    const ListLayout::Role *getExistingRole(QV4::String *key)
+    {
+        return m_layout->getExistingRole(key);
+    }
+
     const ListLayout::Role &getOrCreateListRole(const QString &name)
     {
         return m_layout->getRoleOrCreate(name, ListLayout::Role::List);
@@ -343,7 +366,7 @@ public:
 
     static void sync(ListModel *src, ListModel *target, QHash<int, ListModel *> *srcModelHash);
 
-    ModelObject *getOrCreateModelObject(QQmlListModel *model, int elementIndex);
+    QObject *getOrCreateModelObject(QQmlListModel *model, int elementIndex);
 
 private:
     QPODVector<ListElement *, 4> elements;