diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index bb4a20af93..e7d638afa0 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -26,6 +26,9 @@ #include "ScriptValue.h" #include "ScriptException.h" +// These are used for debugging memory leaks caused by persistent handles +#define OVERTE_V8_HANDLE_COUNTERS + class QByteArray; class QLatin1String; class QString; @@ -53,6 +56,10 @@ public: size_t totalAvailableSize; size_t totalGlobalHandlesSize; size_t usedGlobalHandlesSize; +#ifdef OVERTE_V8_HANDLE_COUNTERS + size_t scriptValueCount; + size_t scriptValueProxyCount; +#endif }; /** diff --git a/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp b/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp index 3c4994d1a5..93ad37d6ca 100644 --- a/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp +++ b/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp @@ -61,6 +61,10 @@ QVariantMap ScriptManagerScriptingInterface::getMemoryUsageStatistics() { map.insert("totalAvailableSize", QVariant((qulonglong)(statistics.totalAvailableSize))); map.insert("totalGlobalHandlesSize", QVariant((qulonglong)(statistics.totalGlobalHandlesSize))); map.insert("usedGlobalHandlesSize", QVariant((qulonglong)(statistics.usedGlobalHandlesSize))); +#ifdef OVERTE_V8_HANDLE_COUNTERS + map.insert("scriptValueCount", QVariant((qulonglong)(statistics.scriptValueCount))); + map.insert("scriptValueProxyCount", QVariant((qulonglong)(statistics.scriptValueProxyCount))); +#endif return map; } diff --git a/libraries/script-engine/src/ScriptValue.h b/libraries/script-engine/src/ScriptValue.h index 55ed804ccf..8aaded85a9 100644 --- a/libraries/script-engine/src/ScriptValue.h +++ b/libraries/script-engine/src/ScriptValue.h @@ -179,7 +179,7 @@ public: virtual QObject* toQObject() const = 0; protected: - ~ScriptValueProxy() {} // prevent explicit deletion of base class + virtual ~ScriptValueProxy() {} // prevent explicit deletion of base class }; // the second template parameter is used to defer evaluation of calls to the engine until ScriptEngine isn't forward-declared diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp index 31a5e3e1ab..07d8423a2a 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8.cpp @@ -1720,6 +1720,10 @@ ScriptEngineMemoryStatistics ScriptEngineV8::getMemoryUsageStatistics() { statistics.totalAvailableSize = heapStatistics.total_available_size(); statistics.totalGlobalHandlesSize = heapStatistics.total_global_handles_size(); statistics.usedGlobalHandlesSize = heapStatistics.used_global_handles_size(); +#ifdef OVERTE_V8_HANDLE_COUNTERS + statistics.scriptValueCount = scriptValueCount; + statistics.scriptValueProxyCount = scriptValueProxyCount; +#endif return statistics; } \ No newline at end of file diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.h b/libraries/script-engine/src/v8/ScriptEngineV8.h index 99cd5a70db..c8bb460ef1 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.h +++ b/libraries/script-engine/src/v8/ScriptEngineV8.h @@ -205,6 +205,13 @@ public: // not for public use, but I don't like how Qt strings this along with p void popContext(); // V8TODO: call this after initializing global object void storeGlobalObjectContents(); +#ifdef OVERTE_V8_HANDLE_COUNTERS + void incrementScriptValueCounter() { scriptValueCount++; }; + void decrementScriptValueCounter() { scriptValueCount--; }; + void incrementScriptValueProxyCounter() { scriptValueProxyCount++; }; + void decrementScriptValueProxyCounter() { scriptValueProxyCount--; }; + //V8TODO: do the same for other proxy objects +#endif protected: // like `newFunction`, but allows mapping inline C++ lambdas with captures as callable V8ScriptValues @@ -257,6 +264,10 @@ protected: //ArrayBufferClass* _arrayBufferClass; // Counts how many nested evaluate calls are there at a given point int _evaluatingCounter; +#ifdef OVERTE_V8_HANDLE_COUNTERS + std::atomic scriptValueCount{0}; + std::atomic scriptValueProxyCount{0}; +#endif }; #include "V8Types.h" diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index 005bc3e7b3..95fbd15f10 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -376,7 +376,6 @@ void ScriptObjectV8Proxy::investigate() { v8Object->SetInternalField(2, propertiesObject); _v8Object.Reset(_engine->getIsolate(), v8Object); if (_ownsObject) { - qDebug(scriptengine_v8) << "ScriptObjectV8Proxy::investigate SetWeak"; _v8Object.SetWeak(this, weakHandleCallback, v8::WeakCallbackType::kParameter); } diff --git a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp index 16189a4b8e..4b8c12abd5 100644 --- a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp @@ -21,7 +21,7 @@ void ScriptValueV8Wrapper::release() { // V8TODO: maybe add an assert to check if it happens on script engine thread? - // With v8::Locker in V8ScriptValue such requirement shouldn't be necessary but deleting on different threadwww can cause deadlocks sometimes + // With v8::Locker in V8ScriptValue such requirement shouldn't be necessary but deleting on different thread can cause deadlocks sometimes delete this; } @@ -31,8 +31,8 @@ ScriptValueProxy* ScriptValueV8Wrapper::copy() const { v8::Locker locker(isolate); v8::Isolate::Scope isolateScope(isolate); v8::HandleScope handleScope(isolate); - // V8TODO: I'm not sure if this part is right: v8::Context::Scope contextScope(_engine->getContext()); + // V8TODO: I'm not sure if this part is right: ScriptValueV8Wrapper *copiedWrapper = new ScriptValueV8Wrapper(_engine, _value); return copiedWrapper; } diff --git a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.h b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.h index 233e5c62a1..f7d4ba8eb1 100644 --- a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.h +++ b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.h @@ -29,14 +29,16 @@ /// [V8] Implements ScriptValue for V8 and translates calls for V8ScriptValue class ScriptValueV8Wrapper final : public ScriptValueProxy { public: // construction + ScriptValueV8Wrapper() = delete; + //ScriptValueV8Wrapper(ScriptValueV8Wrapper &) = delete; inline ScriptValueV8Wrapper(ScriptEngineV8* engine, const V8ScriptValue& value) : - _engine(engine), _value(value) {} + _engine(engine), _value(value) { engine->incrementScriptValueProxyCounter(); } inline ScriptValueV8Wrapper(ScriptEngineV8* engine, V8ScriptValue&& value) : - _engine(engine), _value(std::move(value)) {} + _engine(engine), _value(std::move(value)) { engine->incrementScriptValueProxyCounter(); } static ScriptValueV8Wrapper* unwrap(const ScriptValue& val); inline const V8ScriptValue& toV8Value() const { return _value; } static V8ScriptValue fullUnwrap(ScriptEngineV8* engine, const ScriptValue& value); - ScriptEngineV8* getV8Engine() {return _engine;} + ScriptEngineV8* getV8Engine() {return _engine;}; public: virtual void release() override; @@ -91,12 +93,18 @@ public: // ScriptValue implementation virtual QVariant toVariant() const override; virtual QObject* toQObject() const override; +protected: + virtual ~ScriptValueV8Wrapper() { _engine->decrementScriptValueProxyCounter(); }; + private: // helper functions V8ScriptValue fullUnwrap(const ScriptValue& value) const; private: // storage ScriptEngineV8 *_engine; + //V8TODO: this needs a persistent handle instead, maybe with set weak? V8ScriptValue _value; + + Q_DISABLE_COPY(ScriptValueV8Wrapper) }; #endif // hifi_ScriptValueV8Wrapper_h diff --git a/libraries/script-engine/src/v8/V8Types.h b/libraries/script-engine/src/v8/V8Types.h index c64a807c32..fc67e45691 100644 --- a/libraries/script-engine/src/v8/V8Types.h +++ b/libraries/script-engine/src/v8/V8Types.h @@ -47,6 +47,9 @@ public: return; } Q_ASSERT(false);*/ +#ifdef OVERTE_V8_HANDLE_COUNTERS + _engine->incrementScriptValueCounter(); +#endif _value.reset(new v8::UniquePersistent(_engine->getIsolate(), value)); }; @@ -67,6 +70,9 @@ public: v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), v8::Local())); +#ifdef OVERTE_V8_HANDLE_COUNTERS + _engine->incrementScriptValueCounter(); +#endif _value.reset(new v8::UniquePersistent(_engine->getIsolate(), v8::Local())); }; @@ -76,6 +82,9 @@ public: v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), copied.constGet())); +#ifdef OVERTE_V8_HANDLE_COUNTERS + _engine->incrementScriptValueCounter(); +#endif _value.reset(new v8::UniquePersistent(_engine->getIsolate(), copied.constGet())); } @@ -127,6 +136,9 @@ public: v8::Isolate::Scope isolateScope(_engine->getIsolate()); v8::HandleScope handleScope(_engine->getIsolate()); //v8::Context::Scope(_engine->getContext()); +#ifdef OVERTE_V8_HANDLE_COUNTERS + _engine->decrementScriptValueCounter(); +#endif _value->Reset(); } diff --git a/scripts/developer/debugging/QObjectWrapperGCTest.js b/scripts/developer/debugging/QObjectWrapperGCTest.js index 4cfac22a63..4d3e41f481 100644 --- a/scripts/developer/debugging/QObjectWrapperGCTest.js +++ b/scripts/developer/debugging/QObjectWrapperGCTest.js @@ -13,6 +13,24 @@ // A simple script that prints memory usage statistics for a given script engine every 5 seconds // It can be included for example as a part of default scripts or controller scripts +var memoryStatisticsIntervalHandle = Script.setInterval(function () { + statistics = Script.getMemoryUsageStatistics(); + if (statistics.scriptValueCount != null) { + print("GC test script memory usage: Total heap size: " + statistics.totalHeapSize + + " usedHeapSize: " + statistics.usedHeapSize + + " totalAvailableSize: " + statistics.totalAvailableSize + + " totalGlobalHandlesSize: " + statistics.totalGlobalHandlesSize + + " usedGlobalHandlesSize: " + statistics.usedGlobalHandlesSize + + " scriptValueCount: " + statistics.scriptValueCount + + " scriptValueProxyCount: " + statistics.scriptValueProxyCount); + } else { + print("GC test script memory usage: Total heap size: " + statistics.totalHeapSize + + " usedHeapSize: " + statistics.usedHeapSize + + " totalAvailableSize: " + statistics.totalAvailableSize + + " totalGlobalHandlesSize: " + statistics.totalGlobalHandlesSize + + " usedGlobalHandlesSize: " + statistics.usedGlobalHandlesSize); + } +}, 5000); Script.setInterval(function () { for (let i = 0; i < 50000; i++) { let dbgobj = Script.createGarbageCollectorDebuggingObject(); diff --git a/scripts/developer/debugging/scriptMemoryReport.js b/scripts/developer/debugging/scriptMemoryReport.js index d5c8402f32..d2f109a25f 100644 --- a/scripts/developer/debugging/scriptMemoryReport.js +++ b/scripts/developer/debugging/scriptMemoryReport.js @@ -15,11 +15,21 @@ var memoryStatisticsIntervalHandle = Script.setInterval(function () { statistics = Script.getMemoryUsageStatistics(); - print("Script memory usage: Total heap size: " + statistics.totalHeapSize - + " usedHeapSize: " + statistics.usedHeapSize - + " totalAvailableSize: " + statistics.totalAvailableSize - + " totalGlobalHandlesSize: " + statistics.totalGlobalHandlesSize - + " usedGlobalHandlesSize: " + statistics.usedGlobalHandlesSize); + if (statistics.scriptValueCount != null) { + print("Script memory usage: Total heap size: " + statistics.totalHeapSize + + " usedHeapSize: " + statistics.usedHeapSize + + " totalAvailableSize: " + statistics.totalAvailableSize + + " totalGlobalHandlesSize: " + statistics.totalGlobalHandlesSize + + " usedGlobalHandlesSize: " + statistics.usedGlobalHandlesSize + + " scriptValueCount: " + statistics.scriptValueCount + + " scriptValueProxyCount: " + statistics.scriptValueProxyCount); + } else { + print("Script memory usage: Total heap size: " + statistics.totalHeapSize + + " usedHeapSize: " + statistics.usedHeapSize + + " totalAvailableSize: " + statistics.totalAvailableSize + + " totalGlobalHandlesSize: " + statistics.totalGlobalHandlesSize + + " usedGlobalHandlesSize: " + statistics.usedGlobalHandlesSize); + } }, 5000); Script.scriptEnding.connect(function () {