V8 memory leak fix and optimizations

libraries/script-engine/src/ScriptEngine.h

@@ -60,6 +60,8 @@ public:
 #ifdef OVERTE_V8_MEMORY_DEBUG
     size_t scriptValueCount;
     size_t scriptValueProxyCount;
+    size_t qObjectCount;
+    //size_t qVariantProxyCount;
 #endif
 };
 

libraries/script-engine/src/ScriptManagerScriptingInterface.cpp

@@ -49,7 +49,6 @@
 void ScriptManagerScriptingInterface::scriptManagerException(std::shared_ptr<ScriptException> exception) {
     // V8TODO: What should we actually handle here?
 
-
     // emit unhandledException(exception.thrownValue);
 }
 
@@ -64,6 +63,7 @@ QVariantMap ScriptManagerScriptingInterface::getMemoryUsageStatistics() {
 #ifdef OVERTE_V8_MEMORY_DEBUG
     map.insert("scriptValueCount", QVariant((qulonglong)(statistics.scriptValueCount)));
     map.insert("scriptValueProxyCount", QVariant((qulonglong)(statistics.scriptValueProxyCount)));
+    map.insert("qObjectCount", QVariant((qulonglong)(statistics.qObjectCount)));
 #endif
     return map;
 }

libraries/script-engine/src/v8/ScriptEngineV8.cpp

@@ -1090,6 +1090,64 @@ QString ScriptEngineV8::formatErrorMessageFromTryCatch(v8::TryCatch &tryCatch) {
     return result;
 }
 
