From: Simon Hausmann Date: Fri, 14 Aug 2015 23:31:13 +0000 (+0200) Subject: Fix performance of ListModel::get() X-Git-Tag: v5.5.90+alpha1~38 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4876ea6a18ccdfd72014582aa5d50ab9f6b6ec9e;p=platform%2Fupstream%2Fqtdeclarative.git 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 Reviewed-by: Michael Brasser Reviewed-by: Spencer Schumann --- diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h index 2d10006..542d908 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 38b4e1e..b880c9c 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()) continue; QObjectWrapper *qobjectWrapper = static_cast((*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((*it).managed()); + if (QObjectWrapper *qobjectWrapper = (*it).as()) qobjectWrapper->destroyObject(lastSweep); - } (*it) = Primitive::undefinedValue(); } diff --git a/src/qml/qml/qqmlopenmetaobject.cpp b/src/qml/qml/qqmlopenmetaobject.cpp index c6d2d44..777bb0c 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 &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::iterator it = d->referers.begin(); + while (it != d->referers.end()) { + QQmlOpenMetaObject *omo = *it; + *static_cast(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::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 6a29d08..75ce9ad 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 &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 de46020..799f7a0 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 // 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 *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 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 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(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 &roles) +void ModelNodeMetaObject::updateValues(const QVector &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 &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 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(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 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(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(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(this), index); - result = QV4::QObjectWrapper::wrap(scope.engine, object); + QObject *object = m_listModel->getOrCreateModelObject(const_cast(this), index); + result = scope.engine->memoryManager->alloc(scope.engine, object, const_cast(this), index); } } diff --git a/src/qml/types/qqmllistmodel_p.h b/src/qml/types/qqmllistmodel_p.h index 75373be..b5a5ef3 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 4e3132b..bd0f028 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 &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 &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 *srcModelHash); - ModelObject *getOrCreateModelObject(QQmlListModel *model, int elementIndex); + QObject *getOrCreateModelObject(QQmlListModel *model, int elementIndex); private: QPODVector elements;