From d89ba8d5772c70b48c8621cdd58c1cccf2af336b Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Wed, 15 Oct 2025 20:18:39 -0700 Subject: [PATCH 1/2] [scudo] Secondary release to OS uses LRU to scan. Before this change, the code would scan the entire set of cached entries to find ones to be released. Now, it uses the LRUEntries list to iterate over the live cached entries. In addition, remove the OldestTime variable and replace it with OldestPresentEntry which will always be the oldest entry in the LRU that has Time non-zero. --- compiler-rt/lib/scudo/standalone/secondary.h | 68 +++++++++------ .../scudo/standalone/tests/secondary_test.cpp | 85 +++++++++++++++++++ 2 files changed, 126 insertions(+), 27 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h index f0b7bceb010f0..9f24f5ceeb333 100644 --- a/compiler-rt/lib/scudo/standalone/secondary.h +++ b/compiler-rt/lib/scudo/standalone/secondary.h @@ -249,6 +249,7 @@ class MapAllocatorCache { LRUEntries.clear(); LRUEntries.init(Entries, sizeof(Entries)); + OldestPresentEntry = nullptr; AvailEntries.clear(); AvailEntries.init(Entries, sizeof(Entries)); @@ -322,8 +323,6 @@ class MapAllocatorCache { } CachedBlock PrevEntry = Quarantine[QuarantinePos]; Quarantine[QuarantinePos] = Entry; - if (OldestTime == 0) - OldestTime = Entry.Time; Entry = PrevEntry; } @@ -339,9 +338,6 @@ class MapAllocatorCache { } insert(Entry); - - if (OldestTime == 0) - OldestTime = Entry.Time; } while (0); for (MemMapT &EvictMemMap : EvictionMemMaps) @@ -355,7 +351,6 @@ class MapAllocatorCache { SCUDO_SCOPED_TRACE( GetSecondaryReleaseToOSTraceName(ReleaseToOS::Normal)); - // TODO: Add ReleaseToOS logic to LRU algorithm releaseOlderThan(Time - static_cast(Interval) * 1000000); Mutex.unlock(); } else @@ -535,6 +530,11 @@ class MapAllocatorCache { void unmapTestOnly() { empty(); } + void releaseOlderThanTestOnly(u64 ReleaseTime) { + ScopedLock L(Mutex); + releaseOlderThan(ReleaseTime); + } + private: void insert(const CachedBlock &Entry) REQUIRES(Mutex) { CachedBlock *AvailEntry = AvailEntries.front(); @@ -542,10 +542,16 @@ class MapAllocatorCache { *AvailEntry = Entry; LRUEntries.push_front(AvailEntry); + if (OldestPresentEntry == nullptr && AvailEntry->Time != 0) + OldestPresentEntry = AvailEntry; } void remove(CachedBlock *Entry) REQUIRES(Mutex) { DCHECK(Entry->isValid()); + if (OldestPresentEntry == Entry) { + OldestPresentEntry = LRUEntries.getPrev(Entry); + DCHECK(OldestPresentEntry == nullptr || OldestPresentEntry->Time != 0); + } LRUEntries.remove(Entry); Entry->invalidate(); AvailEntries.push_front(Entry); @@ -560,6 +566,7 @@ class MapAllocatorCache { for (CachedBlock &Entry : LRUEntries) MapInfo[N++] = Entry.MemMap; LRUEntries.clear(); + OldestPresentEntry = nullptr; } for (uptr I = 0; I < N; I++) { MemMapT &MemMap = MapInfo[I]; @@ -567,36 +574,41 @@ class MapAllocatorCache { } } - void releaseIfOlderThan(CachedBlock &Entry, u64 Time) REQUIRES(Mutex) { - if (!Entry.isValid() || !Entry.Time) - return; - if (Entry.Time > Time) { - if (OldestTime == 0 || Entry.Time < OldestTime) - OldestTime = Entry.Time; - return; - } - Entry.MemMap.releaseAndZeroPagesToOS(Entry.CommitBase, Entry.CommitSize); - Entry.Time = 0; - } - - void releaseOlderThan(u64 Time) REQUIRES(Mutex) { + void releaseOlderThan(u64 ReleaseTime) REQUIRES(Mutex) { SCUDO_SCOPED_TRACE(GetSecondaryReleaseOlderThanTraceName()); - if (!LRUEntries.size() || OldestTime == 0 || OldestTime > Time) - return; - OldestTime = 0; if (!Config::getQuarantineDisabled()) - for (uptr I = 0; I < Config::getQuarantineSize(); I++) - releaseIfOlderThan(Quarantine[I], Time); - for (uptr I = 0; I < Config::getEntriesArraySize(); I++) - releaseIfOlderThan(Entries[I], Time); + for (uptr I = 0; I < Config::getQuarantineSize(); I++) { + auto &Entry = Quarantine[I]; + if (!Entry.isValid() || Entry.Time == 0 || Entry.Time > ReleaseTime) + continue; + Entry.MemMap.releaseAndZeroPagesToOS(Entry.CommitBase, + Entry.CommitSize); + Entry.Time = 0; + } + + for (CachedBlock *Entry = OldestPresentEntry; Entry != nullptr; + Entry = LRUEntries.getPrev(Entry)) { + DCHECK(Entry->isValid()); + DCHECK(Entry->Time != 0); + + if (Entry->Time > ReleaseTime) { + // All entries are newer than this, so no need to keep scanning. + OldestPresentEntry = Entry; + return; + } + + Entry->MemMap.releaseAndZeroPagesToOS(Entry->CommitBase, + Entry->CommitSize); + Entry->Time = 0; + } + OldestPresentEntry = nullptr; } HybridMutex Mutex; u32 QuarantinePos GUARDED_BY(Mutex) = 0; atomic_u32 MaxEntriesCount = {}; atomic_uptr MaxEntrySize = {}; - u64 OldestTime GUARDED_BY(Mutex) = 0; atomic_s32 ReleaseToOsIntervalMs = {}; u32 CallsToRetrieve GUARDED_BY(Mutex) = 0; u32 SuccessfulRetrieves GUARDED_BY(Mutex) = 0; @@ -606,6 +618,8 @@ class MapAllocatorCache { NonZeroLengthArray Quarantine GUARDED_BY(Mutex) = {}; + // The oldest entry in the LRUEntries that has Time non-zero. + CachedBlock *OldestPresentEntry GUARDED_BY(Mutex) = nullptr; // Cached blocks stored in LRU order DoublyLinkedList LRUEntries GUARDED_BY(Mutex); // The unused Entries diff --git a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp index d8a7f6bd66ed2..855a3e6e6109f 100644 --- a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp @@ -403,6 +403,11 @@ template struct CacheInfoType { MemMap.getBase(), MemMap); } } + + void storeMemMap(scudo::MemMapT &MemMap) { + Cache->store(Options, MemMap.getBase(), MemMap.getCapacity(), + MemMap.getBase(), MemMap); + } }; TEST(ScudoSecondaryTest, AllocatorCacheEntryOrder) { @@ -503,3 +508,83 @@ TEST(ScudoSecondaryTest, AllocatorCacheOptions) { Info.Cache->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 20)); EXPECT_TRUE(Info.Cache->canCache(1UL << 16)); } + +TEST(ScudoSecondaryTest, ReleaseOlderThanAllEntries) { + CacheInfoType Info; + using CacheConfig = CacheInfoType::CacheConfig; + + Info.Cache->releaseOlderThanTestOnly(UINT64_MAX); + + Info.fillCacheWithSameSizeBlocks(CacheConfig::getDefaultMaxEntriesCount(), + 1024); + for (size_t I = 0; I < Info.MemMaps.size(); I++) { + // Set the first u32 value to a non-zero value. + *reinterpret_cast(Info.MemMaps[I].getBase()) = 10; + } + + Info.Cache->releaseOlderThanTestOnly(UINT64_MAX); + + EXPECT_EQ(Info.MemMaps.size(), CacheConfig::getDefaultMaxEntriesCount()); + for (size_t I = 0; I < Info.MemMaps.size(); I++) { + // All released maps will now be zero. + EXPECT_EQ(*reinterpret_cast(Info.MemMaps[I].getBase()), 0U); + } +} + +// This test assumes that the timestamp comes from getMonotonicFast. +TEST(ScudoSecondaryTest, ReleaseOlderThanGroups) { + CacheInfoType Info; + + // Disable the release interval so we can do tests the releaseOlderThan + // function. + Info.Cache->setOption(scudo::Option::ReleaseInterval, -1); + + // Create all of the maps we are going to use. + for (size_t I = 0; I < 6; I++) { + Info.MemMaps.emplace_back(Info.allocate(1024)); + // Set the first u32 value to a non-zero value. + *reinterpret_cast(Info.MemMaps[I].getBase()) = 10; + } + + // Create three groups of entries at three different intervals. + Info.storeMemMap(Info.MemMaps[0]); + Info.storeMemMap(Info.MemMaps[1]); + scudo::u64 FirstTime = scudo::getMonotonicTimeFast(); + + // Need to make sure the next set of entries are stamped with a newer time. + while (scudo::getMonotonicTimeFast() <= FirstTime) + ; + + Info.storeMemMap(Info.MemMaps[2]); + Info.storeMemMap(Info.MemMaps[3]); + scudo::u64 SecondTime = scudo::getMonotonicTimeFast(); + + // Need to make sure the next set of entries are stamped with a newer time. + while (scudo::getMonotonicTimeFast() <= SecondTime) + ; + + Info.storeMemMap(Info.MemMaps[4]); + Info.storeMemMap(Info.MemMaps[5]); + scudo::u64 ThirdTime = scudo::getMonotonicTimeFast(); + + Info.Cache->releaseOlderThanTestOnly(FirstTime); + for (size_t I = 0; I < 2; I++) { + EXPECT_EQ(*reinterpret_cast(Info.MemMaps[I].getBase()), 0U); + } + for (size_t I = 2; I < 6; I++) { + EXPECT_EQ(*reinterpret_cast(Info.MemMaps[I].getBase()), 10U); + } + + Info.Cache->releaseOlderThanTestOnly(SecondTime); + for (size_t I = 0; I < 4; I++) { + EXPECT_EQ(*reinterpret_cast(Info.MemMaps[I].getBase()), 0U); + } + for (size_t I = 4; I < 6; I++) { + EXPECT_EQ(*reinterpret_cast(Info.MemMaps[I].getBase()), 10U); + } + + Info.Cache->releaseOlderThanTestOnly(ThirdTime); + for (size_t I = 0; I < 6; I++) { + EXPECT_EQ(*reinterpret_cast(Info.MemMaps[I].getBase()), 0U); + } +} From a067f66387fabefb930cbba1bca06dbc70630707 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Thu, 23 Oct 2025 16:41:54 -0700 Subject: [PATCH 2/2] Update for review. --- compiler-rt/lib/scudo/standalone/secondary.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h index 9f24f5ceeb333..2509db26e6c82 100644 --- a/compiler-rt/lib/scudo/standalone/secondary.h +++ b/compiler-rt/lib/scudo/standalone/secondary.h @@ -577,7 +577,7 @@ class MapAllocatorCache { void releaseOlderThan(u64 ReleaseTime) REQUIRES(Mutex) { SCUDO_SCOPED_TRACE(GetSecondaryReleaseOlderThanTraceName()); - if (!Config::getQuarantineDisabled()) + if (!Config::getQuarantineDisabled()) { for (uptr I = 0; I < Config::getQuarantineSize(); I++) { auto &Entry = Quarantine[I]; if (!Entry.isValid() || Entry.Time == 0 || Entry.Time > ReleaseTime) @@ -586,6 +586,7 @@ class MapAllocatorCache { Entry.CommitSize); Entry.Time = 0; } + } for (CachedBlock *Entry = OldestPresentEntry; Entry != nullptr; Entry = LRUEntries.getPrev(Entry)) {