+v8::Local<v8::ObjectTemplate> ScriptEngineV8::getObjectProxyTemplate() {
+    v8::EscapableHandleScope handleScope(_v8Isolate);
+    if (_objectProxyTemplate.IsEmpty()) {
+        auto objectTemplate = v8::ObjectTemplate::New(_v8Isolate);
+        objectTemplate->SetInternalFieldCount(3);
+        objectTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(ScriptObjectV8Proxy::v8Get, ScriptObjectV8Proxy::v8Set, nullptr, nullptr, ScriptObjectV8Proxy::v8GetPropertyNames));
+        _objectProxyTemplate.Reset(_v8Isolate, objectTemplate);
+    }
+
+    return handleScope.Escape(_objectProxyTemplate.Get(_v8Isolate));
+}
+
+v8::Local<v8::ObjectTemplate> ScriptEngineV8::getMethodDataTemplate() {
+    v8::EscapableHandleScope handleScope(_v8Isolate);
+    if (_methodDataTemplate.IsEmpty()) {
+        auto methodDataTemplate = v8::ObjectTemplate::New(_v8Isolate);
+        methodDataTemplate->SetInternalFieldCount(2);
+        _methodDataTemplate.Reset(_v8Isolate, methodDataTemplate);
+    }
+
+    return handleScope.Escape(_methodDataTemplate.Get(_v8Isolate));
+}
+
+v8::Local<v8::ObjectTemplate> ScriptEngineV8::getFunctionDataTemplate() {
+    v8::EscapableHandleScope handleScope(_v8Isolate);
+    if (_functionDataTemplate.IsEmpty()) {
+        auto functionDataTemplate = v8::ObjectTemplate::New(_v8Isolate);
+        functionDataTemplate->SetInternalFieldCount(2);
+        _functionDataTemplate.Reset(_v8Isolate, functionDataTemplate);
+    }
+
+    return handleScope.Escape(_functionDataTemplate.Get(_v8Isolate));
+}
+
+v8::Local<v8::ObjectTemplate> ScriptEngineV8::getVariantDataTemplate() {
+    v8::EscapableHandleScope handleScope(_v8Isolate);
+    if (_variantDataTemplate.IsEmpty()) {
+        auto variantDataTemplate = v8::ObjectTemplate::New(_v8Isolate);
+        variantDataTemplate->SetInternalFieldCount(2);
+        _variantDataTemplate.Reset(_v8Isolate, variantDataTemplate);
+    }
+
+    return handleScope.Escape(_variantDataTemplate.Get(_v8Isolate));
+}
+
+v8::Local<v8::ObjectTemplate> ScriptEngineV8::getVariantProxyTemplate() {
+    v8::EscapableHandleScope handleScope(_v8Isolate);
+    if (_variantProxyTemplate.IsEmpty()) {
+        auto variantProxyTemplate = v8::ObjectTemplate::New(_v8Isolate);
+        variantProxyTemplate->SetInternalFieldCount(2);
+        variantProxyTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(ScriptVariantV8Proxy::v8Get, ScriptVariantV8Proxy::v8Set, nullptr, nullptr, ScriptVariantV8Proxy::v8GetPropertyNames));
+        _variantProxyTemplate.Reset(_v8Isolate, variantProxyTemplate);
+    }
+
+    return handleScope.Escape(_variantProxyTemplate.Get(_v8Isolate));
+}
+
+
 ScriptContextV8Pointer ScriptEngineV8::pushContext(v8::Local<v8::Context> context) {
     v8::HandleScope handleScope(_v8Isolate);
     Q_ASSERT(!_contexts.isEmpty());
@@ -1463,8 +1521,9 @@ ScriptValue ScriptEngineV8::newFunction(ScriptEngine::FunctionSignature fun, int
     //auto functionTemplate = v8::FunctionTemplate::New(_v8Isolate, v8FunctionCallback, v8::Local<v8::Value>(), v8::Local<v8::Signature>(), length);
     //auto functionData = v8::Object::New(_v8Isolate);
     //functionData->setIn
-    auto functionDataTemplate = v8::ObjectTemplate::New(_v8Isolate);
+    auto functionDataTemplate = getFunctionDataTemplate();
-    functionDataTemplate->SetInternalFieldCount(2);
+    //auto functionDataTemplate = v8::ObjectTemplate::New(_v8Isolate);
+    //functionDataTemplate->SetInternalFieldCount(2);
     auto functionData = functionDataTemplate->NewInstance(getContext()).ToLocalChecked();
     functionData->SetAlignedPointerInInternalField(0, reinterpret_cast<void*>(fun));
     functionData->SetAlignedPointerInInternalField(1, reinterpret_cast<void*>(this));
@@ -1729,6 +1788,7 @@ ScriptEngineMemoryStatistics ScriptEngineV8::getMemoryUsageStatistics() {
 #ifdef OVERTE_V8_MEMORY_DEBUG
     statistics.scriptValueCount = scriptValueCount;
     statistics.scriptValueProxyCount = scriptValueProxyCount;
+    statistics.qObjectCount = _qobjectWrapperMapV8.size();
 #endif
     return statistics;
 }

libraries/script-engine/src/v8/ScriptEngineV8.h

@@ -203,6 +203,13 @@ public: // not for public use, but I don't like how Qt strings this along with p
     // V8TODO add a V8 callback that removes pointer from the map so that it gets deleted
     QMap<QObject*, QSharedPointer<ScriptObjectV8Proxy>> _qobjectWrapperMapV8;
 
+    // Used by ScriptObjectV8Proxy to create JS objects referencing C++ ones
+    v8::Local<v8::ObjectTemplate> getObjectProxyTemplate();
+    v8::Local<v8::ObjectTemplate> getMethodDataTemplate();
+    v8::Local<v8::ObjectTemplate> getFunctionDataTemplate();
+    v8::Local<v8::ObjectTemplate> getVariantDataTemplate();
+    v8::Local<v8::ObjectTemplate> getVariantProxyTemplate();
+
     ScriptContextV8Pointer pushContext(v8::Local<v8::Context> context);
     void popContext();
     // V8TODO: call this after initializing global object
@@ -259,9 +266,18 @@ protected:
     //mutable ScriptContextV8Pointer _currContext;
     // Current context stack. Main context is first on the list and current one is last.
     QList<ScriptContextV8Pointer> _contexts;
+    // V8TODO: release in destructor
     v8::Persistent<v8::Object> _globalObjectContents;
     bool areGlobalObjectContentsStored {false};
 
+    // Used by ScriptObjectV8Proxy to create JS objects referencing C++ ones
+    // V8TODO: release in destructor
+    v8::Persistent<v8::ObjectTemplate> _objectProxyTemplate;
+    v8::Persistent<v8::ObjectTemplate> _methodDataTemplate;
+    v8::Persistent<v8::ObjectTemplate> _functionDataTemplate;
+    v8::Persistent<v8::ObjectTemplate> _variantDataTemplate;
+    v8::Persistent<v8::ObjectTemplate> _variantProxyTemplate;
+
     //V8TODO
     //ArrayBufferClass* _arrayBufferClass;
     // Counts how many nested evaluate calls are there at a given point

libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp

@@ -258,9 +258,10 @@ void ScriptObjectV8Proxy::investigate() {
     const QMetaObject* metaObject = qobject->metaObject();
 
     //auto objectTemplate = _v8ObjectTemplate.Get(_engine->getIsolate());
-    auto objectTemplate = v8::ObjectTemplate::New(_engine->getIsolate());
+    auto objectTemplate = _engine->getObjectProxyTemplate();
+    /*auto objectTemplate = v8::ObjectTemplate::New(_engine->getIsolate());
     objectTemplate->SetInternalFieldCount(3);
-    objectTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set, nullptr, nullptr, v8GetPropertyNames));
+    objectTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set, nullptr, nullptr, v8GetPropertyNames));*/
 
     //qCDebug(scriptengine_v8) << "Investigate: " << metaObject->className();
     if (QString("ConsoleScriptingInterface") == metaObject->className()) {
@@ -375,21 +376,22 @@ void ScriptObjectV8Proxy::investigate() {
     }
 
     v8::Local<v8::Object> v8Object = objectTemplate->NewInstance(_engine->getContext()).ToLocalChecked();
+    /*if (QString(metaObject->className()) == QString("TestQObject")) {
+        //qDebug() << "TestQObject investigate: _methods.size: " << _methods.size();
+        return;
+    }*/
 
     v8Object->SetAlignedPointerInInternalField(0, const_cast<void*>(internalPointsToQObjectProxy));
     v8Object->SetAlignedPointerInInternalField(1, reinterpret_cast<void*>(this));
-    // Properties added later will be stored in this object
+
-    v8::Local<v8::Object> propertiesObject = v8::Object::New(_engine->getIsolate());
-    v8Object->SetInternalField(2, propertiesObject);
     _v8Object.Reset(_engine->getIsolate(), v8Object);
     if (_ownsObject) {
         _v8Object.SetWeak(this, weakHandleCallback, v8::WeakCallbackType::kParameter);
     }
 
-    /*if (QString(metaObject->className()) == QString("TestQObject")) {
+    // Properties added later will be stored in this object
-        //qDebug() << "TestQObject investigate: _methods.size: " << _methods.size();
+    v8::Local<v8::Object> propertiesObject = v8::Object::New(_engine->getIsolate());
-        return;
+    v8Object->SetInternalField(2, propertiesObject);
-    }*/
 
     // Add all the methods objects as properties - this allows adding properties to a given method later. Is used by Script.request.
     // V8TODO: Should these be deleted when the script-owned object is destroyed? It needs checking if script-owned objects will be garbage-collected, or will self-referencing prevent it.
@@ -774,8 +776,9 @@ ScriptVariantV8Proxy::ScriptVariantV8Proxy(ScriptEngineV8* engine, const QVarian
     v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(engine->getContext());
-    auto variantDataTemplate = v8::ObjectTemplate::New(isolate);
+    auto variantDataTemplate = _engine->getVariantDataTemplate();
-    variantDataTemplate->SetInternalFieldCount(2);
+    //auto variantDataTemplate = v8::ObjectTemplate::New(isolate);
+    //variantDataTemplate->SetInternalFieldCount(2);
     auto variantData = variantDataTemplate->NewInstance(engine->getContext()).ToLocalChecked();
     variantData->SetAlignedPointerInInternalField(0, const_cast<void*>(internalPointsToQVariantInProxy));
     // Internal field doesn't point directly to QVariant, because then alignment would need to be guaranteed in all compilers
@@ -810,9 +813,10 @@ V8ScriptValue ScriptVariantV8Proxy::newVariant(ScriptEngineV8* engine, const QVa
     // V8TODO probably needs connection to be deleted
     auto proxy = new ScriptVariantV8Proxy(engine, variant, proto, protoProxy);
 
-    auto variantProxyTemplate = v8::ObjectTemplate::New(isolate);
+    auto variantProxyTemplate = engine->getVariantProxyTemplate();
-    variantProxyTemplate->SetInternalFieldCount(2);
+    //auto variantProxyTemplate = v8::ObjectTemplate::New(isolate);
-    variantProxyTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set, nullptr, nullptr, v8GetPropertyNames));
+    //variantProxyTemplate->SetInternalFieldCount(2);
+    //variantProxyTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set, nullptr, nullptr, v8GetPropertyNames));
     auto variantProxy = variantProxyTemplate->NewInstance(engine->getContext()).ToLocalChecked();
     variantProxy->SetAlignedPointerInInternalField(0, const_cast<void*>(internalPointsToQVariantProxy));
     variantProxy->SetAlignedPointerInInternalField(1, reinterpret_cast<void*>(proxy));
@@ -1014,8 +1018,9 @@ V8ScriptValue ScriptMethodV8Proxy::newMethod(ScriptEngineV8* engine, QObject* ob
     v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(engine->getContext());
-    auto methodDataTemplate = v8::ObjectTemplate::New(isolate);
+    auto methodDataTemplate = engine->getMethodDataTemplate();
-    methodDataTemplate->SetInternalFieldCount(2);
+    //auto methodDataTemplate = v8::ObjectTemplate::New(isolate);
+    //methodDataTemplate->SetInternalFieldCount(2);
     auto methodData = methodDataTemplate->NewInstance(engine->getContext()).ToLocalChecked();
     methodData->SetAlignedPointerInInternalField(0, const_cast<void*>(internalPointsToMethodProxy));
     // V8TODO it needs to be deleted somehow on object destruction

scripts/developer/debugging/QObjectWrapperGCTest.js

@@ -22,7 +22,8 @@ var memoryStatisticsIntervalHandle = Script.setInterval(function () {
             + " totalGlobalHandlesSize: " + statistics.totalGlobalHandlesSize
             + " usedGlobalHandlesSize: " + statistics.usedGlobalHandlesSize
             + " scriptValueCount: " + statistics.scriptValueCount
-            + " scriptValueProxyCount: " + statistics.scriptValueProxyCount);
+            + " scriptValueProxyCount: " + statistics.scriptValueProxyCount
+            + " qObjectCount: " + statistics.qObjectCount);
     } else {
         print("GC test script memory usage: Total heap size: " + statistics.totalHeapSize
             + " usedHeapSize: " + statistics.usedHeapSize

scripts/developer/debugging/scriptMemoryReport.js

@@ -22,7 +22,8 @@ var memoryStatisticsIntervalHandle = Script.setInterval(function () {
             + " totalGlobalHandlesSize: " + statistics.totalGlobalHandlesSize
             + " usedGlobalHandlesSize: " + statistics.usedGlobalHandlesSize
             + " scriptValueCount: " + statistics.scriptValueCount
-            + " scriptValueProxyCount: " + statistics.scriptValueProxyCount);
+            + " scriptValueProxyCount: " + statistics.scriptValueProxyCount
+            + " qObjectCount: " + statistics.qObjectCount);
     } else {
         print("Script memory usage: Total heap size: " + statistics.totalHeapSize
             + " usedHeapSize: " + statistics.usedHeapSize