Skip to content

Commit ee985d4

Browse files
committed
[scudo] Add time-based release to OS logic.
In order to preserve the least recently used (LRU) ordering of the cache in the list of decommitted entries, the cache entries are released from least recent to most recent. This also helps to avoid unnecessary scans of the cache since entries ready to be released (specifically, entries that are considered old relative to the configurable release interval) will always be at the tail of the list of committed entries by the LRU ordering.
1 parent 0889809 commit ee985d4

File tree

2 files changed

+44
-27
lines changed

2 files changed

+44
-27
lines changed

compiler-rt/lib/scudo/standalone/secondary.h

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,6 @@ class MapAllocatorCache {
321321
}
322322
CachedBlock PrevEntry = Quarantine[QuarantinePos];
323323
Quarantine[QuarantinePos] = Entry;
324-
if (OldestTime == 0)
325-
OldestTime = Entry.Time;
326324
Entry = PrevEntry;
327325
}
328326

@@ -344,8 +342,6 @@ class MapAllocatorCache {
344342

345343
insert(Entry, (Entry.Time == 0) ? DECOMMITTED : COMMITTED);
346344

347-
if (OldestTime == 0)
348-
OldestTime = Entry.Time;
349345
} while (0); // ScopedLock L(Mutex);
350346

351347
for (MemMapT &EvictMemMap : EvictionMemMaps)
@@ -585,32 +581,29 @@ class MapAllocatorCache {
585581
}
586582
}
587583

588-
void releaseIfOlderThan(CachedBlock &Entry, u64 Time) REQUIRES(Mutex) {
589-
if (!Entry.isValid() || !Entry.Time)
590-
return;
591-
if (Entry.Time > Time) {
592-
if (OldestTime == 0 || Entry.Time < OldestTime)
593-
OldestTime = Entry.Time;
594-
return;
595-
}
584+
inline void release(CachedBlock &Entry) {
585+
DCHECK(Entry.Time != 0);
596586
Entry.MemMap.releaseAndZeroPagesToOS(Entry.CommitBase, Entry.CommitSize);
597587
Entry.Time = 0;
598588
}
599589

600590
void releaseOlderThan(u64 Time) EXCLUDES(Mutex) {
601591
ScopedLock L(Mutex);
602-
if (!EntriesCount || OldestTime == 0 || OldestTime > Time)
603-
return;
604-
OldestTime = 0;
605-
for (uptr I = 0; I < Config::getQuarantineSize(); I++)
606-
releaseIfOlderThan(Quarantine[I], Time);
607-
for (u16 I = EntryLists[COMMITTED].Head; I != CachedBlock::InvalidEntry;
608-
I = Entries[I].Next) {
609-
if (Entries[I].Time && Entries[I].Time <= Time) {
610-
unlink(I, COMMITTED);
611-
pushFront(I, DECOMMITTED);
612-
}
613-
releaseIfOlderThan(Entries[I], Time);
592+
for (uptr I = 0; I < Config::getQuarantineSize(); I++) {
593+
CachedBlock &Entry = Quarantine[I];
594+
if (!Entry.isValid() || Entry.Time > Time)
595+
continue;
596+
release(Entry);
597+
}
598+
599+
// Release oldest entries first by releasing from tail
600+
u16 ReleaseIndex = EntryLists[COMMITTED].Tail;
601+
while (ReleaseIndex != CachedBlock::InvalidEntry &&
602+
Entries[ReleaseIndex].Time <= Time) {
603+
release(Entries[ReleaseIndex]);
604+
unlink(ReleaseIndex, COMMITTED);
605+
pushFront(ReleaseIndex, DECOMMITTED);
606+
ReleaseIndex = Entries[ReleaseIndex].Prev;
614607
}
615608
}
616609

@@ -619,7 +612,6 @@ class MapAllocatorCache {
619612
u32 QuarantinePos GUARDED_BY(Mutex) = 0;
620613
atomic_u32 MaxEntriesCount = {};
621614
atomic_uptr MaxEntrySize = {};
622-
u64 OldestTime GUARDED_BY(Mutex) = 0;
623615
atomic_s32 ReleaseToOsIntervalMs = {};
624616
u32 CallsToRetrieve GUARDED_BY(Mutex) = 0;
625617
u32 SuccessfulRetrieves GUARDED_BY(Mutex) = 0;

compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,14 +308,14 @@ struct MapAllocatorCacheTest : public Test {
308308
scudo::uptr NumEntries, scudo::uptr Size) {
309309
for (scudo::uptr I = 0; I < NumEntries; I++) {
310310
MemMaps.emplace_back(allocate(Size));
311-
auto &MemMap = MemMaps[I];
311+
auto &MemMap = MemMaps.back();
312312
Cache->store(Options, MemMap.getBase(), MemMap.getCapacity(),
313313
MemMap.getBase(), MemMap);
314314
}
315315
}
316316
};
317317

318-
TEST_F(MapAllocatorCacheTest, CacheOrder) {
318+
TEST_F(MapAllocatorCacheTest, CacheOrderNoRelease) {
319319
std::vector<scudo::MemMapT> MemMaps;
320320
Cache->setOption(scudo::Option::MaxCacheEntriesCount,
321321
CacheConfig::getEntriesArraySize());
@@ -336,6 +336,31 @@ TEST_F(MapAllocatorCacheTest, CacheOrder) {
336336
MemMap.unmap();
337337
}
338338

339+
TEST_F(MapAllocatorCacheTest, CacheOrderWithRelease) {
340+
std::vector<scudo::MemMapT> MemMaps;
341+
Cache->setOption(scudo::Option::MaxCacheEntriesCount,
342+
CacheConfig::getEntriesArraySize());
343+
344+
fillCacheWithSameSizeBlocks(MemMaps, CacheConfig::getEntriesArraySize(),
345+
TestAllocSize);
346+
347+
// Release all entries to transfer COMMITTED list contents to
348+
// DECOMMITTED list
349+
Cache->releaseToOS();
350+
351+
// Retrieval order should be the inverse of insertion order
352+
for (scudo::uptr I = CacheConfig::getEntriesArraySize(); I > 0; I--) {
353+
scudo::uptr EntryHeaderPos;
354+
scudo::CachedBlock Entry =
355+
Cache->retrieve(TestAllocSize, PageSize, 0, EntryHeaderPos);
356+
EXPECT_EQ(Entry.MemMap.getBase(), MemMaps[I - 1].getBase());
357+
}
358+
359+
// Clean up MemMaps
360+
for (auto &MemMap : MemMaps)
361+
MemMap.unmap();
362+
}
363+
339364
TEST_F(MapAllocatorCacheTest, MemoryLeakTest) {
340365
std::vector<scudo::MemMapT> MemMaps;
341366
// Fill the cache above MaxEntriesCount to force an eviction

0 commit comments

Comments
 (0)