Skip to content

Commit

Permalink
Fix performance of ListModel::get()
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Simon Hausmann committed Aug 19, 2015
1 parent 7fccfc4 commit 4876ea6
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 63 deletions.
1 change: 0 additions & 1 deletion src/qml/jsruntime/qv4qobjectwrapper_p.h
View file
Expand Up @@ -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);

Expand Down
6 changes: 2 additions & 4 deletions src/qml/memory/qv4mm.cpp
View file
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
Expand Down
30 changes: 30 additions & 0 deletions src/qml/qml/qqmlopenmetaobject.cpp
View file
Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions src/qml/qml/qqmlopenmetaobject_p.h
View file
Expand Up @@ -54,6 +54,7 @@ class Q_QML_PRIVATE_EXPORT QQmlOpenMetaObjectType : public QQmlRefCount, public
QQmlOpenMetaObjectType(const QMetaObject *base, QQmlEngine *engine);
~QQmlOpenMetaObjectType();

void createProperties(const QVector<QByteArray> &names);
int createProperty(const QByteArray &name);

int propertyOffset() const;
Expand Down Expand Up @@ -101,6 +102,8 @@ class Q_QML_PRIVATE_EXPORT QQmlOpenMetaObject : public QAbstractDynamicMetaObjec

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 *);
Expand Down
154 changes: 123 additions & 31 deletions src/qml/types/qqmllistmodel.cpp
View file
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/qml/types/qqmllistmodel_p.h
View file
Expand Up @@ -54,6 +54,10 @@ class QQmlListModelWorkerAgent;
class ListModel;
class ListLayout;

namespace QV4 {
struct ModelObject;
}

class Q_QML_PRIVATE_EXPORT QQmlListModel : public QAbstractListModel
{
Q_OBJECT
Expand Down Expand Up @@ -94,6 +98,7 @@ class Q_QML_PRIVATE_EXPORT QQmlListModel : public QAbstractListModel
friend class QQmlListModelParser;
friend class QQmlListModelWorkerAgent;
friend class ModelObject;
friend struct QV4::ModelObject;
friend class ModelNodeMetaObject;
friend class ListModel;
friend class ListElement;
Expand Down

0 comments on commit 4876ea6

Please sign in to comment.