From a58e9bee51d9417920051b4e70df515482f278d8 Mon Sep 17 00:00:00 2001 From: Yong Zhou Date: Wed, 20 Aug 2025 16:32:28 +0800 Subject: [PATCH] This is combined interface of ForEach and GetSize mainly used by Mark. 1. Reduces Operator overhead 2. Reduces GetSize/Foreach freeObject judgment overhead 3. Reduces GetSize IsForwarded overhead Issue: https://gitee.com/openharmony/arkcompiler_ets_runtime/issues/ICU10X Change-Id: I6a18a6ef26b2961e71f55e136b8be7892af9f061 Signed-off-by: Yong Zhou --- common_components/common/mark_work_stack.h | 8 +- .../heap/ark_collector/ark_collector.cpp | 6 - .../heap/ark_collector/ark_collector.h | 2 - .../tests/ark_collector_test.cpp | 130 +++++++++++++++++- .../heap/collector/marking_collector.cpp | 8 +- .../heap/collector/marking_collector.h | 2 - .../heap/tests/verification_test.cpp | 6 + ecmascript/mem/dynamic_object_operator.h | 11 ++ 8 files changed, 154 insertions(+), 19 deletions(-) diff --git a/common_components/common/mark_work_stack.h b/common_components/common/mark_work_stack.h index 37ee3cce84..b7d621929e 100755 --- a/common_components/common/mark_work_stack.h +++ b/common_components/common/mark_work_stack.h @@ -33,20 +33,20 @@ public: void push_back(T t) { - LOGF_CHECK(!full()) << "Mark stack buffer can not be full when push back"; + ASSERT_LOGF(!full(), "Mark stack buffer can not be full when push back"); stack_[count] = t; count++; } T back() { - LOGF_CHECK(!empty()) << "Mark stack buffer can not be empty when get back"; + ASSERT_LOGF(!empty(), "Mark stack buffer can not be empty when get back"); return stack_[count - 1]; } void pop_back() { - LOGF_CHECK(!empty()) << "Mark stack buffer can not be empty when pop back"; + ASSERT_LOGF(!empty(), "Mark stack buffer can not be empty when pop back"); count--; } size_t count = 0; @@ -138,7 +138,7 @@ public: void pop_back() { - LOGF_CHECK(!empty()) << "Mark stack can not be empty when pop back"; + ASSERT_LOGF(!empty(), "Mark stack can not be empty when pop back"); this->t_->pop_back(); if (this->t_->empty()) { auto tmp = pop(); diff --git a/common_components/heap/ark_collector/ark_collector.cpp b/common_components/heap/ark_collector/ark_collector.cpp index 49985689be..fe263801aa 100755 --- a/common_components/heap/ark_collector/ark_collector.cpp +++ b/common_components/heap/ark_collector/ark_collector.cpp @@ -212,12 +212,6 @@ MarkingCollector::MarkingRefFieldVisitor ArkCollector::CreateMarkingObjectRefFie return visitor; } -void ArkCollector::MarkingObjectRefFields(BaseObject *obj, MarkingRefFieldVisitor *data) -{ - data->SetMarkingRefFieldArgs(obj); - obj->ForEachRefField(data->GetRefFieldVisitor()); -} - void ArkCollector::FixRefField(BaseObject* obj, RefField<>& field) const { RefField<> oldField(field); diff --git a/common_components/heap/ark_collector/ark_collector.h b/common_components/heap/ark_collector/ark_collector.h index a80338c744..a26e5786f1 100755 --- a/common_components/heap/ark_collector/ark_collector.h +++ b/common_components/heap/ark_collector/ark_collector.h @@ -92,8 +92,6 @@ public: MarkingRefFieldVisitor CreateMarkingObjectRefFieldsVisitor(ParallelLocalMarkStack &workStack, WeakStack &weakStack) override; - void MarkingObjectRefFields(BaseObject *obj, MarkingRefFieldVisitor *data) override; - void FixObjectRefFields(BaseObject* obj) const override; void FixRefField(BaseObject* obj, RefField<>& field) const; diff --git a/common_components/heap/ark_collector/tests/ark_collector_test.cpp b/common_components/heap/ark_collector/tests/ark_collector_test.cpp index dc3e611ce0..6ebb09a96c 100644 --- a/common_components/heap/ark_collector/tests/ark_collector_test.cpp +++ b/common_components/heap/ark_collector/tests/ark_collector_test.cpp @@ -21,6 +21,9 @@ #include "common_components/heap/heap_manager.h" #include "common_components/heap/allocator/region_desc.h" #include "common_components/mutator/mutator_manager-inl.h" +#include "objects/base_object.h" +#include "gtest/gtest.h" +#include using namespace common; @@ -202,6 +205,30 @@ private: BaseStateWord stateWord_; }; +class TestBaseObjectOperator : public common::BaseObjectOperatorInterfaces { +public: + bool IsValidObject([[maybe_unused]] const BaseObject *object) const override { return true; } + void ForEachRefField(const BaseObject *object, const common::RefFieldVisitor &visitor) const override {} + size_t GetSize(const BaseObject *object) const override{ return size_; } + size_t ForEachRefFieldAndGetSize( + const BaseObject *object, + const common::RefFieldVisitor &visitor) const override + { + return 0; + } + BaseObject *GetForwardingPointer(const BaseObject *object) const override + { + return fwdPtr_; + } + void SetForwardingPointerAfterExclusive(BaseObject *object, BaseObject *fwdPtr) override + { + fwdPtr_ = fwdPtr; + } + void SetSize(size_t size) { size_ = size; } +private: + size_t size_ = 0; + BaseObject *fwdPtr_ = nullptr; +}; HWTEST_F_L0(ArkCollectorTest, ForwardObject_WithUnmovedObject_ReturnsSameAddress) { std::unique_ptr arkCollector = GetArkCollector(); @@ -334,8 +361,10 @@ public: void AddRawPointerObject(BaseObject*) override {} void RemoveRawPointerObject(BaseObject*) override {} bool MarkObject(BaseObject* obj) const override { return false; } - void MarkingObjectRefFields(BaseObject *obj, MarkingRefFieldVisitor *data) override {} - BaseObject* CopyObjectAfterExclusive(BaseObject* obj) override { return nullptr; } + BaseObject *CopyObjectAfterExclusive(BaseObject *obj) override + { + return nullptr; + } void DoGarbageCollection() override {} bool IsCurrentPointer(RefField<>&) const override { return false; } MarkingRefFieldVisitor CreateMarkingObjectRefFieldsVisitor(ParallelLocalMarkStack &workStack, @@ -388,11 +417,108 @@ HWTEST_F_L0(ArkCollectorTest, FixRefField_TEST2) EXPECT_FALSE(Heap::IsHeapAddress(obj)); } +HWTEST_F_L0(ArkCollectorTest, FixRefField_RefNotInFrom) +{ + // refRegion Not In IsFrom Branch + std::unique_ptr arkCollector = GetArkCollector(); + ASSERT_TRUE(arkCollector != nullptr); + HeapAddress addr = HeapManager::Allocate(8, AllocType::MOVEABLE_OBJECT); + BaseObject* obj = reinterpret_cast(addr); + RegionDesc *refRegion = RegionDesc::GetRegionDescAt( + reinterpret_cast(obj)); + refRegion->SetRegionType(RegionDesc::RegionType::TO_REGION); + RefField<> field(obj); + arkCollector->FixRefField(obj, field); + // Because it is in TO_REGION, it will not be moved, so the results are consistent + EXPECT_EQ((uint64_t)field.GetFieldValue(), (uint64_t)obj); +} + +HWTEST_F_L0(ArkCollectorTest, FixRefField_RefInFrom) +{ + // refRegion In IsFrom Branch + std::unique_ptr arkCollector = GetArkCollector(); + ASSERT_TRUE(arkCollector != nullptr); + TestBaseObjectOperator operatorImpl; + BaseObject::RegisterDynamic(&operatorImpl); + + HeapAddress addr = HeapManager::Allocate(sizeof(BaseObject) * 3, AllocType::MOVEABLE_OBJECT); + HeapAddress fwdAddr = HeapManager::Allocate(sizeof(BaseObject) * 3, AllocType::MOVEABLE_OBJECT); + BaseObject* obj = reinterpret_cast(addr); + BaseObject* fwdObj = reinterpret_cast(fwdAddr); + RegionDesc *refRegion = RegionDesc::GetRegionDescAt( + reinterpret_cast(obj)); + refRegion->SetRegionType(RegionDesc::RegionType::FROM_REGION); + obj->SetForwardState(BaseStateWord::ForwardState::FORWARDING); + obj->SetForwardingPointerAfterExclusive(fwdObj); + RefField<> field(obj); + arkCollector->FixRefField(obj, field); + // Because it is in FromSpace, it will be forwarded to a new pointer + EXPECT_EQ((uint64_t)field.GetFieldValue(), (uint64_t)fwdObj); +} + +HWTEST_F_L0(ArkCollectorTest, FixRefField_RefInRecent1) +{ + // refRegion InRecent + std::unique_ptr arkCollector = GetArkCollector(); + ASSERT_TRUE(arkCollector != nullptr); + HeapAddress addr = HeapManager::Allocate(8, AllocType::MOVEABLE_OBJECT); + BaseObject* ref = reinterpret_cast(addr); + RegionDesc *refRegion = RegionDesc::GetRegionDescAt( + reinterpret_cast(ref)); + refRegion->SetRegionType(RegionDesc::RegionType::RECENT_FULL_REGION); + RefField<> field(ref); + + // && objRegion InRecent + HeapAddress addr2 = HeapManager::Allocate(8, AllocType::MOVEABLE_OLD_OBJECT); + BaseObject* obj = reinterpret_cast(addr2); + RegionDesc *objRegion = RegionDesc::GetRegionDescAt( + reinterpret_cast(obj)); + objRegion->SetRegionType(RegionDesc::RegionType::RECENT_FULL_REGION); + objRegion->ClearRSet(); + arkCollector->FixRefField(obj, field); + // refRegion in RecentSpace will not be forwarded (fwd), + // because it belongs to the Recent generation + // so the Remembered Set (Rset) will not be updated, resulting in false + EXPECT_EQ((uint64_t)field.GetFieldValue(), (uint64_t)ref); + EXPECT_EQ(objRegion->MarkRSetCardTable(obj), false); +} + +HWTEST_F_L0(ArkCollectorTest, FixRefField_RefInRecent2) +{ + // refRegion InRecent + std::unique_ptr arkCollector = GetArkCollector(); + ASSERT_TRUE(arkCollector != nullptr); + HeapAddress addr = HeapManager::Allocate(8, AllocType::MOVEABLE_OBJECT); + BaseObject* ref = reinterpret_cast(addr); + RegionDesc *refRegion = RegionDesc::GetRegionDescAt( + reinterpret_cast(ref)); + refRegion->SetRegionType(RegionDesc::RegionType::RECENT_FULL_REGION); + RefField<> field(ref); + + // && !objRegion InRecent + HeapAddress addr2 = HeapManager::Allocate(8, AllocType::MOVEABLE_OLD_OBJECT); + BaseObject* obj = reinterpret_cast(addr2); + RegionDesc *objRegion = RegionDesc::GetRegionDescAt( + reinterpret_cast(obj)); + objRegion->SetRegionType(RegionDesc::RegionType::TO_REGION); + // && !objRegion->MarkRSetCardTable(obj) + objRegion->ClearRSet(); + arkCollector->FixRefField(obj, field); + EXPECT_EQ((uint64_t)field.GetFieldValue(), (uint64_t)ref); + EXPECT_EQ(objRegion->MarkRSetCardTable(obj), true); +} + class TestStaticObject : public BaseObjectOperatorInterfaces { public: size_t GetSize(const BaseObject *object) const override { return 0; } bool IsValidObject(const BaseObject *object) const override { return false; } void ForEachRefField(const BaseObject *object, const RefFieldVisitor &visitor) const override {} + size_t ForEachRefFieldAndGetSize( + const BaseObject *object, + const common::RefFieldVisitor &visitor) const override + { + return 0; + } void SetForwardingPointerAfterExclusive(BaseObject *object, BaseObject *fwdPtr) override {} BaseObject *GetForwardingPointer(const BaseObject *object) const override { diff --git a/common_components/heap/collector/marking_collector.cpp b/common_components/heap/collector/marking_collector.cpp index 241f35a168..c2ec9aee53 100755 --- a/common_components/heap/collector/marking_collector.cpp +++ b/common_components/heap/collector/marking_collector.cpp @@ -179,9 +179,11 @@ void MarkingCollector::ProcessMarkStack([[maybe_unused]] uint32_t threadIndex, P BaseObject *object; while (markStack.Pop(&object)) { ++nNewlyMarked; - auto region = RegionDesc::GetAliveRegionDescAt(static_cast(reinterpret_cast(object))); - region->AddLiveByteCount(object->GetSize()); - MarkingObjectRefFields(object, &visitor); + auto region = RegionDesc::GetAliveRegionDescAt( + static_cast(reinterpret_cast(object))); + visitor.SetMarkingRefFieldArgs(object); + auto objSize = object->ForEachRefFieldAndGetSize(visitor.GetRefFieldVisitor()); + region->AddLiveByteCount(objSize); } // Try some task from satb buffer, bound the loop to make sure it converges in time if (++iterationCnt >= maxIterationLoopNum) { diff --git a/common_components/heap/collector/marking_collector.h b/common_components/heap/collector/marking_collector.h index fd8c46c7f2..421906e3ca 100755 --- a/common_components/heap/collector/marking_collector.h +++ b/common_components/heap/collector/marking_collector.h @@ -218,8 +218,6 @@ public: }; virtual MarkingRefFieldVisitor CreateMarkingObjectRefFieldsVisitor(ParallelLocalMarkStack &workStack, WeakStack &weakStack) = 0; - virtual void MarkingObjectRefFields(BaseObject *obj, MarkingRefFieldVisitor *data) = 0; - inline bool IsResurrectedObject(const BaseObject* obj) const { return RegionalHeap::IsResurrectedObject(obj); } Allocator& GetAllocator() const { return theAllocator_; } diff --git a/common_components/heap/tests/verification_test.cpp b/common_components/heap/tests/verification_test.cpp index 1709de37c1..d5815e379c 100644 --- a/common_components/heap/tests/verification_test.cpp +++ b/common_components/heap/tests/verification_test.cpp @@ -25,6 +25,12 @@ class TestBaseObjectOperator : public common::BaseObjectOperatorInterfaces { public: bool IsValidObject([[maybe_unused]] const BaseObject *object) const override { return enbaleValidObject_; } void ForEachRefField(const BaseObject *object, const common::RefFieldVisitor &visitor) const override {} + size_t ForEachRefFieldAndGetSize( + const BaseObject *object, + const common::RefFieldVisitor &visitor) const override + { + return 0; + } size_t GetSize(const BaseObject *object) const override{ return size_; } BaseObject *GetForwardingPointer(const BaseObject *object) const override { return nullptr; } void SetForwardingPointerAfterExclusive(BaseObject *object, BaseObject *fwdPtr) override {} diff --git a/ecmascript/mem/dynamic_object_operator.h b/ecmascript/mem/dynamic_object_operator.h index 3f79e3f422..b2983f6768 100644 --- a/ecmascript/mem/dynamic_object_operator.h +++ b/ecmascript/mem/dynamic_object_operator.h @@ -109,6 +109,17 @@ public: } } + size_t ForEachRefFieldAndGetSize(const BaseObject *object, const common::RefFieldVisitor &visitor) const override + { + // Only used in the MarkingPhase phase. + auto obj = TaggedObject::Cast(object); + auto klass = obj->GetClass(); + auto size = klass->SizeFromJSHClass(obj); + RefFieldObjectVisitor refFieldObjectVisitor(visitor); + refFieldObjectVisitor.VisitAllRefFields(obj); + return size; + } + size_t GetSize(const BaseObject *object) const override { ASSERT(!g_isEnableCMCGC || !object->IsForwarded()); -- Gitee