Trigger the garbage collector when allocating InternalClass objects

As we check the icAllocator's slots on shouldRunGC() we should also
check shouldRunGC() when adding slots. Otherwise we might never run the
GC when only allocating InternalClasses. In addition, account for the
"unmanaged" size of the PropertyAttributes that are part of the
InternalClass objects. Those can be large.

In cases where an excessive number of large InternalClass objects is
created the garbage collector is now invoked frequently, which costs a
significant number of CPU cycles, but prevents the memory usage from
growing indefinitely.

Task-number: QTBUG-58559
Change-Id: Icf102cb6100f6dba212b8bffe1c178897880eda0
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

2 files changed, 45 insertions, 54 deletions

diff --git a/src/qml/memory/qv4mm.cpp b/src/qml/memory/qv4mm.cpp
index 203f1f424f..cac68bdcaf 100644
--- a/src/qml/memory/qv4mm.cpp
+++ b/src/qml/memory/qv4mm.cpp
@@ -93,8 +93,6 @@
 #include <pthread_np.h>
 #endif
 
-#define MIN_UNMANAGED_HEAPSIZE_GC_LIMIT std::size_t(128 * 1024)
-
 Q_LOGGING_CATEGORY(lcGcStats, "qt.qml.gc.statistics")
 Q_DECLARE_LOGGING_CATEGORY(lcGcStats)
 Q_LOGGING_CATEGORY(lcGcAllocatorStats, "qt.qml.gc.allocatorStats")
@@ -759,7 +757,7 @@ MemoryManager::MemoryManager(ExecutionEngine *engine)
     , hugeItemAllocator(chunkAllocator, engine)
     , m_persistentValues(new PersistentValueStorage(engine))
     , m_weakValues(new PersistentValueStorage(engine))
-    , unmanagedHeapSizeGCLimit(MIN_UNMANAGED_HEAPSIZE_GC_LIMIT)
+    , unmanagedHeapSizeGCLimit(MinUnmanagedHeapSizeGCLimit)
     , aggressiveGC(!qEnvironmentVariableIsEmpty("QV4_MM_AGGRESSIVE_GC"))
     , gcStats(lcGcStats().isDebugEnabled())
     , gcCollectorStats(lcGcAllocatorStats().isDebugEnabled())
@@ -779,35 +777,9 @@ Heap::Base *MemoryManager::allocString(std::size_t unmanagedSize)
     lastAllocRequestedSlots = stringSize >> Chunk::SlotSizeShift;
     ++allocationCount;
 #endif
-
-    bool didGCRun = false;
-    if (aggressiveGC) {
-        runGC();
-        didGCRun = true;
-    }
-
     unmanagedHeapSize += unmanagedSize;
-    if (unmanagedHeapSize > unmanagedHeapSizeGCLimit) {
-        if (!didGCRun)
-            runGC();
-
-        if (3*unmanagedHeapSizeGCLimit <= 4*unmanagedHeapSize)
-            // more than 75% full, raise limit
-            unmanagedHeapSizeGCLimit = std::max(unmanagedHeapSizeGCLimit, unmanagedHeapSize) * 2;
-        else if (unmanagedHeapSize * 4 <= unmanagedHeapSizeGCLimit)
-            // less than 25% full, lower limit
-            unmanagedHeapSizeGCLimit = qMax(MIN_UNMANAGED_HEAPSIZE_GC_LIMIT, unmanagedHeapSizeGCLimit/2);
-        didGCRun = true;
-    }
-
-    HeapItem *m = blockAllocator.allocate(stringSize);
-    if (!m) {
-        if (!didGCRun && shouldRunGC())
-            runGC();
-        m = blockAllocator.allocate(stringSize, true);
-    }
 
-//    qDebug() << "allocated string" << m;
+    HeapItem *m = allocate(&blockAllocator, stringSize);
     memset(m, 0, stringSize);
     return *m;
 }
@@ -819,32 +791,11 @@ Heap::Base *MemoryManager::allocData(std::size_t size)
     ++allocationCount;
 #endif
 
