From 4b92d2e76cb1b4f7dc95f03c83ffb0da04ac649a Mon Sep 17 00:00:00 2001 From: Johan Bay Date: Fri, 16 Sep 2022 14:43:54 +0000 Subject: [PATCH] Add `OnSuspendForGC` hook to GCImpl::ThreadData Co-authored-by: Johan Bay Merge-request: KOTLIN-MR-506 Merged-by: Alexander Shabalin --- .../runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp | 5 +---- .../runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.hpp | 2 +- .../runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp | 2 +- kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp | 4 ++++ kotlin-native/runtime/src/gc/common/cpp/GC.hpp | 1 + kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp | 2 ++ kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp | 2 ++ kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp | 8 +------- kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp | 5 ----- 9 files changed, 13 insertions(+), 18 deletions(-) diff --git a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp index 69c0e7b3190..818697e4414 100644 --- a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp +++ b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp @@ -95,7 +95,7 @@ void gc::ConcurrentMarkAndSweep::ThreadData::OnOOM(size_t size) noexcept { ScheduleAndWaitFullGC(); } -NO_EXTERNAL_CALLS_CHECK void gc::ConcurrentMarkAndSweep::ThreadData::PublishAndMark() noexcept { +NO_EXTERNAL_CALLS_CHECK void gc::ConcurrentMarkAndSweep::ThreadData::OnSuspendForGC() noexcept { std::unique_lock lock(markingMutex); if (!markingRequested.load()) return; @@ -133,9 +133,6 @@ gc::ConcurrentMarkAndSweep::ConcurrentMarkAndSweep( } }); markingBehavior_ = kotlin::compiler::gcMarkSingleThreaded() ? MarkingBehavior::kDoNotMark : MarkingBehavior::kMarkOwnStack; - mm::SetOnSuspendCallback([](mm::ThreadData& thread) { - thread.gc().impl().gc().PublishAndMark(); - }); RuntimeLogDebug({kTagGC}, "Concurrent Mark & Sweep GC initialized"); } diff --git a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.hpp b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.hpp index 3bc125859df..24aaaa577da 100644 --- a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.hpp +++ b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.hpp @@ -93,7 +93,7 @@ public: void OnOOM(size_t size) noexcept; - void PublishAndMark() noexcept; + void OnSuspendForGC() noexcept; Allocator CreateAllocator() noexcept { return Allocator(gc::Allocator(), *this); } diff --git a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp index f417222ec60..3c7becdb4ef 100644 --- a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp +++ b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp @@ -1176,7 +1176,7 @@ TEST_P(ConcurrentMarkAndSweepTest, MutatorsCanMarkOwnLocals) { for (int i = 0; i < kDefaultThreadCount; ++i) { gcFutures[i] = mutators[i] - .Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().impl().gc().PublishAndMark(); }); + .Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().impl().gc().OnSuspendForGC(); }); } if (GetParam() == gc::ConcurrentMarkAndSweep::kMarkOwnStack) { diff --git a/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp b/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp index e10855a52eb..f7b57dbe435 100644 --- a/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp +++ b/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp @@ -60,6 +60,10 @@ void gc::GC::ThreadData::OnStoppedForGC() noexcept { impl_->gcScheduler().OnStoppedForGC(); } +void gc::GC::ThreadData::OnSuspendForGC() noexcept { + impl_->gc().OnSuspendForGC(); +} + gc::GC::GC() noexcept : impl_(std_support::make_unique()) {} gc::GC::~GC() = default; diff --git a/kotlin-native/runtime/src/gc/common/cpp/GC.hpp b/kotlin-native/runtime/src/gc/common/cpp/GC.hpp index 199d91e6e53..5631df4a965 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/GC.hpp +++ b/kotlin-native/runtime/src/gc/common/cpp/GC.hpp @@ -45,6 +45,7 @@ public: ArrayHeader* CreateArray(const TypeInfo* typeInfo, uint32_t elements) noexcept; void OnStoppedForGC() noexcept; + void OnSuspendForGC() noexcept; private: std_support::unique_ptr impl_; diff --git a/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp b/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp index 5c3743fe514..181ba39354a 100644 --- a/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp +++ b/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp @@ -50,6 +50,8 @@ void gc::GC::ThreadData::OnStoppedForGC() noexcept { impl_->gcScheduler().OnStoppedForGC(); } +void gc::GC::ThreadData::OnSuspendForGC() noexcept { } + gc::GC::GC() noexcept : impl_(std_support::make_unique()) {} gc::GC::~GC() = default; diff --git a/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp b/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp index e05b77614a2..c80c1cb1ab9 100644 --- a/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp +++ b/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp @@ -62,6 +62,8 @@ void gc::GC::ThreadData::OnStoppedForGC() noexcept { impl_->gcScheduler().OnStoppedForGC(); } +void gc::GC::ThreadData::OnSuspendForGC() noexcept { } + gc::GC::GC() noexcept : impl_(std_support::make_unique()) {} gc::GC::~GC() = default; diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp index 559d8334da0..d87fe7b0759 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp @@ -45,7 +45,6 @@ void yield() noexcept { THREAD_LOCAL_VARIABLE bool gSuspensionRequestedByCurrentThread = false; [[clang::no_destroy]] std::mutex gSuspensionMutex; [[clang::no_destroy]] std::condition_variable gSuspensionCondVar; -[[clang::no_destroy]] std::function gPreSuspend = nullptr; } // namespace @@ -53,8 +52,7 @@ std::atomic kotlin::mm::internal::gSuspensionRequested = false; NO_EXTERNAL_CALLS_CHECK void kotlin::mm::ThreadSuspensionData::suspendIfRequestedSlowPath() noexcept { if (IsThreadSuspensionRequested()) { - if (gPreSuspend) - gPreSuspend(threadData_); + threadData_.gc().OnSuspendForGC(); std::unique_lock lock(gSuspensionMutex); auto threadId = konan::currentThreadId(); auto suspendStartMs = konan::getTimeMicros(); @@ -99,10 +97,6 @@ ALWAYS_INLINE void kotlin::mm::SuspendIfRequested() noexcept { } } -void kotlin::mm::SetOnSuspendCallback(std::function preSuspend) noexcept { - gPreSuspend = preSuspend; -} - void kotlin::mm::ResumeThreads() noexcept { // From the std::condition_variable docs: // Even if the shared variable is atomic, it must be modified under diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp index 20823699b6b..fe2c8af93d0 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp @@ -78,11 +78,6 @@ inline bool SuspendThreads() noexcept { return true; } -/** - * Set function `preSuspend` which will be called in threads before suspending. - */ -void SetOnSuspendCallback(std::function) noexcept; - /** * Resumes all threads registered in ThreadRegistry that were suspended by the SuspendThreads call. * Does not wait until all such threads are actually resumed.