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 <erik.verbruggen@digia.com>

diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp
index 4f43097..cb799a4 100644 (file)
--- 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<int>(size));
+    uint propIdx = propertyTable.lookup(id);
+    Q_ASSERT(propIdx < size);
 
     Transition t = { { id } , -1 };
     QHash<Transition, InternalClass *>::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<String *>();
+    propertyData.~SharedInternalClassData<PropertyAttributes>();
 
     if (m_sealed)
         m_sealed->destroy();

diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h
index 659789b..9586637 100644 (file)
--- 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 <typename T>
+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<String *> nameMap;
-
-    QVector<PropertyAttributes> propertyData;
+    SharedInternalClassData<String *> nameMap;
+    SharedInternalClassData<PropertyAttributes> propertyData;
 
     typedef InternalClassTransition Transition;
     QHash<Transition, InternalClass *> 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 df2f6ca..4fadec7 100644 (file)
--- 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) {