From b3de7fcb2d468aa41c75ab5d7eb43d0fb4b35ebe Mon Sep 17 00:00:00 2001 From: James Chen Date: Thu, 1 Aug 2024 16:27:28 +0800 Subject: [PATCH 1/5] Add ExternalStringResource::freeMemory --- native/cocos/bindings/jswrapper/v8/Utils.cpp | 24 ++++++++++++++++++++ native/cocos/bindings/jswrapper/v8/Utils.h | 20 ++++------------ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/native/cocos/bindings/jswrapper/v8/Utils.cpp b/native/cocos/bindings/jswrapper/v8/Utils.cpp index 6e5334253fa..7936edffee0 100644 --- a/native/cocos/bindings/jswrapper/v8/Utils.cpp +++ b/native/cocos/bindings/jswrapper/v8/Utils.cpp @@ -243,6 +243,30 @@ void clearPrivate(v8::Isolate *isolate, ObjectWrap &wrap) { } } +std::atomic_int gExternalCount = 0; + +// ExternalStringResource +ExternalStringResource::ExternalStringResource(std::u16string &&s) +: _s(std::move(s)) { +} + +const uint16_t* ExternalStringResource::data() const { + return reinterpret_cast(_s.data()); +} + +size_t ExternalStringResource::length() const { + return _s.length(); +} + +void ExternalStringResource::Dispose() { + delete this; +} + +void ExternalStringResource::freeMemory() { + _s.clear(); + _s.shrink_to_fit(); +} + } // namespace internal } // namespace se diff --git a/native/cocos/bindings/jswrapper/v8/Utils.h b/native/cocos/bindings/jswrapper/v8/Utils.h index a906862ed3c..66f996ec677 100644 --- a/native/cocos/bindings/jswrapper/v8/Utils.h +++ b/native/cocos/bindings/jswrapper/v8/Utils.h @@ -52,24 +52,14 @@ void clearPrivate(v8::Isolate *isolate, ObjectWrap &wrap); class ExternalStringResource : public v8::String::ExternalStringResource { public: - explicit ExternalStringResource(std::u16string &&s) - : _s(std::move(s)) { - - } - + explicit ExternalStringResource(std::u16string &&s); ~ExternalStringResource() override = default; - const uint16_t* data() const override { - return reinterpret_cast(_s.data()); - } + const uint16_t* data() const override; + size_t length() const override; + void Dispose() override; - size_t length() const override { - return _s.length(); - } - - void Dispose() override { - delete this; - } + void freeMemory(); private: std::u16string _s; }; From df6a24927dcc63bca7954ad61d5f0ce48f915270 Mon Sep 17 00:00:00 2001 From: James Chen Date: Thu, 1 Aug 2024 17:39:27 +0800 Subject: [PATCH 2/5] [optimization] free string memory after JSON is parsed. V8's garbage collector only take care of JS Heap and ignore the external string size. So ExternalStringResource::Dispose may be delayed which will cause memory in high value. --- native/cocos/bindings/jswrapper/v8/Object.cpp | 12 +++++++++++- native/cocos/bindings/jswrapper/v8/Utils.cpp | 13 ++++++++++++- native/cocos/bindings/jswrapper/v8/Utils.h | 4 ++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/native/cocos/bindings/jswrapper/v8/Object.cpp b/native/cocos/bindings/jswrapper/v8/Object.cpp index 92b0424c9ca..451134f8987 100644 --- a/native/cocos/bindings/jswrapper/v8/Object.cpp +++ b/native/cocos/bindings/jswrapper/v8/Object.cpp @@ -401,13 +401,23 @@ Object *Object::createJSONObject(const ccstd::string &jsonStr) { } Object *Object::createJSONObject(std::u16string &&jsonStr) { - auto v8Str = v8::String::NewExternalTwoByte(__isolate, ccnew internal::ExternalStringResource(std::move(jsonStr))); + auto *external = ccnew internal::ExternalStringResource(std::move(jsonStr)); + external->addRef(); + + auto v8Str = v8::String::NewExternalTwoByte(__isolate, external); if (v8Str.IsEmpty()) { + external->release(); return nullptr; } v8::Local context = __isolate->GetCurrentContext(); v8::MaybeLocal ret = v8::JSON::Parse(context, v8Str.ToLocalChecked()); + + // After v8::JSON::Parse, the memory of u16string could be freed. + external->freeMemory(); + external->release(); + external = nullptr; + if (ret.IsEmpty()) { return nullptr; } diff --git a/native/cocos/bindings/jswrapper/v8/Utils.cpp b/native/cocos/bindings/jswrapper/v8/Utils.cpp index 7936edffee0..88417cb1b24 100644 --- a/native/cocos/bindings/jswrapper/v8/Utils.cpp +++ b/native/cocos/bindings/jswrapper/v8/Utils.cpp @@ -259,7 +259,7 @@ size_t ExternalStringResource::length() const { } void ExternalStringResource::Dispose() { - delete this; + release(); } void ExternalStringResource::freeMemory() { @@ -267,6 +267,17 @@ void ExternalStringResource::freeMemory() { _s.shrink_to_fit(); } +void ExternalStringResource::addRef() { + ++_refCount; +} + +void ExternalStringResource::release() { + --_refCount; + if (_refCount == 0) { + delete this; + } +} + } // namespace internal } // namespace se diff --git a/native/cocos/bindings/jswrapper/v8/Utils.h b/native/cocos/bindings/jswrapper/v8/Utils.h index 66f996ec677..cb8a1d9e280 100644 --- a/native/cocos/bindings/jswrapper/v8/Utils.h +++ b/native/cocos/bindings/jswrapper/v8/Utils.h @@ -60,8 +60,12 @@ class ExternalStringResource : public v8::String::ExternalStringResource { void Dispose() override; void freeMemory(); + + void addRef(); + void release(); private: std::u16string _s; + uint32_t _refCount{1}; }; } // namespace internal From 37f0f339c67401979576f467761945f2922c5ab6 Mon Sep 17 00:00:00 2001 From: James Chen Date: Thu, 1 Aug 2024 17:42:02 +0800 Subject: [PATCH 3/5] Remove unused code. --- native/cocos/bindings/jswrapper/v8/Utils.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/native/cocos/bindings/jswrapper/v8/Utils.cpp b/native/cocos/bindings/jswrapper/v8/Utils.cpp index 88417cb1b24..7bc0befb6f8 100644 --- a/native/cocos/bindings/jswrapper/v8/Utils.cpp +++ b/native/cocos/bindings/jswrapper/v8/Utils.cpp @@ -243,8 +243,6 @@ void clearPrivate(v8::Isolate *isolate, ObjectWrap &wrap) { } } -std::atomic_int gExternalCount = 0; - // ExternalStringResource ExternalStringResource::ExternalStringResource(std::u16string &&s) : _s(std::move(s)) { From 7e723bc56f95a82dd78dc91c2e98a536eb263527 Mon Sep 17 00:00:00 2001 From: James Chen Date: Thu, 1 Aug 2024 17:54:52 +0800 Subject: [PATCH 4/5] Revert. --- native/cocos/bindings/jswrapper/v8/Utils.cpp | 13 +------------ native/cocos/bindings/jswrapper/v8/Utils.h | 5 +---- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/native/cocos/bindings/jswrapper/v8/Utils.cpp b/native/cocos/bindings/jswrapper/v8/Utils.cpp index 7bc0befb6f8..a2f7cd4f82c 100644 --- a/native/cocos/bindings/jswrapper/v8/Utils.cpp +++ b/native/cocos/bindings/jswrapper/v8/Utils.cpp @@ -257,7 +257,7 @@ size_t ExternalStringResource::length() const { } void ExternalStringResource::Dispose() { - release(); + delete this; } void ExternalStringResource::freeMemory() { @@ -265,17 +265,6 @@ void ExternalStringResource::freeMemory() { _s.shrink_to_fit(); } -void ExternalStringResource::addRef() { - ++_refCount; -} - -void ExternalStringResource::release() { - --_refCount; - if (_refCount == 0) { - delete this; - } -} - } // namespace internal } // namespace se diff --git a/native/cocos/bindings/jswrapper/v8/Utils.h b/native/cocos/bindings/jswrapper/v8/Utils.h index cb8a1d9e280..3528057be42 100644 --- a/native/cocos/bindings/jswrapper/v8/Utils.h +++ b/native/cocos/bindings/jswrapper/v8/Utils.h @@ -60,12 +60,9 @@ class ExternalStringResource : public v8::String::ExternalStringResource { void Dispose() override; void freeMemory(); - - void addRef(); - void release(); + private: std::u16string _s; - uint32_t _refCount{1}; }; } // namespace internal From 8063452106173b287f474e346e68f7b50144e996 Mon Sep 17 00:00:00 2001 From: James Chen Date: Thu, 1 Aug 2024 17:55:52 +0800 Subject: [PATCH 5/5] Tweak. --- native/cocos/bindings/jswrapper/v8/Object.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/native/cocos/bindings/jswrapper/v8/Object.cpp b/native/cocos/bindings/jswrapper/v8/Object.cpp index 451134f8987..a7a094705a7 100644 --- a/native/cocos/bindings/jswrapper/v8/Object.cpp +++ b/native/cocos/bindings/jswrapper/v8/Object.cpp @@ -402,11 +402,8 @@ Object *Object::createJSONObject(const ccstd::string &jsonStr) { Object *Object::createJSONObject(std::u16string &&jsonStr) { auto *external = ccnew internal::ExternalStringResource(std::move(jsonStr)); - external->addRef(); - auto v8Str = v8::String::NewExternalTwoByte(__isolate, external); if (v8Str.IsEmpty()) { - external->release(); return nullptr; } @@ -415,8 +412,6 @@ Object *Object::createJSONObject(std::u16string &&jsonStr) { // After v8::JSON::Parse, the memory of u16string could be freed. external->freeMemory(); - external->release(); - external = nullptr; if (ret.IsEmpty()) { return nullptr;