-    bool didRunGC = false;
-    if (aggressiveGC) {
-        runGC();
-        didRunGC = true;
-    }
-
     Q_ASSERT(size >= Chunk::SlotSize);
     Q_ASSERT(size % Chunk::SlotSize == 0);
 
-//    qDebug() << "unmanagedHeapSize:" << unmanagedHeapSize << "limit:" << unmanagedHeapSizeGCLimit << "unmanagedSize:" << unmanagedSize;
-
-    if (size > Chunk::DataSize) {
-        HeapItem *h = hugeItemAllocator.allocate(size);
-//        qDebug() << "allocating huge item" << h;
-        return *h;
-    }
-
-    HeapItem *m = blockAllocator.allocate(size);
-    if (!m) {
-        if (!didRunGC && shouldRunGC())
-            runGC();
-        m = blockAllocator.allocate(size, true);
-    }
-
+    HeapItem *m = allocate(&blockAllocator, size);
     memset(m, 0, size);
-//    qDebug() << "allocating data" << m;
     return *m;
 }
 
diff --git a/src/qml/memory/qv4mm_p.h b/src/qml/memory/qv4mm_p.h
index bbbbb1aef6..6dfdd81cb2 100644
--- a/src/qml/memory/qv4mm_p.h
+++ b/src/qml/memory/qv4mm_p.h
@@ -264,13 +264,13 @@ public:
     size_t getLargeItemsMem() const;
 
     // called when a JS object grows itself. Specifically: Heap::String::append
+    // and InternalClassDataPrivate<PropertyAttributes>.
     void changeUnmanagedHeapSizeUsage(qptrdiff delta) { unmanagedHeapSize += delta; }
 
     template<typename ManagedType>
     typename ManagedType::Data *allocIC()
     {
-        size_t size = align(sizeof(typename ManagedType::Data));
-        Heap::Base *b = *icAllocator.allocate(size, true);
+        Heap::Base *b = *allocate(&icAllocator, align(sizeof(typename ManagedType::Data)));
         return static_cast<typename ManagedType::Data *>(b);
     }
 
@@ -284,12 +284,52 @@ protected:
     Heap::Object *allocObjectWithMemberData(const QV4::VTable *vtable, uint nMembers);
 
 private:
+    enum {
+        MinUnmanagedHeapSizeGCLimit = 128 * 1024
+    };
+
     void collectFromJSStack(MarkStack *markStack) const;
     void mark();
     void sweep(bool lastSweep = false, ClassDestroyStatsCallback classCountPtr = nullptr);
     bool shouldRunGC() const;
     void collectRoots(MarkStack *markStack);
 
+    HeapItem *allocate(BlockAllocator *allocator, std::size_t size)
+    {
+        bool didGCRun = false;
+        if (aggressiveGC) {
+            runGC();
+            didGCRun = true;
+        }
+
+        if (unmanagedHeapSize > unmanagedHeapSizeGCLimit) {
+            if (!didGCRun)
+                runGC();
+
+            if (3*unmanagedHeapSizeGCLimit <= 4 * unmanagedHeapSize) {
+                // more than 75% full, raise limit
+                unmanagedHeapSizeGCLimit = std::max(unmanagedHeapSizeGCLimit,
+                                                    unmanagedHeapSize) * 2;
+            } else if (unmanagedHeapSize * 4 <= unmanagedHeapSizeGCLimit) {
+                // less than 25% full, lower limit
+                unmanagedHeapSizeGCLimit = qMax(std::size_t(MinUnmanagedHeapSizeGCLimit),
+                                                unmanagedHeapSizeGCLimit/2);
+            }
+            didGCRun = true;
+        }
+
+        if (size > Chunk::DataSize)
+            return hugeItemAllocator.allocate(size);
+
+        if (HeapItem *m = allocator->allocate(size))
+            return m;
+
+        if (!didGCRun && shouldRunGC())
+            runGC();
+
+        return allocator->allocate(size, true);
+    }
+
 public:
     QV4::ExecutionEngine *engine;
     ChunkAllocator *chunkAllocator;