From 2602390bdb0c67b6efc7433a00c4981243df8cc5 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Tue, 19 Nov 2013 12:46:34 +0100 Subject: Greatly reduce memory consumed by QQmlEngine instances Due to a inefficiency in our InternalClass implementation, we were not sharing the string and attribute table between internal class instances. This was extremely inefficient with the Qt object, as it created around 1000 internal classes with large string and property tables. With the patch these tables are now shared. Reduces memory consumption of a QQmlEngine instance from around 6.5M to a couple of 100k. Change-Id: Ib763f31deca0808c000ac2c30aa0b05e806bda40 Reviewed-by: Erik Verbruggen --- src/qml/jsruntime/qv4internalclass.cpp | 39 +++++++------- src/qml/jsruntime/qv4internalclass_p.h | 98 +++++++++++++++++++++++++++++++--- src/qml/jsruntime/qv4object.cpp | 4 +- 3 files changed, 112 insertions(+), 29 deletions(-) diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp index 4f4309769a..cb799a473c 100644 --- a/src/qml/jsruntime/qv4internalclass.cpp +++ b/src/qml/jsruntime/qv4internalclass.cpp @@ -69,10 +69,10 @@ static inline int primeForNumBits(int numBits) } PropertyHashData::PropertyHashData(int numBits) - : size(0) + : refCount(1) + , size(0) , numBits(numBits) { - refCount.store(1); alloc = primeForNumBits(numBits); entries = (PropertyHash::Entry *)malloc(alloc*sizeof(PropertyHash::Entry)); memset(entries, 0, alloc*sizeof(PropertyHash::Entry)); @@ -97,8 +97,8 @@ void PropertyHash::addEntry(const PropertyHash::Entry &entry, int classSize) dd->entries[idx] = e; } dd->size = classSize; - assert(d->refCount.load() > 1); - d->refCount.deref(); + Q_ASSERT(d->refCount > 1); + --d->refCount; d = dd; } @@ -113,7 +113,7 @@ void PropertyHash::addEntry(const PropertyHash::Entry &entry, int classSize) uint PropertyHash::lookup(const Identifier *identifier) const { - assert(d->entries); + Q_ASSERT(d->entries); uint idx = identifier->hashValue % d->alloc; while (1) { @@ -149,9 +149,9 @@ InternalClass *InternalClass::changeMember(String *string, PropertyAttributes da if (index) *index = idx; - assert(idx != UINT_MAX); + Q_ASSERT(idx != UINT_MAX); - if (data == propertyData[idx]) + if (data == propertyData.at(idx)) return this; @@ -162,7 +162,7 @@ InternalClass *InternalClass::changeMember(String *string, PropertyAttributes da // create a new class and add it to the tree InternalClass *newClass = engine->newClass(*this); - newClass->propertyData[idx] = data; + newClass->propertyData.set(idx, data); transitions.insert(t, newClass); return newClass; @@ -189,7 +189,7 @@ InternalClass *InternalClass::changePrototype(Object *proto) newClass->prototype = proto; } else { newClass = engine->emptyClass->changePrototype(proto); - for (int i = 0; i < nameMap.size(); ++i) + for (uint i = 0; i < size; ++i) newClass = newClass->addMember(nameMap.at(i), propertyData.at(i)); } @@ -228,9 +228,9 @@ InternalClass *InternalClass::addMember(String *string, PropertyAttributes data, // store a string in the nameMap that's guaranteed to get // marked properly during GC. String *name = engine->newIdentifier(string->toQString()); - newClass->nameMap.append(name); + newClass->nameMap.add(size, name); - newClass->propertyData.append(data); + newClass->propertyData.add(size, data); ++newClass->size; transitions.insert(t, newClass); return newClass; @@ -238,8 +238,8 @@ InternalClass *InternalClass::addMember(String *string, PropertyAttributes data, void InternalClass::removeMember(Object *object, Identifier *id) { - int propIdx = propertyTable.lookup(id); - assert(propIdx < static_cast(size)); + uint propIdx = propertyTable.lookup(id); + Q_ASSERT(propIdx < size); Transition t = { { id } , -1 }; QHash::const_iterator tit = transitions.constFind(t); @@ -251,7 +251,7 @@ void InternalClass::removeMember(Object *object, Identifier *id) // create a new class and add it to the tree object->internalClass = engine->emptyClass->changePrototype(prototype); - for (int i = 0; i < nameMap.size(); ++i) { + for (uint i = 0; i < size; ++i) { if (i == propIdx) continue; object->internalClass = object->internalClass->addMember(nameMap.at(i), propertyData.at(i)); @@ -284,7 +284,7 @@ InternalClass *InternalClass::sealed() m_sealed = engine->emptyClass; m_sealed = m_sealed->changePrototype(prototype); - for (int i = 0; i < nameMap.size(); ++i) { + for (uint i = 0; i < size; ++i) { PropertyAttributes attrs = propertyData.at(i); attrs.setConfigurable(false); m_sealed = m_sealed->addMember(nameMap.at(i), attrs); @@ -301,7 +301,7 @@ InternalClass *InternalClass::frozen() m_frozen = engine->emptyClass; m_frozen = m_frozen->changePrototype(prototype); - for (int i = 0; i < nameMap.size(); ++i) { + for (uint i = 0; i < size; ++i) { PropertyAttributes attrs = propertyData.at(i); attrs.setWritable(false); attrs.setConfigurable(false); @@ -319,12 +319,9 @@ void InternalClass::destroy() return; engine = 0; - // Free the memory of the hashes/vectors by calling clear(), which - // re-assigns them to the shared null instance. Therefore Internalclass - // doesn't need a destructor to be called. propertyTable.~PropertyHash(); - nameMap.clear(); - propertyData.clear(); + nameMap.~SharedInternalClassData(); + propertyData.~SharedInternalClassData(); if (m_sealed) m_sealed->destroy(); diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h index 659789b344..9586637b32 100644 --- a/src/qml/jsruntime/qv4internalclass_p.h +++ b/src/qml/jsruntime/qv4internalclass_p.h @@ -82,7 +82,7 @@ struct PropertyHashData free(entries); } - QBasicAtomicInt refCount; + int refCount; int alloc; int size; int numBits; @@ -97,15 +97,102 @@ inline PropertyHash::PropertyHash() inline PropertyHash::PropertyHash(const PropertyHash &other) { d = other.d; - d->refCount.ref(); + ++d->refCount; } inline PropertyHash::~PropertyHash() { - if (!d->refCount.deref()) + if (!--d->refCount) delete d; } + +template +struct SharedInternalClassData { + struct Private { + Private(int alloc) + : refcount(1), + alloc(alloc), + size(0) + { data = new T [alloc]; } + ~Private() { delete [] data; } + + int refcount; + uint alloc; + uint size; + T *data; + }; + Private *d; + + inline SharedInternalClassData() { + d = new Private(8); + } + + inline SharedInternalClassData(const SharedInternalClassData &other) + : d(other.d) + { + ++d->refcount; + } + inline ~SharedInternalClassData() { + if (!--d->refcount) + delete d; + } + + void add(uint pos, T value) { + if (pos < d->size) { + Q_ASSERT(d->refcount > 1); + // need to detach + Private *dd = new Private(pos + 8); + memcpy(dd->data, d->data, pos*sizeof(T)); + dd->size = pos + 1; + dd->data[pos] = value; + if (!--d->refcount) + delete d; + d = dd; + return; + } + Q_ASSERT(pos == d->size); + if (pos == d->alloc) { + T *n = new T[d->alloc * 2]; + memcpy(n, d->data, d->alloc*sizeof(T)); + delete [] d->data; + d->data = n; + d->alloc *= 2; + } + d->data[pos] = value; + ++d->size; + } + + void set(uint pos, T value) { + Q_ASSERT(pos < d->size); + if (d->refcount > 1) { + // need to detach + Private *dd = new Private(d->alloc); + memcpy(dd->data, d->data, d->size*sizeof(T)); + dd->size = d->size; + if (!--d->refcount) + delete d; + d = dd; + } + d->data[pos] = value; + } + + T *constData() const { + return d->data; + } + T at(uint i) const { + Q_ASSERT(i < d->size); + return d->data[i]; + } + T operator[] (uint i) { + Q_ASSERT(i < d->size); + return d->data[i]; + } + +private: + SharedInternalClassData &operator=(const SharedInternalClassData &other); +}; + struct InternalClassTransition { union { @@ -124,9 +211,8 @@ struct InternalClass { ExecutionEngine *engine; Object *prototype; PropertyHash propertyTable; // id to valueIndex - QVector nameMap; - - QVector propertyData; + SharedInternalClassData nameMap; + SharedInternalClassData propertyData; typedef InternalClassTransition Transition; QHash transitions; // id to next class, positive means add, negative delete diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp index df2f6ca3f4..4fadec7860 100644 --- a/src/qml/jsruntime/qv4object.cpp +++ b/src/qml/jsruntime/qv4object.cpp @@ -918,7 +918,7 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, const StringRef name, if (isArrayObject() && name->equals(ctx->engine->id_length)) { assert(ArrayObject::LengthPropertyIndex == internalClass->find(ctx->engine->id_length)); Property *lp = memberData + ArrayObject::LengthPropertyIndex; - cattrs = internalClass->propertyData.data() + ArrayObject::LengthPropertyIndex; + cattrs = internalClass->propertyData.constData() + ArrayObject::LengthPropertyIndex; if (attrs.isEmpty() || p.isSubset(attrs, *lp, *cattrs)) return true; if (!cattrs->isWritable() || attrs.type() == PropertyAttributes::Accessor || attrs.isConfigurable() || attrs.isEnumerable()) @@ -947,7 +947,7 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, const StringRef name, { uint member = internalClass->find(name.getPointer()); current = (member < UINT_MAX) ? memberData + member : 0; - cattrs = internalClass->propertyData.data() + member; + cattrs = internalClass->propertyData.constData() + member; } if (!current) { -- cgit v1.2.3