From 30550a6da132f90a23cc96655d96ca4dad4024c2 Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Thu, 11 Jan 2024 16:47:44 +0100 Subject: [PATCH] [K/N] Immediately destroy objects that finalize only via ExtraObjectData ^KT-63423 --- .../alloc/common/cpp/AllocatorTestSupport.hpp | 1 + .../custom/cpp/AllocatorImplTestSupport.cpp | 7 +++++++ .../custom/cpp/CustomFinalizerProcessor.hpp | 9 +++++++-- .../runtime/src/alloc/custom/cpp/GCApi.cpp | 15 ++++++++++++--- .../legacy/cpp/AllocatorImplTestSupport.cpp | 5 +++++ .../alloc/legacy/cpp/ExtraObjectDataFactory.hpp | 16 +++++++++++++++- .../runtime/src/gc/common/cpp/TracingGCTest.hpp | 16 ---------------- .../runtime/src/main/cpp/FinalizerHooks.cpp | 6 +++++- .../runtime/src/main/cpp/FinalizerHooks.hpp | 1 + .../runtime/src/main/cpp/MultiSourceQueue.hpp | 6 ++++++ .../runtime/src/mm/cpp/ExtraObjectData.cpp | 5 ++++- .../runtime/src/mm/cpp/ExtraObjectDataTest.cpp | 4 ++-- 12 files changed, 65 insertions(+), 26 deletions(-) diff --git a/kotlin-native/runtime/src/alloc/common/cpp/AllocatorTestSupport.hpp b/kotlin-native/runtime/src/alloc/common/cpp/AllocatorTestSupport.hpp index 038bbd232af..f0f310e7c74 100644 --- a/kotlin-native/runtime/src/alloc/common/cpp/AllocatorTestSupport.hpp +++ b/kotlin-native/runtime/src/alloc/common/cpp/AllocatorTestSupport.hpp @@ -15,5 +15,6 @@ namespace kotlin::alloc::test_support { void assertClear(Allocator& allocator) noexcept; std::vector allocatedObjects(mm::ThreadData& threadData) noexcept; +void detachAndDestroyExtraObjectData(mm::ExtraObjectData& extraObject) noexcept; } // namespace kotlin::alloc::test_support diff --git a/kotlin-native/runtime/src/alloc/custom/cpp/AllocatorImplTestSupport.cpp b/kotlin-native/runtime/src/alloc/custom/cpp/AllocatorImplTestSupport.cpp index ac3b76da346..8f8cf6addd0 100644 --- a/kotlin-native/runtime/src/alloc/custom/cpp/AllocatorImplTestSupport.cpp +++ b/kotlin-native/runtime/src/alloc/custom/cpp/AllocatorImplTestSupport.cpp @@ -33,3 +33,10 @@ void alloc::test_support::assertClear(Allocator& allocator) noexcept { std::vector alloc::test_support::allocatedObjects(mm::ThreadData& threadData) noexcept { return threadData.allocator().impl().alloc().heap().GetAllocatedObjects(); } + +void alloc::test_support::detachAndDestroyExtraObjectData(mm::ExtraObjectData& extraObject) noexcept { + extraObject.ClearRegularWeakReferenceImpl(); + extraObject.UnlinkFromBaseObject(); + destroyExtraObjectData(extraObject); + extraObject.setFlag(mm::ExtraObjectData::FLAGS_SWEEPABLE); +} \ No newline at end of file diff --git a/kotlin-native/runtime/src/alloc/custom/cpp/CustomFinalizerProcessor.hpp b/kotlin-native/runtime/src/alloc/custom/cpp/CustomFinalizerProcessor.hpp index b87205b1667..78d17db33f0 100644 --- a/kotlin-native/runtime/src/alloc/custom/cpp/CustomFinalizerProcessor.hpp +++ b/kotlin-native/runtime/src/alloc/custom/cpp/CustomFinalizerProcessor.hpp @@ -23,8 +23,13 @@ struct FinalizerQueueTraits { static void process(FinalizerQueue queue) noexcept { while (auto* cell = queue.Pop()) { auto* extraObject = cell->Data(); - auto* baseObject = extraObject->GetBaseObject(); - RunFinalizers(baseObject); + if (auto* baseObject = extraObject->GetBaseObject()) { + RunFinalizers(baseObject); + } else { + // This `ExtraObjectData` does not have an object attached. This means + // that the only finalization step is destroying it. + destroyExtraObjectData(*extraObject); + } } } }; diff --git a/kotlin-native/runtime/src/alloc/custom/cpp/GCApi.cpp b/kotlin-native/runtime/src/alloc/custom/cpp/GCApi.cpp index 07e992cf4f6..95c1c30a6b7 100644 --- a/kotlin-native/runtime/src/alloc/custom/cpp/GCApi.cpp +++ b/kotlin-native/runtime/src/alloc/custom/cpp/GCApi.cpp @@ -20,6 +20,7 @@ #include "CustomLogging.hpp" #include "ExtraObjectData.hpp" #include "ExtraObjectPage.hpp" +#include "FinalizerHooks.hpp" #include "GC.hpp" #include "GCStatistics.hpp" #include "KAssert.h" @@ -49,9 +50,17 @@ bool SweepObject(uint8_t* object, FinalizerQueue& finalizerQueue, gc::GCHandle:: extraObject->ClearRegularWeakReferenceImpl(); CustomAllocDebug("SweepObject: fromExtraObject(%p) = %p", extraObject, ExtraObjectCell::fromExtraObject(extraObject)); finalizerQueue.Push(ExtraObjectCell::fromExtraObject(extraObject)); - gcHandle.addMarkedObject(); - gcHandle.addKeptObject(size); - return true; + if (HasFinalizersDataInObject(heapObjHeader->object())) { + // The object must survive until the finalizers for it are finished. + gcHandle.addMarkedObject(); + gcHandle.addKeptObject(size); + return true; + } + // The object has a finalizer, but all the data for it resides in `ExtraObjectData`. So, detach the object from it, and free it. + extraObject->UnlinkFromBaseObject(); + CustomAllocDebug("SweepObject(%p): can be reclaimed", heapObjHeader); + gcHandle.addSweptObject(); + return false; } if (!extraObject->getFlag(mm::ExtraObjectData::FLAGS_FINALIZED)) { CustomAllocDebug("SweepObject(%p): already waiting to be finalized", heapObjHeader); diff --git a/kotlin-native/runtime/src/alloc/legacy/cpp/AllocatorImplTestSupport.cpp b/kotlin-native/runtime/src/alloc/legacy/cpp/AllocatorImplTestSupport.cpp index 104f30cce83..79d1f85532e 100644 --- a/kotlin-native/runtime/src/alloc/legacy/cpp/AllocatorImplTestSupport.cpp +++ b/kotlin-native/runtime/src/alloc/legacy/cpp/AllocatorImplTestSupport.cpp @@ -51,3 +51,8 @@ std::vector alloc::test_support::allocatedObjects(mm::ThreadData& th } return objects; } + +void alloc::test_support::detachAndDestroyExtraObjectData(mm::ExtraObjectData& extraObject) noexcept { + extraObject.ClearRegularWeakReferenceImpl(); + destroyExtraObjectData(extraObject); +} \ No newline at end of file diff --git a/kotlin-native/runtime/src/alloc/legacy/cpp/ExtraObjectDataFactory.hpp b/kotlin-native/runtime/src/alloc/legacy/cpp/ExtraObjectDataFactory.hpp index ffaa9edf00a..9adf207a130 100644 --- a/kotlin-native/runtime/src/alloc/legacy/cpp/ExtraObjectDataFactory.hpp +++ b/kotlin-native/runtime/src/alloc/legacy/cpp/ExtraObjectDataFactory.hpp @@ -30,6 +30,14 @@ public: // when it's asked by GC to stop. using Producer::Publish; + void ClearForTests() noexcept { + forEachNode([](auto& extraObject) noexcept { + extraObject.ClearRegularWeakReferenceImpl(); + extraObject.Uninstall(); + }); + Producer::ClearForTests(); + } + // Do not add fields as this is just a wrapper and Producer does not have virtual destructor. }; @@ -43,7 +51,13 @@ public: // Lock registry for safe iteration. Iterable LockForIter() noexcept { return extraObjects_.LockForIter(); } - void ClearForTests() noexcept { extraObjects_.ClearForTests(); } + void ClearForTests() noexcept { + for (auto& extraObject : extraObjects_.LockForIter()) { + extraObject.ClearRegularWeakReferenceImpl(); + extraObject.Uninstall(); + } + extraObjects_.ClearForTests(); + } size_t GetSizeUnsafe() noexcept { return extraObjects_.GetSizeUnsafe(); } diff --git a/kotlin-native/runtime/src/gc/common/cpp/TracingGCTest.hpp b/kotlin-native/runtime/src/gc/common/cpp/TracingGCTest.hpp index 94ddab71e3a..563db0ea5ab 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/TracingGCTest.hpp +++ b/kotlin-native/runtime/src/gc/common/cpp/TracingGCTest.hpp @@ -1234,17 +1234,6 @@ TYPED_TEST_P(TracingGCTest, WeakResuractionInMark) { for (auto& future : mutatorFutures) { future.wait(); } - - for (int i = 0; i < kDefaultThreadCount; ++i) { - mutators[i].Execute([&, i](mm::ThreadData&, Mutator&) noexcept { - if (auto weakReferee = weaks[i]->get()) { - auto& extraObj = *mm::ExtraObjectData::Get(weakReferee); - extraObj.ClearRegularWeakReferenceImpl(); - extraObj.Uninstall(); - alloc::destroyExtraObjectData(extraObj); - } - }).wait(); - } } #define TRACING_GC_TEST_LIST \ @@ -1356,11 +1345,6 @@ TYPED_TEST_P(STWMarkGCTest, MultipleMutatorsWeakNewObj) { })(); EXPECT_NE(objectWeak.get(), nullptr); - auto& extraObj = *mm::ExtraObjectData::Get(object.header()); - extraObj.ClearRegularWeakReferenceImpl(); - extraObj.Uninstall(); - alloc::destroyExtraObjectData(extraObj); - while (!gcDone.load(std::memory_order_relaxed)) { mm::safePoint(threadData); } diff --git a/kotlin-native/runtime/src/main/cpp/FinalizerHooks.cpp b/kotlin-native/runtime/src/main/cpp/FinalizerHooks.cpp index 5925c7dd3ba..ea2740631ca 100644 --- a/kotlin-native/runtime/src/main/cpp/FinalizerHooks.cpp +++ b/kotlin-native/runtime/src/main/cpp/FinalizerHooks.cpp @@ -35,7 +35,11 @@ NO_INLINE void RunFinalizerHooksImpl(ObjHeader* object, const TypeInfo* type) no } // namespace ALWAYS_INLINE bool kotlin::HasFinalizers(ObjHeader* object) noexcept { - return object->has_meta_object() || (object->type_info()->flags_ & TF_HAS_FINALIZER) != 0; + return object->has_meta_object() || HasFinalizersDataInObject(object); +} + +ALWAYS_INLINE bool kotlin::HasFinalizersDataInObject(ObjHeader* object) noexcept { + return (object->type_info()->flags_ & TF_HAS_FINALIZER) != 0; } ALWAYS_INLINE void kotlin::RunFinalizers(ObjHeader* object) noexcept { diff --git a/kotlin-native/runtime/src/main/cpp/FinalizerHooks.hpp b/kotlin-native/runtime/src/main/cpp/FinalizerHooks.hpp index 9571efbdc0a..e0d37bba324 100644 --- a/kotlin-native/runtime/src/main/cpp/FinalizerHooks.hpp +++ b/kotlin-native/runtime/src/main/cpp/FinalizerHooks.hpp @@ -14,6 +14,7 @@ namespace kotlin { // finalizer must never try to reference them. bool HasFinalizers(ObjHeader* object) noexcept; +bool HasFinalizersDataInObject(ObjHeader* object) noexcept; void RunFinalizers(ObjHeader* object) noexcept; void SetFinalizerHookForTesting(void (*hook)(ObjHeader*)) noexcept; diff --git a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp index 144fc739129..d6dbabf9e58 100644 --- a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp +++ b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp @@ -102,6 +102,12 @@ public: deletionQueue_.clear(); } + protected: + template + void forEachNode(F&& f) noexcept(noexcept(f(std::declval()))) { + for (auto& node : queue_) f(*node); + } + private: MultiSourceQueue& owner_; // weak List queue_; diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp index 2153e48a1ab..d368b82a70f 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp @@ -40,7 +40,10 @@ mm::ExtraObjectData& mm::ExtraObjectData::Install(ObjHeader* object) noexcept { } void mm::ExtraObjectData::UnlinkFromBaseObject() noexcept { - auto *object = GetBaseObject(); + auto* object = weakReferenceOrBaseObject_.exchange(nullptr); + RuntimeAssert( + !hasPointerBits(object, WEAK_REF_TAG), "ExtraObjectData %p has uncleared weak reference %p during unlink", this, + clearPointerBits(object, WEAK_REF_TAG)); atomicSetRelease(const_cast(&object->typeInfoOrMeta_), typeInfo_); RuntimeAssert( !object->has_meta_object(), "Object %p has metaobject %p after removing metaobject %p", object, object->meta_object_or_null(), diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp index e638c89c025..c930781362c 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectDataTest.cpp @@ -10,6 +10,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "AllocatorTestSupport.hpp" #include "ObjectTestSupport.hpp" #include "ScopedThread.hpp" #include "TestSupport.hpp" @@ -51,8 +52,7 @@ TEST_F(ExtraObjectDataTest, Install) { EXPECT_FALSE(extraData.HasRegularWeakReferenceImpl()); EXPECT_THAT(extraData.GetBaseObject(), object.header()); - extraData.Uninstall(); - mm::GlobalData::Instance().threadRegistry().CurrentThreadData()->ClearForTests(); + alloc::test_support::detachAndDestroyExtraObjectData(extraData); EXPECT_FALSE(object.header()->has_meta_object()); EXPECT_THAT(object.header()->type_info(), typeInfo);