diff --git a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp index ac3c6d81630..1fa439befdc 100644 --- a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp +++ b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweep.cpp @@ -61,7 +61,6 @@ struct ProcessWeaksTraits { void gc::ConcurrentMarkAndSweep::ThreadData::SafePointAllocation(size_t size) noexcept { gcScheduler_.OnSafePointAllocation(size); - mm::SuspendIfRequested(); } void gc::ConcurrentMarkAndSweep::ThreadData::Schedule() noexcept { diff --git a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp index ee938758258..c8b2d4ab2a7 100644 --- a/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp +++ b/kotlin-native/runtime/src/gc/cms/cpp/ConcurrentMarkAndSweepTest.cpp @@ -19,6 +19,7 @@ #include "GlobalData.hpp" #include "ObjectOps.hpp" #include "ObjectTestSupport.hpp" +#include "SafePoint.hpp" #include "SingleThreadExecutor.hpp" #include "TestSupport.hpp" #include "ThreadData.hpp" @@ -736,8 +737,7 @@ TEST_P(ConcurrentMarkAndSweepTest, MultipleMutatorsCollect) { } for (int i = 1; i < kDefaultThreadCount; ++i) { - gcFutures[i] = - mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().SafePointFunctionPrologue(); }); + gcFutures[i] = mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { mm::safePoint(threadData); }); } for (auto& future : gcFutures) { @@ -870,7 +870,7 @@ TEST_P(ConcurrentMarkAndSweepTest, MultipleMutatorsAddToRootSetAfterCollectionRe for (int i = 1; i < kDefaultThreadCount; ++i) { gcFutures[i] = mutators[i].Execute([i, expandRootSet](mm::ThreadData& threadData, Mutator& mutator) { expandRootSet(threadData, mutator, i); - threadData.gc().SafePointFunctionPrologue(); + mm::safePoint(threadData); }); } @@ -933,8 +933,7 @@ TEST_P(ConcurrentMarkAndSweepTest, CrossThreadReference) { } for (int i = 1; i < kDefaultThreadCount; ++i) { - gcFutures[i] = - mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().SafePointFunctionPrologue(); }); + gcFutures[i] = mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { mm::safePoint(threadData); }); } for (auto& future : gcFutures) { @@ -998,7 +997,7 @@ TEST_P(ConcurrentMarkAndSweepTest, MultipleMutatorsWeaks) { for (int i = 1; i < kDefaultThreadCount; ++i) { gcFutures[i] = mutators[i].Execute([weak](mm::ThreadData& threadData, Mutator& mutator) { - threadData.gc().SafePointFunctionPrologue(); + mm::safePoint(threadData); EXPECT_THAT(weak->get(), nullptr); }); } @@ -1055,8 +1054,7 @@ TEST_P(ConcurrentMarkAndSweepTest, NewThreadsWhileRequestingCollection) { // All the other threads are stopping at safe points. for (int i = 1; i < kDefaultThreadCount; ++i) { - gcFutures[i] = - mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().SafePointFunctionPrologue(); }); + gcFutures[i] = mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { mm::safePoint(threadData); }); } // GC will be completed first @@ -1129,7 +1127,7 @@ TEST_P(ConcurrentMarkAndSweepTest, FreeObjectWithFreeWeakReversedOrder) { auto& weak_local = InstallWeakReference(threadData, object1.load()->header(), holder.slot()); weak = &weak_local; *holder.slot() = nullptr; - while (!done) threadData.gc().SafePointLoopBody(); + while (!done) mm::safePoint(threadData); }); f0.wait(); diff --git a/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp b/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp index c3c1c62a69b..6f19156391d 100644 --- a/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp +++ b/kotlin-native/runtime/src/gc/cms/cpp/GCImpl.cpp @@ -18,14 +18,6 @@ gc::GC::ThreadData::ThreadData(GC& gc, gcScheduler::GCSchedulerThreadData& gcSch gc::GC::ThreadData::~ThreadData() = default; -ALWAYS_INLINE void gc::GC::ThreadData::SafePointFunctionPrologue() noexcept { - mm::SuspendIfRequested(); -} - -ALWAYS_INLINE void gc::GC::ThreadData::SafePointLoopBody() noexcept { - mm::SuspendIfRequested(); -} - void gc::GC::ThreadData::Schedule() noexcept { impl_->gc().Schedule(); } diff --git a/kotlin-native/runtime/src/gc/common/cpp/GC.hpp b/kotlin-native/runtime/src/gc/common/cpp/GC.hpp index 83af9af573b..eea89c77bc3 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/GC.hpp +++ b/kotlin-native/runtime/src/gc/common/cpp/GC.hpp @@ -32,9 +32,6 @@ public: Impl& impl() noexcept { return *impl_; } - void SafePointFunctionPrologue() noexcept; - void SafePointLoopBody() noexcept; - void Schedule() noexcept; void ScheduleAndWaitFullGC() noexcept; void ScheduleAndWaitFullGCWithFinalizers() noexcept; diff --git a/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp b/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp index 6ad027b22f4..5c4a523632b 100644 --- a/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp +++ b/kotlin-native/runtime/src/gc/noop/cpp/GCImpl.cpp @@ -17,14 +17,6 @@ gc::GC::ThreadData::ThreadData(GC& gc, gcScheduler::GCSchedulerThreadData&, mm:: gc::GC::ThreadData::~ThreadData() = default; -ALWAYS_INLINE void gc::GC::ThreadData::SafePointFunctionPrologue() noexcept { - impl_->gc().SafePointFunctionPrologue(); -} - -ALWAYS_INLINE void gc::GC::ThreadData::SafePointLoopBody() noexcept { - impl_->gc().SafePointLoopBody(); -} - void gc::GC::ThreadData::Schedule() noexcept { impl_->gc().Schedule(); } diff --git a/kotlin-native/runtime/src/gc/noop/cpp/NoOpGC.hpp b/kotlin-native/runtime/src/gc/noop/cpp/NoOpGC.hpp index dba87b7137a..932a2362346 100644 --- a/kotlin-native/runtime/src/gc/noop/cpp/NoOpGC.hpp +++ b/kotlin-native/runtime/src/gc/noop/cpp/NoOpGC.hpp @@ -35,8 +35,6 @@ public: ThreadData() noexcept {} ~ThreadData() = default; - void SafePointFunctionPrologue() noexcept {} - void SafePointLoopBody() noexcept {} void SafePointAllocation(size_t size) noexcept {} void Schedule() noexcept {} diff --git a/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp b/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp index 63207dda329..69c9a23ae65 100644 --- a/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp +++ b/kotlin-native/runtime/src/gc/stms/cpp/GCImpl.cpp @@ -18,14 +18,6 @@ gc::GC::ThreadData::ThreadData(GC& gc, gcScheduler::GCSchedulerThreadData& gcSch gc::GC::ThreadData::~ThreadData() = default; -ALWAYS_INLINE void gc::GC::ThreadData::SafePointFunctionPrologue() noexcept { - mm::SuspendIfRequested(); -} - -ALWAYS_INLINE void gc::GC::ThreadData::SafePointLoopBody() noexcept { - mm::SuspendIfRequested(); -} - void gc::GC::ThreadData::Schedule() noexcept { impl_->gc().Schedule(); } diff --git a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp index cefda3da458..7debc925ae5 100644 --- a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp +++ b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweep.cpp @@ -55,7 +55,6 @@ struct ProcessWeaksTraits { void gc::SameThreadMarkAndSweep::ThreadData::SafePointAllocation(size_t size) noexcept { gcScheduler_.OnSafePointAllocation(size); - mm::SuspendIfRequested(); } void gc::SameThreadMarkAndSweep::ThreadData::Schedule() noexcept { diff --git a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp index a2ecc10a2d1..a2142084848 100644 --- a/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp +++ b/kotlin-native/runtime/src/gc/stms/cpp/SameThreadMarkAndSweepTest.cpp @@ -19,6 +19,7 @@ #include "GlobalData.hpp" #include "ObjectOps.hpp" #include "ObjectTestSupport.hpp" +#include "SafePoint.hpp" #include "SingleThreadExecutor.hpp" #include "TestSupport.hpp" #include "ThreadData.hpp" @@ -733,8 +734,7 @@ TEST_F(SameThreadMarkAndSweepTest, MultipleMutatorsCollect) { } for (int i = 1; i < kDefaultThreadCount; ++i) { - gcFutures[i] = - mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().SafePointFunctionPrologue(); }); + gcFutures[i] = mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { mm::safePoint(threadData); }); } for (auto& future : gcFutures) { @@ -868,7 +868,7 @@ TEST_F(SameThreadMarkAndSweepTest, MultipleMutatorsAddToRootSetAfterCollectionRe for (int i = 1; i < kDefaultThreadCount; ++i) { gcFutures[i] = mutators[i].Execute([i, expandRootSet](mm::ThreadData& threadData, Mutator& mutator) { expandRootSet(threadData, mutator, i); - threadData.gc().SafePointFunctionPrologue(); + mm::safePoint(threadData); }); } @@ -932,8 +932,7 @@ TEST_F(SameThreadMarkAndSweepTest, CrossThreadReference) { } for (int i = 1; i < kDefaultThreadCount; ++i) { - gcFutures[i] = - mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().SafePointFunctionPrologue(); }); + gcFutures[i] = mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { mm::safePoint(threadData); }); } for (auto& future : gcFutures) { @@ -997,7 +996,7 @@ TEST_F(SameThreadMarkAndSweepTest, MultipleMutatorsWeaks) { for (int i = 1; i < kDefaultThreadCount; ++i) { gcFutures[i] = mutators[i].Execute([weak](mm::ThreadData& threadData, Mutator& mutator) { - threadData.gc().SafePointFunctionPrologue(); + mm::safePoint(threadData); EXPECT_THAT(weak->get(), nullptr); }); } @@ -1055,8 +1054,7 @@ TEST_F(SameThreadMarkAndSweepTest, NewThreadsWhileRequestingCollection) { // All the other threads are stopping at safe points. for (int i = 1; i < kDefaultThreadCount; ++i) { - gcFutures[i] = - mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { threadData.gc().SafePointFunctionPrologue(); }); + gcFutures[i] = mutators[i].Execute([](mm::ThreadData& threadData, Mutator& mutator) { mm::safePoint(threadData); }); } // GC will be completed first @@ -1129,7 +1127,7 @@ TEST_F(SameThreadMarkAndSweepTest, FreeObjectWithFreeWeakReversedOrder) { auto& weak_local = InstallWeakReference(threadData, object1.load()->header(), holder.slot()); weak = &weak_local; *holder.slot() = nullptr; - while (!done) threadData.gc().SafePointLoopBody(); + while (!done) mm::safePoint(threadData); }); f0.wait(); diff --git a/kotlin-native/runtime/src/gcScheduler/adaptive/cpp/GCSchedulerImpl.cpp b/kotlin-native/runtime/src/gcScheduler/adaptive/cpp/GCSchedulerImpl.cpp index 1f53a9eb5e5..f39029fb6bd 100644 --- a/kotlin-native/runtime/src/gcScheduler/adaptive/cpp/GCSchedulerImpl.cpp +++ b/kotlin-native/runtime/src/gcScheduler/adaptive/cpp/GCSchedulerImpl.cpp @@ -5,6 +5,7 @@ #include "GCSchedulerImpl.hpp" +#include "CallsChecker.hpp" #include "GlobalData.hpp" #include "Memory.h" #include "Logging.hpp" @@ -12,11 +13,12 @@ using namespace kotlin; -void gcScheduler::GCSchedulerThreadData::OnSafePointRegular(size_t weight) noexcept {} - gcScheduler::GCScheduler::GCScheduler() noexcept : gcData_(std_support::make_unique>(config_, []() noexcept { - // This call acquires a lock, so we need to ensure that we're in the safe state. - NativeOrUnregisteredThreadGuard guard(/* reentrant = */ true); + // This call acquires a lock, but the lock are always short-lived, + // so we ignore thread state switching to avoid recursive safe points. + CallsCheckerIgnoreGuard guard; mm::GlobalData::Instance().gc().Schedule(); })) {} + +ALWAYS_INLINE void gcScheduler::GCScheduler::safePoint() noexcept {} diff --git a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.cpp b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.cpp index 0b95069d768..ed5393b3489 100644 --- a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.cpp +++ b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.cpp @@ -5,6 +5,7 @@ #include "GCSchedulerImpl.hpp" +#include "CallsChecker.hpp" #include "GlobalData.hpp" #include "Memory.h" #include "Logging.hpp" @@ -12,13 +13,14 @@ using namespace kotlin; -void gcScheduler::GCSchedulerThreadData::OnSafePointRegular(size_t weight) noexcept { - OnSafePointRegularImpl(weight); -} - gcScheduler::GCScheduler::GCScheduler() noexcept : gcData_(std_support::make_unique(config_, []() noexcept { - // This call acquires a lock, so we need to ensure that we're in the safe state. - NativeOrUnregisteredThreadGuard guard(/* reentrant = */ true); + // This call acquires a lock, but the lock are always short-lived, + // so we ignore thread state switching to avoid recursive safe points. + CallsCheckerIgnoreGuard guard; mm::GlobalData::Instance().gc().Schedule(); })) {} + +ALWAYS_INLINE void gcScheduler::GCScheduler::safePoint() noexcept { + static_cast(gcData()).safePoint(); +} diff --git a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.hpp b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.hpp index 7c0ad992312..9e298ace9fe 100644 --- a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.hpp +++ b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImpl.hpp @@ -12,17 +12,17 @@ #include "GCSchedulerConfig.hpp" #include "HeapGrowthController.hpp" #include "Logging.hpp" +#include "SafePoint.hpp" #include "SafePointTracker.hpp" namespace kotlin::gcScheduler::internal { +// The slowpath will trigger GC if this thread didn't meet this safepoint/allocation site before. class GCSchedulerDataAggressive : public GCSchedulerData { public: GCSchedulerDataAggressive(GCSchedulerConfig& config, std::function scheduleGC) noexcept : scheduleGC_(std::move(scheduleGC)), heapGrowthController_(config) { - // Trigger the slowpath on each safepoint and on each allocation. - // The slowpath will trigger GC if this thread didn't meet this safepoint/allocation site before. - config.threshold = 1; + // Trigger the slowpath on each allocation. config.allocationThresholdBytes = 1; RuntimeLogInfo({kTagGC}, "Aggressive GC scheduler initialized"); } @@ -34,19 +34,26 @@ public: // might be "met", so that's the only trigger to not run out of memory. RuntimeLogDebug({kTagGC}, "Scheduling GC by allocation"); scheduleGC_(); - } else if (safePointTracker_.registerCurrentSafePoint(1)) { - RuntimeLogDebug({kTagGC}, "Scheduling GC by safepoint"); - scheduleGC_(); + } else { + safePoint(); } } void OnPerformFullGC() noexcept override { heapGrowthController_.OnPerformFullGC(); } void UpdateAliveSetBytes(size_t bytes) noexcept override { heapGrowthController_.UpdateAliveSetBytes(bytes); } + void safePoint() noexcept { + if (safePointTracker_.registerCurrentSafePoint(1)) { + RuntimeLogDebug({kTagGC}, "Scheduling GC by safepoint"); + scheduleGC_(); + } + } + private: std::function scheduleGC_; HeapGrowthController heapGrowthController_; SafePointTracker<> safePointTracker_; + mm::SafePointActivator safePointActivator_; }; } // namespace kotlin::gcScheduler::internal diff --git a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImplTest.cpp b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImplTest.cpp index 7f7c17e34b5..e84dbf1a7bd 100644 --- a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImplTest.cpp +++ b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/GCSchedulerImplTest.cpp @@ -31,18 +31,15 @@ TEST(AggressiveSchedulerTest, TriggerGCOnUniqueSafePoint) { gcScheduler::GCSchedulerConfig config; gcScheduler::internal::GCSchedulerDataAggressive scheduler(config, scheduleGC.AsStdFunction()); - ASSERT_EQ(config.threshold, 1); - - gcScheduler::GCSchedulerThreadData threadSchedulerData(config, [](gcScheduler::GCSchedulerThreadData&) {}); EXPECT_CALL(scheduleGC, Call()).Times(1); for (int i = 0; i < 10; i++) { - scheduler.UpdateFromThreadData(threadSchedulerData); + scheduler.safePoint(); } testing::Mock::VerifyAndClearExpectations(&scheduleGC); EXPECT_CALL(scheduleGC, Call()).Times(1); - scheduler.UpdateFromThreadData(threadSchedulerData); + scheduler.safePoint(); testing::Mock::VerifyAndClearExpectations(&scheduleGC); }(); } diff --git a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/SafePointTracker.hpp b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/SafePointTracker.hpp index 6420af306da..2520b7490b6 100644 --- a/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/SafePointTracker.hpp +++ b/kotlin-native/runtime/src/gcScheduler/aggressive/cpp/SafePointTracker.hpp @@ -7,6 +7,7 @@ #include +#include "CallsChecker.hpp" #include "KAssert.h" #include "Logging.hpp" #include "Mutex.hpp" @@ -24,6 +25,7 @@ public: /** Returns whether the GC must be triggered on the current safe point or not. */ NO_INLINE bool registerCurrentSafePoint(size_t skipFrames) noexcept { + CallsCheckerIgnoreGuard guard; auto currentSP = SafePointID::current(skipFrames + 1); std::unique_lock lock(mutex_); diff --git a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCScheduler.hpp b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCScheduler.hpp index 8cc12d5f6ca..5855586bd5c 100644 --- a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCScheduler.hpp +++ b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCScheduler.hpp @@ -42,17 +42,11 @@ public: class GCSchedulerThreadData { public: - static constexpr size_t kFunctionPrologueWeight = 1; - static constexpr size_t kLoopBodyWeight = 1; - explicit GCSchedulerThreadData(GCSchedulerConfig& config, std::function slowPath) noexcept : config_(config), slowPath_(std::move(slowPath)) { ClearCountersAndUpdateThresholds(); } - // Should be called on encountering a safepoint. - void OnSafePointRegular(size_t weight) noexcept; - // Should be called on encountering a safepoint placed by the allocator. // TODO: Should this even be a safepoint (i.e. a place, where we suspend)? void OnSafePointAllocation(size_t size) noexcept { @@ -67,19 +61,9 @@ public: size_t allocatedBytes() const noexcept { return allocatedBytes_; } - size_t safePointsCounter() const noexcept { return safePointsCounter_; } - private: friend class test_support::GCSchedulerThreadDataTestApi; - void OnSafePointRegularImpl(size_t weight) noexcept { - safePointsCounter_ += weight; - if (safePointsCounter_ < safePointsCounterThreshold_) { - return; - } - OnSafePointSlowPath(); - } - void OnSafePointSlowPath() noexcept { slowPath_(*this); ClearCountersAndUpdateThresholds(); @@ -87,10 +71,8 @@ private: void ClearCountersAndUpdateThresholds() noexcept { allocatedBytes_ = 0; - safePointsCounter_ = 0; allocatedBytesThreshold_ = config_.allocationThresholdBytes; - safePointsCounterThreshold_ = config_.threshold; } GCSchedulerConfig& config_; @@ -98,8 +80,6 @@ private: size_t allocatedBytes_ = 0; size_t allocatedBytesThreshold_ = 0; - size_t safePointsCounter_ = 0; - size_t safePointsCounterThreshold_ = 0; }; class GCScheduler : private Pinned { @@ -113,6 +93,9 @@ public: return GCSchedulerThreadData(config_, [this](auto& threadData) { gcData_->UpdateFromThreadData(threadData); }); } + // Should be called on encountering a safepoint. + void safePoint() noexcept; + private: GCSchedulerConfig config_; std_support::unique_ptr gcData_; diff --git a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerConfig.hpp b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerConfig.hpp index 4772a634287..dccb9e7a855 100644 --- a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerConfig.hpp +++ b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerConfig.hpp @@ -13,8 +13,6 @@ namespace kotlin::gcScheduler { // NOTE: When changing default values, reflect them in GC.kt as well. struct GCSchedulerConfig { - // Only used when useGCTimer() is false. How many regular safepoints will trigger slow path. - std::atomic threshold = 100000; // How many object bytes a thread must allocate to trigger slow path. std::atomic allocationThresholdBytes = 10 * 1024; std::atomic autoTune = true; diff --git a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTest.cpp b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTest.cpp index 66aeb20a818..abec5b70a21 100644 --- a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTest.cpp +++ b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTest.cpp @@ -14,41 +14,6 @@ using namespace kotlin; using testing::_; -TEST(GCSchedulerThreadDataTest, RegularSafePoint) { - constexpr size_t kWeight = 2; - constexpr size_t kCount = 10; - constexpr size_t kThreshold = kCount * kWeight; - testing::MockFunction slowPath; - gcScheduler::GCSchedulerConfig config; - config.allocationThresholdBytes = 1; - config.threshold = kThreshold; - gcScheduler::GCSchedulerThreadData scheduler(config, slowPath.AsStdFunction()); - gcScheduler::test_support::GCSchedulerThreadDataTestApi schedulerTestApi(scheduler); - - EXPECT_CALL(slowPath, Call(_)).Times(0); - for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); - } - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), kThreshold - kWeight); - - EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), kThreshold); - }); - schedulerTestApi.OnSafePointRegularImpl(kWeight); - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); - - EXPECT_CALL(slowPath, Call(_)).Times(0); - schedulerTestApi.OnSafePointRegularImpl(kWeight); - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), kWeight); -} - TEST(GCSchedulerThreadDataTest, AllocationSafePoint) { constexpr size_t kSize = 2; constexpr size_t kCount = 10; @@ -56,7 +21,6 @@ TEST(GCSchedulerThreadDataTest, AllocationSafePoint) { testing::MockFunction slowPath; gcScheduler::GCSchedulerConfig config; config.allocationThresholdBytes = kAllocationThreshold; - config.threshold = 1; gcScheduler::GCSchedulerThreadData scheduler(config, slowPath.AsStdFunction()); gcScheduler::test_support::GCSchedulerThreadDataTestApi schedulerTestApi(scheduler); @@ -66,88 +30,62 @@ TEST(GCSchedulerThreadDataTest, AllocationSafePoint) { } testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), kAllocationThreshold - kSize); - EXPECT_THAT(scheduler.safePointsCounter(), 0); EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { EXPECT_THAT(scheduler.allocatedBytes(), kAllocationThreshold); - EXPECT_THAT(scheduler.safePointsCounter(), 0); }); scheduler.OnSafePointAllocation(kSize); testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); EXPECT_CALL(slowPath, Call(_)).Times(0); scheduler.OnSafePointAllocation(kSize); testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), kSize); - EXPECT_THAT(scheduler.safePointsCounter(), 0); } TEST(GCSchedulerThreadDataTest, ResetByGC) { - constexpr size_t kWeight = 2; constexpr size_t kSize = 2; constexpr size_t kCount = 10; - constexpr size_t kThreshold = kCount * kWeight; constexpr size_t kAllocationThreshold = kCount * kSize; testing::MockFunction slowPath; gcScheduler::GCSchedulerConfig config; config.allocationThresholdBytes = kAllocationThreshold; - config.threshold = kThreshold; gcScheduler::GCSchedulerThreadData scheduler(config, slowPath.AsStdFunction()); gcScheduler::test_support::GCSchedulerThreadDataTestApi schedulerTestApi(scheduler); EXPECT_CALL(slowPath, Call(_)).Times(0); for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); scheduler.OnSafePointAllocation(kSize); } testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), kAllocationThreshold - kSize); - EXPECT_THAT(scheduler.safePointsCounter(), kThreshold - kWeight); EXPECT_CALL(slowPath, Call(_)).Times(0); scheduler.OnStoppedForGC(); testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); } TEST(GCSchedulerThreadDataTest, UpdateThresholdsAfterResetByGC) { - constexpr size_t kWeight = 2; constexpr size_t kSize = 2; constexpr size_t kCount = 10; - constexpr size_t kThreshold = kCount * kWeight; constexpr size_t kAllocationThreshold = kCount * kSize; testing::MockFunction slowPath; gcScheduler::GCSchedulerConfig config; config.allocationThresholdBytes = kAllocationThreshold; - config.threshold = kThreshold; gcScheduler::GCSchedulerThreadData scheduler(config, slowPath.AsStdFunction()); gcScheduler::test_support::GCSchedulerThreadDataTestApi schedulerTestApi(scheduler); config.allocationThresholdBytes = kAllocationThreshold - kSize; - config.threshold = kThreshold - kWeight; EXPECT_CALL(slowPath, Call(_)).Times(0); for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); scheduler.OnSafePointAllocation(kSize); } scheduler.OnStoppedForGC(); testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); - - EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { - EXPECT_THAT(scheduler.safePointsCounter(), kThreshold - kWeight); - }); - for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); - } - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { EXPECT_THAT(scheduler.allocatedBytes(), kAllocationThreshold - kSize); @@ -157,79 +95,22 @@ TEST(GCSchedulerThreadDataTest, UpdateThresholdsAfterResetByGC) { } testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); -} - -TEST(GCSchedulerThreadDataTest, UpdateThresholdsAfterRegularSafePoint) { - constexpr size_t kWeight = 2; - constexpr size_t kSize = 2; - constexpr size_t kCount = 10; - constexpr size_t kThreshold = kCount * kWeight; - constexpr size_t kAllocationThreshold = kCount * kSize; - testing::MockFunction slowPath; - gcScheduler::GCSchedulerConfig config; - config.allocationThresholdBytes = kAllocationThreshold; - config.threshold = kThreshold; - gcScheduler::GCSchedulerThreadData scheduler(config, slowPath.AsStdFunction()); - gcScheduler::test_support::GCSchedulerThreadDataTestApi schedulerTestApi(scheduler); - - config.allocationThresholdBytes = kAllocationThreshold - kSize; - config.threshold = kThreshold - kWeight; - - EXPECT_CALL(slowPath, Call(_)).Times(0); - for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); - scheduler.OnSafePointAllocation(kSize); - } - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { - EXPECT_THAT(scheduler.safePointsCounter(), kThreshold); - }); - schedulerTestApi.OnSafePointRegularImpl(kWeight); - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); - - EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { - EXPECT_THAT(scheduler.safePointsCounter(), kThreshold - kWeight); - }); - for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); - } - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); - - EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { - EXPECT_THAT(scheduler.allocatedBytes(), kAllocationThreshold - kSize); - }); - for (size_t i = 0; i < kCount - 1; ++i) { - scheduler.OnSafePointAllocation(kSize); - } - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); } TEST(GCSchedulerThreadDataTest, UpdateThresholdsAfterAllocationSafePoint) { - constexpr size_t kWeight = 2; constexpr size_t kSize = 2; constexpr size_t kCount = 10; - constexpr size_t kThreshold = kCount * kWeight; constexpr size_t kAllocationThreshold = kCount * kSize; testing::MockFunction slowPath; gcScheduler::GCSchedulerConfig config; config.allocationThresholdBytes = kAllocationThreshold; - config.threshold = kThreshold; gcScheduler::GCSchedulerThreadData scheduler(config, slowPath.AsStdFunction()); gcScheduler::test_support::GCSchedulerThreadDataTestApi schedulerTestApi(scheduler); config.allocationThresholdBytes = kAllocationThreshold - kSize; - config.threshold = kThreshold - kWeight; EXPECT_CALL(slowPath, Call(_)).Times(0); for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); scheduler.OnSafePointAllocation(kSize); } testing::Mock::VerifyAndClearExpectations(&slowPath); @@ -239,17 +120,6 @@ TEST(GCSchedulerThreadDataTest, UpdateThresholdsAfterAllocationSafePoint) { scheduler.OnSafePointAllocation(kSize); testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); - - EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { - EXPECT_THAT(scheduler.safePointsCounter(), kThreshold - kWeight); - }); - for (size_t i = 0; i < kCount - 1; ++i) { - schedulerTestApi.OnSafePointRegularImpl(kWeight); - } - testing::Mock::VerifyAndClearExpectations(&slowPath); - EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); EXPECT_CALL(slowPath, Call(testing::Ref(scheduler))).WillOnce([&](gcScheduler::GCSchedulerThreadData& scheduler) { EXPECT_THAT(scheduler.allocatedBytes(), kAllocationThreshold - kSize); @@ -259,5 +129,4 @@ TEST(GCSchedulerThreadDataTest, UpdateThresholdsAfterAllocationSafePoint) { } testing::Mock::VerifyAndClearExpectations(&slowPath); EXPECT_THAT(scheduler.allocatedBytes(), 0); - EXPECT_THAT(scheduler.safePointsCounter(), 0); } diff --git a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTestSupport.hpp b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTestSupport.hpp index 33f545f4b21..25fe6fa00af 100644 --- a/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTestSupport.hpp +++ b/kotlin-native/runtime/src/gcScheduler/common/cpp/GCSchedulerTestSupport.hpp @@ -13,8 +13,6 @@ class GCSchedulerThreadDataTestApi : private Pinned { public: explicit GCSchedulerThreadDataTestApi(GCSchedulerThreadData& scheduler) : scheduler_(scheduler) {} - void OnSafePointRegularImpl(size_t weight) { scheduler_.OnSafePointRegularImpl(weight); } - void SetAllocatedBytes(size_t bytes) { scheduler_.allocatedBytes_ = bytes; } private: diff --git a/kotlin-native/runtime/src/gcScheduler/manual/cpp/GCSchedulerImpl.cpp b/kotlin-native/runtime/src/gcScheduler/manual/cpp/GCSchedulerImpl.cpp index 4dac174fef7..fd34aff9283 100644 --- a/kotlin-native/runtime/src/gcScheduler/manual/cpp/GCSchedulerImpl.cpp +++ b/kotlin-native/runtime/src/gcScheduler/manual/cpp/GCSchedulerImpl.cpp @@ -7,6 +7,6 @@ using namespace kotlin; -void gcScheduler::GCSchedulerThreadData::OnSafePointRegular(size_t weight) noexcept {} - gcScheduler::GCScheduler::GCScheduler() noexcept : gcData_(std_support::make_unique()) {} + +ALWAYS_INLINE void gcScheduler::GCScheduler::safePoint() noexcept {} diff --git a/kotlin-native/runtime/src/main/cpp/CallsChecker.hpp b/kotlin-native/runtime/src/main/cpp/CallsChecker.hpp index 59e4fb1d054..e0f84d5798f 100644 --- a/kotlin-native/runtime/src/main/cpp/CallsChecker.hpp +++ b/kotlin-native/runtime/src/main/cpp/CallsChecker.hpp @@ -3,6 +3,8 @@ * that can be found in the LICENSE file. */ +#pragma once + #include "Common.h" #include "Utils.hpp" diff --git a/kotlin-native/runtime/src/main/cpp/KAssert.cpp b/kotlin-native/runtime/src/main/cpp/KAssert.cpp index b1a8639f2e1..59e026281e4 100644 --- a/kotlin-native/runtime/src/main/cpp/KAssert.cpp +++ b/kotlin-native/runtime/src/main/cpp/KAssert.cpp @@ -9,6 +9,7 @@ #include #include "std_support/Span.hpp" +#include "CallsChecker.hpp" #include "Format.h" #include "Porting.h" #include "StackTrace.hpp" @@ -20,6 +21,8 @@ namespace { THREAD_LOCAL_VARIABLE bool assertionReportInProgress = false; void PrintAssert(bool allowStacktrace, const char* location, const char* format, std::va_list args) noexcept { + CallsCheckerIgnoreGuard ignoreCallsChecker; + if (assertionReportInProgress) { // WARNING: avoid anything that can assert ar panic here konan::consoleErrorf("An attempt to report an assertion lead to another failure:\n"); diff --git a/kotlin-native/runtime/src/main/cpp/StackTrace.cpp b/kotlin-native/runtime/src/main/cpp/StackTrace.cpp index cc3f0d73c2b..afc4a06f5c9 100644 --- a/kotlin-native/runtime/src/main/cpp/StackTrace.cpp +++ b/kotlin-native/runtime/src/main/cpp/StackTrace.cpp @@ -129,7 +129,6 @@ int getSourceInfo(void* symbol, SourceInfo *result, int result_len) { // TODO: this implementation is just a hack, e.g. the result is inexact; // however it is better to have an inexact stacktrace than not to have any. NO_INLINE std_support::vector kotlin::internal::GetCurrentStackTrace(size_t skipFrames) noexcept { - NativeOrUnregisteredThreadGuard guard(true); #if KONAN_NO_BACKTRACE return {}; #else @@ -174,7 +173,6 @@ NO_INLINE std_support::vector kotlin::internal::GetCurrentStackTrace(size // TODO: this implementation is just a hack, e.g. the result is inexact; // however it is better to have an inexact stacktrace than not to have any. NO_INLINE size_t kotlin::internal::GetCurrentStackTrace(size_t skipFrames, std_support::span buffer) noexcept { - NativeOrUnregisteredThreadGuard guard(true); #if KONAN_NO_BACKTRACE return {}; #else @@ -276,7 +274,6 @@ KNativePtr adjustAddressForSourceInfo(KNativePtr address) { return address; } #endif std_support::vector kotlin::GetStackTraceStrings(std_support::span stackTrace) noexcept { - NativeOrUnregisteredThreadGuard guard(true); #if KONAN_NO_BACKTRACE std_support::vector strings; strings.push_back(""); diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/GC.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/GC.kt index bee4c6bb1bf..b1ecd7cb15d 100644 --- a/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/GC.kt +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/GC.kt @@ -98,17 +98,17 @@ object GC { external fun start() /** - * GC threshold, controlling how frequenly GC is activated, and how much time GC + * Deprecated and unused. + * + * Legacy MM: GC threshold, controlling how frequenly GC is activated, and how much time GC * takes. Bigger values lead to longer GC pauses, but less GCs. - * Usually unused. For the on-safepoints GC scheduler counts - * how many safepoints must the code pass before informing the GC scheduler. * - * Default: (old MM) 8 * 1024 - * - * Default: (new MM) 100000 + * Default: 8 * 1024 * * @throws [IllegalArgumentException] when value is not positive. */ + @Suppress("DEPRECATION") + @Deprecated("No-op in modern GC implementation") var threshold: Int by kotlin.native.runtime.GC::threshold /** diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/native/runtime/GC.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/native/runtime/GC.kt index a40f3bdb2ac..4bd61fbed32 100644 --- a/kotlin-native/runtime/src/main/kotlin/kotlin/native/runtime/GC.kt +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/native/runtime/GC.kt @@ -99,17 +99,16 @@ public object GC { external fun start() /** - * GC threshold, controlling how frequenly GC is activated, and how much time GC + * Deprecated and unused. + * + * Legacy MM: GC threshold, controlling how frequenly GC is activated, and how much time GC * takes. Bigger values lead to longer GC pauses, but less GCs. - * Usually unused. For the on-safepoints GC scheduler counts - * how many safepoints must the code pass before informing the GC scheduler. * - * Default: (old MM) 8 * 1024 - * - * Default: (new MM) 100000 + * Default: 8 * 1024 * * @throws [IllegalArgumentException] when value is not positive. */ + @Deprecated("No-op in modern GC implementation") var threshold: Int get() = getThreshold() set(value) { diff --git a/kotlin-native/runtime/src/mm/cpp/Memory.cpp b/kotlin-native/runtime/src/mm/cpp/Memory.cpp index 584e5f7d589..89a7524de25 100644 --- a/kotlin-native/runtime/src/mm/cpp/Memory.cpp +++ b/kotlin-native/runtime/src/mm/cpp/Memory.cpp @@ -16,6 +16,7 @@ #include "ObjectOps.hpp" #include "Porting.h" #include "Runtime.h" +#include "SafePoint.hpp" #include "StableRef.hpp" #include "ThreadData.hpp" #include "ThreadRegistry.hpp" @@ -342,12 +343,14 @@ extern "C" void Kotlin_native_internal_GC_start(ObjHeader*) { } extern "C" void Kotlin_native_internal_GC_setThreshold(ObjHeader*, KInt value) { - RuntimeAssert(value > 0, "Must be handled by the caller"); - mm::GlobalData::Instance().gcScheduler().config().threshold = value; + // TODO: Remove when legacy MM is gone. + // Nothing to do } extern "C" KInt Kotlin_native_internal_GC_getThreshold(ObjHeader*) { - return mm::GlobalData::Instance().gcScheduler().config().threshold.load(); + // TODO: Remove when legacy MM is gone. + // Nothing to do + return 0; } extern "C" void Kotlin_native_internal_GC_setCollectCyclesThreshold(ObjHeader*, int64_t value) { @@ -541,17 +544,11 @@ extern "C" void CheckGlobalsAccessible() { // it would be inlined manually in RemoveRedundantSafepointsPass extern "C" RUNTIME_NOTHROW NO_INLINE void Kotlin_mm_safePointFunctionPrologue() { - auto* threadData = mm::ThreadRegistry::Instance().CurrentThreadData(); - AssertThreadState(threadData, ThreadState::kRunnable); - threadData->gcScheduler().OnSafePointRegular(gcScheduler::GCSchedulerThreadData::kFunctionPrologueWeight); - threadData->gc().SafePointFunctionPrologue(); + mm::safePoint(); } extern "C" RUNTIME_NOTHROW CODEGEN_INLINE_POLICY void Kotlin_mm_safePointWhileLoopBody() { - auto* threadData = mm::ThreadRegistry::Instance().CurrentThreadData(); - AssertThreadState(threadData, ThreadState::kRunnable); - threadData->gcScheduler().OnSafePointRegular(gcScheduler::GCSchedulerThreadData::kLoopBodyWeight); - threadData->gc().SafePointLoopBody(); + mm::safePoint(); } extern "C" CODEGEN_INLINE_POLICY RUNTIME_NOTHROW void Kotlin_mm_switchThreadStateNative() { diff --git a/kotlin-native/runtime/src/mm/cpp/SafePoint.cpp b/kotlin-native/runtime/src/mm/cpp/SafePoint.cpp new file mode 100644 index 00000000000..c843ef0aebf --- /dev/null +++ b/kotlin-native/runtime/src/mm/cpp/SafePoint.cpp @@ -0,0 +1,94 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the LICENSE file. + */ + +#include "SafePoint.hpp" + +#include + +#include "GCScheduler.hpp" +#include "KAssert.h" +#include "ThreadData.hpp" +#include "ThreadState.hpp" + +using namespace kotlin; + +namespace { + +[[clang::no_destroy]] std::mutex safePointActionMutex; +int64_t activeCount = 0; +std::atomic safePointAction = nullptr; + +void safePointActionImpl(mm::ThreadData& threadData) noexcept { + static thread_local bool recursion = false; + RuntimeAssert(!recursion, "Recursive safepoint"); + AutoReset guard(&recursion, true); + + mm::GlobalData::Instance().gcScheduler().safePoint(); + threadData.suspensionData().suspendIfRequested(); +} + +ALWAYS_INLINE void slowPathImpl(mm::ThreadData& threadData) noexcept { + // reread an action to avoid register pollution outside the function + auto action = safePointAction.load(std::memory_order_seq_cst); + if (action != nullptr) { + action(threadData); + } +} + +NO_INLINE void slowPath() noexcept { + slowPathImpl(*mm::ThreadRegistry::Instance().CurrentThreadData()); +} + +NO_INLINE void slowPath(mm::ThreadData& threadData) noexcept { + slowPathImpl(threadData); +} + +void incrementActiveCount() noexcept { + std::unique_lock guard{safePointActionMutex}; + ++activeCount; + RuntimeAssert(activeCount >= 1, "Unexpected activeCount: %" PRId64, activeCount); + if (activeCount == 1) { + auto prev = safePointAction.exchange(safePointActionImpl, std::memory_order_seq_cst); + RuntimeAssert(prev == nullptr, "Action cannot have been set. Was %p", prev); + } +} + +void decrementActiveCount() noexcept { + std::unique_lock guard{safePointActionMutex}; + --activeCount; + RuntimeAssert(activeCount >= 0, "Unexpected activeCount: %" PRId64, activeCount); + if (activeCount == 0) { + auto prev = safePointAction.exchange(nullptr, std::memory_order_seq_cst); + RuntimeAssert(prev == safePointActionImpl, "Action must have been %p. Was %p", safePointActionImpl, prev); + } +} + +} // namespace + +mm::SafePointActivator::SafePointActivator() noexcept : active_(true) { + incrementActiveCount(); +} + +mm::SafePointActivator::~SafePointActivator() { + if (active_) { + decrementActiveCount(); + } +} + +ALWAYS_INLINE void mm::safePoint() noexcept { + AssertThreadState(ThreadState::kRunnable); + auto action = safePointAction.load(std::memory_order_relaxed); + if (__builtin_expect(action != nullptr, false)) { + slowPath(); + } +} + +ALWAYS_INLINE void mm::safePoint(mm::ThreadData& threadData) noexcept { + AssertThreadState(&threadData, ThreadState::kRunnable); + auto action = safePointAction.load(std::memory_order_relaxed); + if (__builtin_expect(action != nullptr, false)) { + slowPath(threadData); + } +} diff --git a/kotlin-native/runtime/src/mm/cpp/SafePoint.hpp b/kotlin-native/runtime/src/mm/cpp/SafePoint.hpp new file mode 100644 index 00000000000..d8d883bc456 --- /dev/null +++ b/kotlin-native/runtime/src/mm/cpp/SafePoint.hpp @@ -0,0 +1,38 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the LICENSE file. + */ + +#pragma once + +#include + +#include "Utils.hpp" + +namespace kotlin::mm { + +class ThreadData; + +class SafePointActivator : private MoveOnly { +public: + SafePointActivator() noexcept; + ~SafePointActivator(); + + SafePointActivator(SafePointActivator&& rhs) noexcept : active_(rhs.active_) { rhs.active_ = false; } + + SafePointActivator& operator=(SafePointActivator&& rhs) noexcept { + SafePointActivator other(std::move(rhs)); + swap(other); + return *this; + } + + void swap(SafePointActivator& rhs) noexcept { std::swap(active_, rhs.active_); } + +private: + bool active_; +}; + +void safePoint() noexcept; +void safePoint(ThreadData& threadData) noexcept; + +} // namespace kotlin::mm diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp index bba307da4a2..af1faf84903 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.cpp @@ -12,21 +12,17 @@ #include "CallsChecker.hpp" #include "Logging.hpp" +#include "SafePoint.hpp" #include "StackTrace.hpp" -namespace { +using namespace kotlin; -bool isSuspendedOrNative(kotlin::mm::ThreadData& thread) noexcept { - auto& suspensionData = thread.suspensionData(); - return suspensionData.suspended() || suspensionData.state() == kotlin::ThreadState::kNative; -} +namespace { template bool allThreads(F predicate) noexcept { auto& threadRegistry = kotlin::mm::ThreadRegistry::Instance(); - auto* currentThread = (threadRegistry.IsCurrentThreadRegistered()) - ? threadRegistry.CurrentThreadData() - : nullptr; + auto* currentThread = (threadRegistry.IsCurrentThreadRegistered()) ? threadRegistry.CurrentThreadData() : nullptr; kotlin::mm::ThreadRegistry::Iterable threads = kotlin::mm::ThreadRegistry::Instance().LockForIter(); for (auto& thread : threads) { // Handle if suspension was initiated by the mutator thread. @@ -43,7 +39,7 @@ void yield() noexcept { std::this_thread::yield(); } -THREAD_LOCAL_VARIABLE bool gSuspensionRequestedByCurrentThread = false; +[[clang::no_destroy]] thread_local std::optional gSafePointActivator = std::nullopt; [[clang::no_destroy]] std::mutex gSuspensionMutex; [[clang::no_destroy]] std::condition_variable gSuspensionCondVar; @@ -51,9 +47,16 @@ THREAD_LOCAL_VARIABLE bool gSuspensionRequestedByCurrentThread = false; std::atomic kotlin::mm::internal::gSuspensionRequested = false; -void kotlin::mm::ThreadSuspensionData::suspendIfRequestedSlowPath() noexcept { - CallsCheckerIgnoreGuard guard; +kotlin::ThreadState kotlin::mm::ThreadSuspensionData::setState(kotlin::ThreadState newState) noexcept { + ThreadState oldState = state_.exchange(newState); + if (oldState == ThreadState::kNative && newState == ThreadState::kRunnable) { + // must use already acquired ThreadData because TLS may be in invalid state e.g. during thread detach + safePoint(threadData_); + } + return oldState; +} +NO_EXTERNAL_CALLS_CHECK void kotlin::mm::ThreadSuspensionData::suspendIfRequested() noexcept { if (IsThreadSuspensionRequested()) { threadData_.gc().OnSuspendForGC(); std::unique_lock lock(gSuspensionMutex); @@ -71,46 +74,38 @@ void kotlin::mm::ThreadSuspensionData::suspendIfRequestedSlowPath() noexcept { bool kotlin::mm::RequestThreadsSuspension() noexcept { CallsCheckerIgnoreGuard guard; - RuntimeAssert(gSuspensionRequestedByCurrentThread == false, "Current thread already suspended threads."); + RuntimeAssert(gSafePointActivator == std::nullopt, "Current thread already suspended threads."); { std::unique_lock lock(gSuspensionMutex); - bool actual = false; - internal::gSuspensionRequested.compare_exchange_strong(actual, true); - if (actual) { + // Someone else has already suspended threads. + if (internal::gSuspensionRequested.load(std::memory_order_relaxed)) { return false; } + gSafePointActivator = mm::SafePointActivator(); + internal::gSuspensionRequested.store(true); } - gSuspensionRequestedByCurrentThread = true; return true; } void kotlin::mm::WaitForThreadsSuspension() noexcept { // Spin waiting for threads to suspend. Ignore Native threads. - while(!allThreads(isSuspendedOrNative)) { + while (!allThreads([](mm::ThreadData& thread) { return thread.suspensionData().suspendedOrNative(); })) { yield(); } } -NO_INLINE void kotlin::mm::SuspendIfRequestedSlowPath() noexcept { - mm::ThreadRegistry::Instance().CurrentThreadData()->suspensionData().suspendIfRequestedSlowPath(); -} - -ALWAYS_INLINE void kotlin::mm::SuspendIfRequested() noexcept { - if (IsThreadSuspensionRequested()) { - SuspendIfRequestedSlowPath(); - } -} - void kotlin::mm::ResumeThreads() noexcept { + RuntimeAssert(gSafePointActivator != std::nullopt, "Current thread must have suspended threads"); + gSafePointActivator = std::nullopt; + // From the std::condition_variable docs: // Even if the shared variable is atomic, it must be modified under // the mutex in order to correctly publish the modification to the waiting thread. // https://en.cppreference.com/w/cpp/thread/condition_variable { std::unique_lock lock(gSuspensionMutex); - internal::gSuspensionRequested = false; + internal::gSuspensionRequested.store(false); } - gSuspensionRequestedByCurrentThread = false; gSuspensionCondVar.notify_all(); } diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp index c40036c61ae..375aa35ada6 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadSuspension.hpp @@ -34,35 +34,21 @@ public: ThreadState state() noexcept { return state_; } - ThreadState setState(ThreadState newState) noexcept { - ThreadState oldState = state_.exchange(newState); - if (oldState == ThreadState::kNative && newState == ThreadState::kRunnable) { - suspendIfRequested(); - } - return oldState; - } + ThreadState setState(ThreadState newState) noexcept; bool suspended() noexcept { return suspended_; } + bool suspendedOrNative() noexcept { return suspended() || state() == kotlin::ThreadState::kNative; } - void suspendIfRequested() noexcept { - if (IsThreadSuspensionRequested()) { - suspendIfRequestedSlowPath(); - } - } + void suspendIfRequested() noexcept; private: - friend void SuspendIfRequestedSlowPath() noexcept; - std::atomic state_; mm::ThreadData& threadData_; std::atomic suspended_; - void suspendIfRequestedSlowPath() noexcept; }; bool RequestThreadsSuspension() noexcept; void WaitForThreadsSuspension() noexcept; -void SuspendIfRequestedSlowPath() noexcept; -void SuspendIfRequested() noexcept; /** * Suspends all threads registered in ThreadRegistry except threads that are in the Native state. diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp index 800d2e8f648..3f60417065e 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadSuspensionTest.cpp @@ -6,6 +6,7 @@ #include "MemoryPrivate.hpp" #include "Runtime.h" #include "RuntimePrivate.hpp" +#include "SafePoint.hpp" #include "ScopedThread.hpp" #include "ThreadSuspension.hpp" #include "ThreadState.hpp" @@ -228,7 +229,7 @@ TEST_F(ThreadSuspensionTest, FileInitializationWithSuspend) { EXPECT_EQ(GetThreadState(), ThreadState::kRunnable); // Give other threads a chance to call CallInitGlobalPossiblyLock. std::this_thread::yield(); - mm::SuspendIfRequested(); + mm::safePoint(); }); for (size_t i = 0; i < kThreadCount; i++) { @@ -241,7 +242,7 @@ TEST_F(ThreadSuspensionTest, FileInitializationWithSuspend) { CallInitGlobalPossiblyLock(&lock, initializationFunction); // Try to suspend to handle a case when this thread doesn't call the initialization function. - mm::SuspendIfRequested(); + mm::safePoint(); }); } waitUntilThreadsAreReady();