From 78d179e9ab4cd3a30fede0a87080f9dc8619efa9 Mon Sep 17 00:00:00 2001 From: Ilya Matveev Date: Thu, 24 Mar 2022 14:12:10 +0700 Subject: [PATCH] [K/N][runtime] Track safepoints in aggressive GC Issue #KT-49188 Fixed --- .../tests/runtime/basic/cleaner_basic.kt | 8 +- .../src/gc/common/cpp/GCSchedulerImpl.hpp | 61 ++++++++- .../src/gc/common/cpp/GCSchedulerTest.cpp | 126 ++++++++++++++++++ 3 files changed, 186 insertions(+), 9 deletions(-) diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt index 9122240460a..26d9140c070 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt @@ -222,8 +222,12 @@ fun testCleanerCleansWithoutGC() { waitCleanerWorker() assertTrue(called.value) - // If this fails, GC has somehow ran on the cleaners worker. - assertNotNull(funBoxWeak!!.value) + + // Only for legacy MM. + if (Platform.memoryModel != MemoryModel.EXPERIMENTAL) { + // If this fails, GC has somehow ran on the cleaners worker. + assertNotNull(funBoxWeak!!.value) + } } val globalInt = AtomicInt(0) diff --git a/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerImpl.hpp b/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerImpl.hpp index 36c66cc0e03..b7ec2817329 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerImpl.hpp +++ b/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerImpl.hpp @@ -8,6 +8,8 @@ #include "GCScheduler.hpp" #include "Clock.hpp" +#include "StackTrace.hpp" +#include "std_support/UnorderedSet.hpp" #ifndef KONAN_NO_THREADS #include "RepeatedTimer.hpp" @@ -79,6 +81,41 @@ private: std::atomic lastGC_; }; +template +class SafePointTracker { +public: + using SafePointID = kotlin::StackTrace; + + explicit SafePointTracker(size_t maxSize = 100000) : maxSize_(maxSize) {} + + /** Returns whether the GC must be triggered on the current safe point or not. */ + NO_INLINE bool registerCurrentSafePoint(size_t skipFrames) noexcept { + auto currentSP = SafePointID::current(skipFrames + 1); + + std::unique_lock lock(mutex_); + + // TODO: Consider replacing this naive cleaning with an LRU cache. + if (metSafePoints_.size() >= maxSize()) { + RuntimeLogDebug({kTagGC}, "Clear safe point tracker set since it exceeded maximal size"); + metSafePoints_.clear(); + } + + bool inserted = metSafePoints_.insert(currentSP).second; + return inserted; + } + + size_t maxSize() { return maxSize_; } + + size_t size() { return metSafePoints_.size(); } + +private: + size_t maxSize_; + + // TODO: Consider replacing mutex + global set with thread local sets sychronized on STW. + std::mutex mutex_; + std_support::unordered_set metSafePoints_; +}; + class GCEmptySchedulerData : public gc::GCSchedulerData { void UpdateFromThreadData(gc::GCSchedulerThreadData& threadData) noexcept override {} void OnPerformFullGC() noexcept override {} @@ -157,19 +194,29 @@ private: class GCSchedulerDataAggressive : public gc::GCSchedulerData { public: GCSchedulerDataAggressive(gc::GCSchedulerConfig& config, std::function scheduleGC) noexcept : - scheduleGC_(std::move(scheduleGC)) { - // TODO: Make it even more aggressive and run on a subset of backend.native tests. - config.threshold = 1000; - config.allocationThresholdBytes = 10000; + 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; + config.allocationThresholdBytes = 1; } - void UpdateFromThreadData(gc::GCSchedulerThreadData& threadData) noexcept override { scheduleGC_(); } + void UpdateFromThreadData(gc::GCSchedulerThreadData& threadData) noexcept override { + heapGrowthController_.OnAllocated(threadData.allocatedBytes()); + if (heapGrowthController_.NeedsGC()) { + scheduleGC_(); + } else if (safePointTracker_.registerCurrentSafePoint(1)) { + scheduleGC_(); + } + } - void OnPerformFullGC() noexcept override {} - void UpdateAliveSetBytes(size_t bytes) noexcept override {} + void OnPerformFullGC() noexcept override { heapGrowthController_.OnPerformFullGC(); } + void UpdateAliveSetBytes(size_t bytes) noexcept override { heapGrowthController_.UpdateAliveSetBytes(bytes); } private: std::function scheduleGC_; + HeapGrowthController heapGrowthController_; + SafePointTracker<> safePointTracker_; }; } // namespace kotlin::gc::internal diff --git a/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerTest.cpp b/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerTest.cpp index d43b8d9902f..2880a8f2192 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerTest.cpp +++ b/kotlin-native/runtime/src/gc/common/cpp/GCSchedulerTest.cpp @@ -710,5 +710,131 @@ TEST_F(GCSchedulerDataWithTimerTest, TuneTargetHeap) { EXPECT_THAT(config.targetHeapBytes.load(), 5); } +// These tests require a stack trace to contain call site addresses but +// on Windows a trace contains function addresses instead. +// So skip these tests on Windows. +#if (__MINGW32__ || __MINGW64__) +#define SKIP_ON_WINDOWS() do { GTEST_SKIP() << "Skip on Windows"; } while(false) +#else +#define SKIP_ON_WINDOWS() do { } while(false) +#endif + +TEST(SafePointTrackerTest, RegisterSafePoints) { + SKIP_ON_WINDOWS(); + []() OPTNONE { + internal::SafePointTracker<> tracker; + + for (size_t i = 0; i < 10; i++) { + bool registered1 = tracker.registerCurrentSafePoint(0); + bool registered2 = tracker.registerCurrentSafePoint(0); + + bool expected = (i == 0); + + EXPECT_THAT(registered1, expected); + EXPECT_THAT(registered2, expected); + } + }(); +} + +template +OPTNONE bool registerCurrentSafePoint(internal::SafePointTracker& tracker) { + return tracker.registerCurrentSafePoint(0); +} + +TEST(SafePointTrackerTest, TrackTopFramesOnly) { + SKIP_ON_WINDOWS(); + []() OPTNONE { + internal::SafePointTracker<16> longTracker; + internal::SafePointTracker<1> shortTracker; + + bool longRegistered1 = registerCurrentSafePoint(longTracker); + bool longRegistered2 = registerCurrentSafePoint(longTracker); + + EXPECT_THAT(longRegistered1, true); + EXPECT_THAT(longRegistered2, true); + + bool shortRegistered1 = registerCurrentSafePoint(shortTracker); + bool shortRegistered2 = registerCurrentSafePoint(shortTracker); + + EXPECT_THAT(shortRegistered1, true); + EXPECT_THAT(shortRegistered2, false); + }(); +} + +TEST(SafePointTrackerTest, CleanOnSizeLimit) { + SKIP_ON_WINDOWS(); + []() OPTNONE { + internal::SafePointTracker<> tracker(2); + + ASSERT_THAT(tracker.size(), 0); + ASSERT_THAT(tracker.maxSize(), 2); + + for (size_t i = 0; i < 3; i++) { + bool registered1 = tracker.registerCurrentSafePoint(0); + + EXPECT_THAT(registered1, true); + EXPECT_THAT(tracker.size(), 1); + + bool registered2 = tracker.registerCurrentSafePoint(0); + + EXPECT_THAT(registered2, true); + EXPECT_THAT(tracker.size(), 2); + } + }(); +} + +TEST(AggressiveSchedulerTest, TriggerGCOnUniqueSafePoint) { + SKIP_ON_WINDOWS(); + []() OPTNONE { + testing::MockFunction scheduleGC; + + GCSchedulerConfig config; + gc::internal::GCSchedulerDataAggressive scheduler(config, scheduleGC.AsStdFunction()); + ASSERT_EQ(config.threshold, 1); + + GCSchedulerThreadData threadSchedulerData(config, [](GCSchedulerThreadData&){}); + + EXPECT_CALL(scheduleGC, Call()).Times(1); + for (int i = 0; i < 10; i++) { + scheduler.UpdateFromThreadData(threadSchedulerData); + } + testing::Mock::VerifyAndClearExpectations(&scheduleGC); + + EXPECT_CALL(scheduleGC, Call()).Times(1); + scheduler.UpdateFromThreadData(threadSchedulerData); + testing::Mock::VerifyAndClearExpectations(&scheduleGC); + }(); +} + +TEST(AggressiveSchedulerTest, TriggerGCOnAllocationThreshold) { + SKIP_ON_WINDOWS(); + []() OPTNONE { + testing::MockFunction scheduleGC; + + GCSchedulerConfig config; + gc::internal::GCSchedulerDataAggressive scheduler(config, scheduleGC.AsStdFunction()); + GCSchedulerThreadData threadSchedulerData(config, [&scheduler](GCSchedulerThreadData& data){ + scheduler.UpdateFromThreadData(data); + }); + + ASSERT_EQ(config.allocationThresholdBytes, 1); + + config.autoTune = false; + config.targetHeapBytes = 10; + + int i = 0; + // We trigger GC on the first iteration, when the unique allocation point is faced, + // and on the last iteration when target heap size is reached. + EXPECT_CALL(scheduleGC, Call()) + .WillOnce([&i]() { EXPECT_THAT(i, 0); }) + .WillOnce([&i]() { EXPECT_THAT(i, 9); }); + + for (; i < 10; i++) { + threadSchedulerData.OnSafePointAllocation(1); + } + testing::Mock::VerifyAndClearExpectations(&scheduleGC); + }(); +} + } // namespace gc } // namespace kotlin