From 8936057bd4eb22c70ad648a9e4ca06103c7088c2 Mon Sep 17 00:00:00 2001 From: milkpotatoes Date: Thu, 19 Jun 2025 20:57:08 +0800 Subject: [PATCH] Optimize perfermance of create ref Issue: https://gitee.com/openharmony/arkui_napi/issues/ICGJ21 Signed-off-by: milkpotatoes Change-Id: I13cf1a16df1cd142984b342e201cfc331084b454 --- .../impl/ark/ark_native_reference.cpp | 67 +++++--- native_engine/impl/ark/ark_native_reference.h | 26 +++- test/unittest/engine/test.h | 5 +- test/unittest/test_napi.cpp | 146 +++++++++++++++++- 4 files changed, 219 insertions(+), 25 deletions(-) diff --git a/native_engine/impl/ark/ark_native_reference.cpp b/native_engine/impl/ark/ark_native_reference.cpp index 8a1e99ae4..2a7ec59c7 100644 --- a/native_engine/impl/ark/ark_native_reference.cpp +++ b/native_engine/impl/ark/ark_native_reference.cpp @@ -30,16 +30,15 @@ ArkNativeReference::ArkNativeReference(ArkNativeEngine* engine, bool isAsyncCall, size_t nativeBindingSize) : engine_(engine), - ownership_(deleteSelf ? ReferenceOwnerShip::RUNTIME : ReferenceOwnerShip::USER), value_(engine->GetEcmaVm(), LocalValueFromJsValue(value)), refCount_(initialRefcount), - deleteSelf_(deleteSelf), - isAsyncCall_(isAsyncCall), + ownership_(deleteSelf ? ReferenceOwnerShip::RUNTIME : ReferenceOwnerShip::USER), napiCallback_(napiCallback), data_(data), hint_(hint), nativeBindingSize_(nativeBindingSize) { + InitProperties(deleteSelf, isAsyncCall); ArkNativeReferenceConstructor(); } @@ -47,14 +46,14 @@ ArkNativeReference::ArkNativeReference(ArkNativeEngine* engine, napi_value value, ArkNativeReferenceConfig& config) : engine_(engine), - ownership_(config.deleteSelf ? ReferenceOwnerShip::RUNTIME : ReferenceOwnerShip::USER), value_(), refCount_(config.initialRefcount), + ownership_(config.deleteSelf ? ReferenceOwnerShip::RUNTIME : ReferenceOwnerShip::USER), isProxyReference_(config.isProxyReference), - deleteSelf_(config.deleteSelf), napiCallback_(config.napiCallback), data_(config.data) { + InitProperties(config.deleteSelf, false); value_.CreateXRefGloablReference(engine->GetEcmaVm(), LocalValueFromJsValue(value)); ArkNativeReferenceConstructor(); } @@ -69,16 +68,15 @@ ArkNativeReference::ArkNativeReference(ArkNativeEngine* engine, bool isAsyncCall, size_t nativeBindingSize) : engine_(engine), - ownership_(deleteSelf ? ReferenceOwnerShip::RUNTIME : ReferenceOwnerShip::USER), value_(engine->GetEcmaVm(), value), refCount_(initialRefcount), - deleteSelf_(deleteSelf), - isAsyncCall_(isAsyncCall), + ownership_(deleteSelf ? ReferenceOwnerShip::RUNTIME : ReferenceOwnerShip::USER), napiCallback_(napiCallback), data_(data), hint_(hint), nativeBindingSize_(nativeBindingSize) { + InitProperties(deleteSelf, isAsyncCall); ArkNativeReferenceConstructor(); } @@ -86,7 +84,7 @@ void ArkNativeReference::ArkNativeReferenceConstructor() { if (napiCallback_ != nullptr) { // Async callback will redirect to root engine, no monitoring needed. - if (!isAsyncCall_) { + if (!IsAsyncCall()) { engine_->IncreaseCallbackbleRefCounter(); // Non-callback runtime owned napi_ref will free when env teardown. if (ownership_ == ReferenceOwnerShip::RUNTIME && !engine_->IsMainEnvContext()) { @@ -111,6 +109,19 @@ void ArkNativeReference::ArkNativeReferenceConstructor() engineId_ = engine_->GetId(); } +inline void ArkNativeReference::InitProperties(bool deleteSelf, bool isAsyncCall) +{ + if (deleteSelf) { + SetDeleteSelf(); + } + if (isAsyncCall) { + properties_ |= ReferencePropertiesMask::IS_ASYNC_CALL_MASK; + } +} + +// Do not use pure virtual function of NativeEngine, +// it may cause crash if ArkNativeEngine is deconstructed. +// Which would occur when EcmaVM is destroying in CleanEnv. ArkNativeReference::~ArkNativeReference() { VALID_ENGINE_CHECK(engine_, engine_, engineId_); @@ -128,7 +139,7 @@ ArkNativeReference::~ArkNativeReference() if (value_.IsEmpty()) { return; } - hasDelete_ = true; + SetHasDelete(); if (isProxyReference_) { value_.FreeXRefGlobalHandleAddr(); } else { @@ -246,7 +257,7 @@ void ArkNativeReference::EnqueueDeferredTask() void ArkNativeReference::DispatchFinalizeCallback() { - if (isAsyncCall_) { + if (IsAsyncCall()) { EnqueueAsyncTask(); } else { EnqueueDeferredTask(); @@ -256,11 +267,11 @@ void ArkNativeReference::DispatchFinalizeCallback() void ArkNativeReference::FinalizeCallback(FinalizerState state) { // Invoke the callback only if it is callbackble and has not already been invoked. - if (!finalRun_ && napiCallback_ && !engine_->IsInDestructor()) { + if (!GetFinalRun() && napiCallback_ && !engine_->IsInDestructor()) { if (state == FinalizerState::COLLECTION) { DispatchFinalizeCallback(); } else { - if (!isAsyncCall_) { + if (!IsAsyncCall()) { engine_->DecreaseCallbackbleRefCounter(); } if (ownership_ == ReferenceOwnerShip::RUNTIME) { @@ -271,9 +282,9 @@ void ArkNativeReference::FinalizeCallback(FinalizerState state) } data_ = nullptr; hint_ = nullptr; - finalRun_ = true; + SetFinalRan(); - if (deleteSelf_ && !hasDelete_) { + if (GetDeleteSelf() && !HasDelete()) { delete this; } } @@ -296,12 +307,12 @@ void ArkNativeReference::NativeFinalizeCallBack(void* ref) void ArkNativeReference::SetDeleteSelf() { - deleteSelf_ = true; + properties_ |= ReferencePropertiesMask::DELETE_SELF_MASK; } bool ArkNativeReference::GetDeleteSelf() const { - return deleteSelf_; + return (properties_ & ReferencePropertiesMask::DELETE_SELF_MASK) != 0; } uint32_t ArkNativeReference::GetRefCount() @@ -311,7 +322,7 @@ uint32_t ArkNativeReference::GetRefCount() bool ArkNativeReference::GetFinalRun() { - return finalRun_; + return (properties_ & ReferencePropertiesMask::FINAL_RAN_MASK) != 0; } napi_value ArkNativeReference::GetNapiValue() @@ -326,6 +337,26 @@ void ArkNativeReference::ResetFinalizer() hint_ = nullptr; } +inline bool ArkNativeReference::IsAsyncCall() const +{ + return (properties_ & ReferencePropertiesMask::IS_ASYNC_CALL_MASK) != 0; +} + +inline bool ArkNativeReference::HasDelete() const +{ + return (properties_ & ReferencePropertiesMask::HAS_DELETE_MASK) != 0; +} + +inline void ArkNativeReference::SetHasDelete() +{ + properties_ |= ReferencePropertiesMask::HAS_DELETE_MASK; +} + +inline void ArkNativeReference::SetFinalRan() +{ + properties_ |= ReferencePropertiesMask::FINAL_RAN_MASK; +} + #ifdef PANDA_JS_ETS_HYBRID_MODE void ArkNativeReference::MarkFromObject() { diff --git a/native_engine/impl/ark/ark_native_reference.h b/native_engine/impl/ark/ark_native_reference.h index 3bc67be78..10c272843 100644 --- a/native_engine/impl/ark/ark_native_reference.h +++ b/native_engine/impl/ark/ark_native_reference.h @@ -102,20 +102,28 @@ public: #endif // PANDA_JS_ETS_HYBRID_MODE private: + enum ReferencePropertiesMask : uint8_t { + DELETE_SELF_MASK = 1, + IS_ASYNC_CALL_MASK = DELETE_SELF_MASK << 1, + HAS_DELETE_MASK = IS_ASYNC_CALL_MASK << 1, + FINAL_RAN_MASK = HAS_DELETE_MASK << 1, + }; + void ArkNativeReferenceConstructor(); + void InitProperties(bool deleteSelf = false, bool isAsyncCall = false); ArkNativeEngine* engine_; uint64_t engineId_ {0}; - const ReferenceOwnerShip ownership_; Global value_; uint32_t refCount_ {0}; + + const ReferenceOwnerShip ownership_; + // Bit-packed flags: saves memory and speeds up object creation vs. multiple bools. + // std::bitset will use more memory than uint8_t number. + uint8_t properties_ {0}; bool isProxyReference_{false}; - bool deleteSelf_ {false}; - bool isAsyncCall_ {false}; - bool hasDelete_ {false}; - bool finalRun_ {false}; NapiNativeFinalize napiCallback_ {nullptr}; void* data_ {nullptr}; void* hint_ {nullptr}; @@ -124,11 +132,19 @@ private: NativeReference* prev_ {nullptr}; NativeReference* next_ {nullptr}; + bool IsAsyncCall() const; + bool HasDelete() const; + void SetHasDelete(); + void SetFinalRan(); + void FinalizeCallback(FinalizerState state); void DispatchFinalizeCallback(); void EnqueueAsyncTask(); void EnqueueDeferredTask(); + void IncreaseCounter(); + void DecreaseCounter(); + static void FreeGlobalCallBack(void* ref); static void NativeFinalizeCallBack(void* ref); friend class NativeReferenceManager; diff --git a/test/unittest/engine/test.h b/test/unittest/engine/test.h index 9b3c2abea..5bd0d0e2d 100644 --- a/test/unittest/engine/test.h +++ b/test/unittest/engine/test.h @@ -108,6 +108,10 @@ public: } engine_ = new ArkNativeEngine(vm_, nullptr); + engine_->SetCleanEnv([this] { + panda::JSNApi::DestroyJSVM(vm_); + vm_ = nullptr; + }); loop_ = engine_->GetUVLoop(); napi_open_handle_scope(reinterpret_cast(engine_), &scope_); } @@ -126,7 +130,6 @@ public: scope_ = nullptr; if (!isContextEngine_) { delete engine_; - panda::JSNApi::DestroyJSVM(vm_); } else { engine_->DestroyContext(); } diff --git a/test/unittest/test_napi.cpp b/test/unittest/test_napi.cpp index 7fb07d63c..32d7ea4ad 100644 --- a/test/unittest/test_napi.cpp +++ b/test/unittest/test_napi.cpp @@ -22,6 +22,7 @@ #include #include +#include "ark_native_reference.h" #include "gtest/gtest.h" #include "hilog/log.h" #include "ecmascript/napi/include/jsnapi_expo.h" @@ -29,6 +30,7 @@ #include "napi/native_node_api.h" #include "native_create_env.h" #include "native_utils.h" +#include "reference_manager/native_reference_manager.h" #include "securec.h" #include "test.h" @@ -14162,4 +14164,146 @@ HWTEST_F(NapiBasicTest, NapiEnvCleanupTest001, testing::ext::TestSize.Level1) engine->RunCleanup(); ASSERT_TRUE(collector.Includes("CleanupHandles, request waiting:")); ASSERT_TRUE(workDone); -} \ No newline at end of file +} + +/** + * @tc.name: ArkNativeReferenceTest001 + * @tc.desc: Test code of ArkNativeReference + * @tc.type: FUNC + */ +HWTEST_F(NapiBasicTest, ArkNativeReferenceTest001, testing::ext::TestSize.Level1) +{ + napi_env env = reinterpret_cast(engine_); + napi_value value = nullptr; + ASSERT_CHECK_CALL(napi_create_object(env, &value)); + ArkNativeReference *ref = nullptr; + ASSERT_CHECK_CALL(napi_create_reference(env, value, 0, reinterpret_cast(&ref))); + ASSERT_EQ(ref->properties_ & ArkNativeReference::DELETE_SELF_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::IS_ASYNC_CALL_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::HAS_DELETE_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::FINAL_RAN_MASK, 0); + ASSERT_CHECK_CALL(napi_delete_reference(env, reinterpret_cast(ref))); +} + +/** + * @tc.name: ArkNativeReferenceTest002 + * @tc.desc: Test code of ArkNativeReference + * @tc.type: FUNC + */ +HWTEST_F(NapiBasicTest, ArkNativeReferenceTest002, testing::ext::TestSize.Level1) +{ + napi_env env = reinterpret_cast(engine_); + napi_value value = nullptr; + ASSERT_CHECK_CALL(napi_create_object(env, &value)); + ArkNativeReference *ref = nullptr; + ASSERT_CHECK_CALL(napi_create_reference(env, value, 0, reinterpret_cast(&ref))); + ASSERT_CHECK_CALL(napi_delete_reference(env, reinterpret_cast(ref))); + // Weak ref will mark to delete self instead delete directly + ASSERT_NE(ref->properties_ & ArkNativeReference::DELETE_SELF_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::IS_ASYNC_CALL_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::HAS_DELETE_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::FINAL_RAN_MASK, 0); +} + +/** + * @tc.name: ArkNativeReferenceTest003 + * @tc.desc: Test code of ArkNativeReference + * @tc.type: FUNC + */ +HWTEST_F(NapiBasicTest, ArkNativeReferenceTest003, testing::ext::TestSize.Level1) +{ + napi_env env = reinterpret_cast(engine_); + napi_value value = nullptr; + ASSERT_CHECK_CALL(napi_create_object(env, &value)); + ArkNativeReference** ref = new ArkNativeReference* { nullptr }; + ASSERT_CHECK_CALL(napi_add_finalizer( + env, value, ref, + // This callback is execution under deconstructor of ArkNativeReference + [](napi_env env, void* data, void*) { + ArkNativeReference** secondData = reinterpret_cast(data); + ArkNativeReference* ref = *secondData; + ASSERT_EQ(ref->properties_ & ArkNativeReference::DELETE_SELF_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::IS_ASYNC_CALL_MASK, 0); + ASSERT_NE(ref->properties_ & ArkNativeReference::HAS_DELETE_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::FINAL_RAN_MASK, 0); + delete secondData; + }, + nullptr, reinterpret_cast(ref))); + ASSERT_EQ((*ref)->properties_ & ArkNativeReference::DELETE_SELF_MASK, 0); + ASSERT_EQ((*ref)->properties_ & ArkNativeReference::IS_ASYNC_CALL_MASK, 0); + ASSERT_EQ((*ref)->properties_ & ArkNativeReference::HAS_DELETE_MASK, 0); + ASSERT_EQ((*ref)->properties_ & ArkNativeReference::FINAL_RAN_MASK, 0); + ASSERT_CHECK_CALL(napi_delete_reference(env, reinterpret_cast(*ref))); +} + +/** + * @tc.name: ArkNativeReferenceTest004 + * @tc.desc: Test code of ArkNativeReference + * @tc.type: FUNC + */ +HWTEST_F(NapiBasicTest, ArkNativeReferenceTest004, testing::ext::TestSize.Level1) +{ + UVLoopRunner runner(engine_); + napi_env env = reinterpret_cast(engine_); + ArkNativeReference** ref = new ArkNativeReference* { nullptr }; + { + panda::LocalScope scope(engine_->GetEcmaVm()); + napi_value value = nullptr; + ASSERT_CHECK_CALL(napi_create_object(env, &value)); + ASSERT_CHECK_CALL(napi_add_finalizer( + env, value, ref, + // This callback is execution under deconstructor of ArkNativeReference + [](napi_env env, void* data, void*) { + ArkNativeReference** secondData = reinterpret_cast(data); + ArkNativeReference* ref = *secondData; + ASSERT_NE(ref->properties_ & ArkNativeReference::DELETE_SELF_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::IS_ASYNC_CALL_MASK, 0); + ASSERT_EQ(ref->properties_ & ArkNativeReference::HAS_DELETE_MASK, 0); + ASSERT_NE(ref->properties_ & ArkNativeReference::FINAL_RAN_MASK, 0); + *secondData = nullptr; + }, + nullptr, nullptr)); + // Head is last reference which created above. + *ref = reinterpret_cast(engine_->GetReferenceManager()->references_); + ASSERT_NE((*ref)->properties_ & ArkNativeReference::DELETE_SELF_MASK, 0); + } + ASSERT_NE(ref, nullptr); + panda::JSNApi::TriggerGC(engine_->GetEcmaVm(), panda::ecmascript::GCReason::OTHER, panda::JSNApi::TRIGGER_GC_TYPE::FULL_GC); + runner.Run(); + ASSERT_EQ(*ref, nullptr); + delete ref; +} + +/** + * @tc.name: ArkNativeReferenceTest005 + * @tc.desc: Test code of ~ArkNativeReference in ~NativeEngine. + * This test case would crash if tests failed. + * @tc.type: FUNC + */ +HWTEST_F(NapiBasicTest, ArkNativeReferenceTest005, testing::ext::TestSize.Level1) +{ + struct RefTestData { + napi_env env_ { nullptr }; + napi_ref ref_ { nullptr }; + }; + + RefTestData* testData = new RefTestData; + { + NativeEngineProxy env; + testData->env_ = env; + napi_value val = nullptr; + ASSERT_CHECK_CALL(napi_create_object(env, &val)); + ASSERT_CHECK_CALL(napi_create_reference(env, val, 0, &testData->ref_)); + ASSERT_CHECK_CALL(napi_add_env_cleanup_hook( + env, + [](void* arg) { + RefTestData* data = reinterpret_cast(arg); + ASSERT_CHECK_CALL(napi_delete_reference(data->env_, data->ref_)); + data->ref_ = nullptr; + }, + testData)); + } + testData->env_ = nullptr; + ASSERT_EQ(testData->ref_, nullptr); + delete testData; +} -- Gitee