Skip to content

Commit 9ed7b15

Browse files
JoshuaMBaJoshuaMBa
committed
[scudo] Add partial chunk heuristic to retrieval algorithm.
Previously the secondary cache retrieval algorithm would not allow retrievals of memory chunks where the number of unused bytes would be greater than than `MaxUnusedCachePages * PageSize` bytes. This meant that even if a memory chunk satisfied the requirements of the optimal fit algorithm, it may not be returned. This remains true if memory tagging is enabled. However, if memory tagging is disabled, a new heuristic has been put in place. Specifically, If a memory chunk is a non-optimal fit, the cache retrieval algorithm will attempt to release the excess memory to force a cache hit while keeping RSS down. In the event that a memory chunk is a non-optimal fit, the retrieval algorithm will release excess memory as long as the amount of memory to be released is less than or equal to 4 Pages. If the amount of memory to be released exceeds 4 Pages, the retrieval algorithm will not consider that cached memory chunk valid for retrieval.
1 parent bf68e90 commit 9ed7b15

File tree

2 files changed

+127
-38
lines changed

2 files changed

+127
-38
lines changed

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

Lines changed: 99 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ namespace {
7272
struct CachedBlock {
7373
static constexpr u16 CacheIndexMax = UINT16_MAX;
7474
static constexpr u16 InvalidEntry = CacheIndexMax;
75+
// * MaxReleasedCachePages default is currently 4
76+
// - We arrived at this value after noticing that mapping
77+
// in larger memory regions performs better than releasing
78+
// memory and forcing a cache hit. According to the data,
79+
// it suggests that beyond 4 pages, the release execution time is
80+
// longer than the map execution time. In this way, the default
81+
// is dependent on the platform.
82+
static constexpr uptr MaxReleasedCachePages = 4U;
7583

7684
uptr CommitBase = 0;
7785
uptr CommitSize = 0;
@@ -90,8 +98,9 @@ struct CachedBlock {
9098
template <typename Config> class MapAllocatorNoCache {
9199
public:
92100
void init(UNUSED s32 ReleaseToOsInterval) {}
93-
CachedBlock retrieve(UNUSED uptr Size, UNUSED uptr Alignment,
94-
UNUSED uptr HeadersSize, UNUSED uptr &EntryHeaderPos) {
101+
CachedBlock retrieve(UNUSED uptr MaxAllowedFragmentedBytes, UNUSED uptr Size,
102+
UNUSED uptr Alignment, UNUSED uptr HeadersSize,
103+
UNUSED uptr &EntryHeaderPos) {
95104
return {};
96105
}
97106
void store(UNUSED Options Options, UNUSED uptr CommitBase,
@@ -121,7 +130,7 @@ template <typename Config> class MapAllocatorNoCache {
121130
}
122131
};
123132

124-
static const uptr MaxUnusedCachePages = 4U;
133+
static const uptr MaxUnreleasedCachePages = 4U;
125134

126135
template <typename Config>
127136
bool mapSecondary(const Options &Options, uptr CommitBase, uptr CommitSize,
@@ -151,9 +160,11 @@ bool mapSecondary(const Options &Options, uptr CommitBase, uptr CommitSize,
151160
}
152161
}
153162

154-
const uptr MaxUnusedCacheBytes = MaxUnusedCachePages * PageSize;
155-
if (useMemoryTagging<Config>(Options) && CommitSize > MaxUnusedCacheBytes) {
156-
const uptr UntaggedPos = Max(AllocPos, CommitBase + MaxUnusedCacheBytes);
163+
const uptr MaxUnreleasedCacheBytes = MaxUnreleasedCachePages * PageSize;
164+
if (useMemoryTagging<Config>(Options) &&
165+
CommitSize > MaxUnreleasedCacheBytes) {
166+
const uptr UntaggedPos =
167+
Max(AllocPos, CommitBase + MaxUnreleasedCacheBytes);
157168
return MemMap.remap(CommitBase, UntaggedPos - CommitBase, "scudo:secondary",
158169
MAP_MEMTAG | Flags) &&
159170
MemMap.remap(UntaggedPos, CommitBase + CommitSize - UntaggedPos,
@@ -334,61 +345,105 @@ class MapAllocatorCache {
334345
}
335346
}
336347

337-
CachedBlock retrieve(uptr Size, uptr Alignment, uptr HeadersSize,
338-
uptr &EntryHeaderPos) EXCLUDES(Mutex) {
348+
CachedBlock retrieve(uptr MaxAllowedFragmentedPages, uptr Size,
349+
uptr Alignment, uptr HeadersSize, uptr &EntryHeaderPos)
350+
EXCLUDES(Mutex) {
339351
const uptr PageSize = getPageSizeCached();
340352
// 10% of the requested size proved to be the optimal choice for
341353
// retrieving cached blocks after testing several options.
342354
constexpr u32 FragmentedBytesDivisor = 10;
343-
bool Found = false;
355+
bool FoundOptimalFit = false;
344356
CachedBlock Entry;
345357
EntryHeaderPos = 0;
346358
{
347359
ScopedLock L(Mutex);
348360
CallsToRetrieve++;
349361
if (EntriesCount == 0)
350362
return {};
351-
u32 OptimalFitIndex = 0;
363+
u16 RetrievedIndex = CachedBlock::InvalidEntry;
352364
uptr MinDiff = UINTPTR_MAX;
353-
for (u32 I = LRUHead; I != CachedBlock::InvalidEntry;
365+
366+
// Since allocation sizes don't always match cached memory chunk sizes
367+
// we allow some memory to be unused (called fragmented bytes). The
368+
// amount of unused bytes is exactly EntryHeaderPos - CommitBase.
369+
//
370+
// CommitBase CommitBase + CommitSize
371+
// V V
372+
// +---+------------+-----------------+---+
373+
// | | | | |
374+
// +---+------------+-----------------+---+
375+
// ^ ^ ^
376+
// Guard EntryHeaderPos Guard-page-end
377+
// page-begin
378+
//
379+
// [EntryHeaderPos, CommitBase + CommitSize) contains the user data as
380+
// well as the header metadata. If EntryHeaderPos - CommitBase exceeds
381+
// MaxAllowedFragmentedPages * PageSize, the cached memory chunk is
382+
// not considered valid for retrieval.
383+
for (u16 I = LRUHead; I != CachedBlock::InvalidEntry;
354384
I = Entries[I].Next) {
355385
const uptr CommitBase = Entries[I].CommitBase;
356386
const uptr CommitSize = Entries[I].CommitSize;
357387
const uptr AllocPos =
358388
roundDown(CommitBase + CommitSize - Size, Alignment);
359389
const uptr HeaderPos = AllocPos - HeadersSize;
360-
if (HeaderPos > CommitBase + CommitSize)
390+
if (HeaderPos > CommitBase + CommitSize || HeaderPos < CommitBase)
361391
continue;
362-
if (HeaderPos < CommitBase ||
363-
AllocPos > CommitBase + PageSize * MaxUnusedCachePages) {
392+
393+
const uptr Diff = roundDown(HeaderPos, PageSize) - CommitBase;
394+
395+
if (Diff > MaxAllowedFragmentedPages * PageSize || Diff >= MinDiff)
364396
continue;
365-
}
366-
Found = true;
367-
const uptr Diff = HeaderPos - CommitBase;
368-
// immediately use a cached block if it's size is close enough to the
369-
// requested size.
370-
const uptr MaxAllowedFragmentedBytes =
397+
398+
MinDiff = Diff;
399+
RetrievedIndex = I;
400+
EntryHeaderPos = HeaderPos;
401+
402+
const uptr OptimalFitThesholdBytes =
371403
(CommitBase + CommitSize - HeaderPos) / FragmentedBytesDivisor;
372-
if (Diff <= MaxAllowedFragmentedBytes) {
373-
OptimalFitIndex = I;
374-
EntryHeaderPos = HeaderPos;
404+
if (Diff <= OptimalFitThesholdBytes) {
405+
FoundOptimalFit = true;
375406
break;
376407
}
377-
// keep track of the smallest cached block
378-
// that is greater than (AllocSize + HeaderSize)
379-
if (Diff > MinDiff)
380-
continue;
381-
OptimalFitIndex = I;
382-
MinDiff = Diff;
383-
EntryHeaderPos = HeaderPos;
384408
}
385-
if (Found) {
386-
Entry = Entries[OptimalFitIndex];
387-
remove(OptimalFitIndex);
409+
if (RetrievedIndex != CachedBlock::InvalidEntry) {
410+
Entry = Entries[RetrievedIndex];
411+
remove(RetrievedIndex);
388412
SuccessfulRetrieves++;
389413
}
390414
}
391415

416+
// The difference between the retrieved memory chunk and the request
417+
// size is at most MaxAllowedFragmentedPages
418+
//
419+
// / MaxAllowedFragmentedPages * PageSize \
420+
// +--------------------------+-------------+
421+
// | | |
422+
// +--------------------------+-------------+
423+
// \ Bytes to be released / ^
424+
// |
425+
// (may or may not be committed)
426+
//
427+
// The maximum number of bytes released to the OS is capped by
428+
// MaxReleasedCachePages
429+
//
430+
// TODO : Consider making MaxReleasedCachePages configurable since
431+
// the release to OS API can vary across systems.
432+
if (!FoundOptimalFit && Entry.Time != 0) {
433+
const uptr FragmentedBytes =
434+
roundDown(EntryHeaderPos, PageSize) - Entry.CommitBase;
435+
const uptr MaxUnreleasedCacheBytes = MaxUnreleasedCachePages * PageSize;
436+
if (FragmentedBytes > MaxUnreleasedCacheBytes) {
437+
const uptr MaxReleasedCacheBytes =
438+
CachedBlock::MaxReleasedCachePages * PageSize;
439+
uptr BytesToRelease =
440+
roundUp(Min<uptr>(MaxReleasedCacheBytes,
441+
FragmentedBytes - MaxUnreleasedCacheBytes),
442+
PageSize);
443+
Entry.MemMap.releaseAndZeroPagesToOS(Entry.CommitBase, BytesToRelease);
444+
}
445+
}
446+
392447
return Entry;
393448
}
394449

@@ -659,8 +714,18 @@ MapAllocator<Config>::tryAllocateFromCache(const Options &Options, uptr Size,
659714
FillContentsMode FillContents) {
660715
CachedBlock Entry;
661716
uptr EntryHeaderPos;
717+
uptr MaxAllowedFragmentedPages;
718+
719+
if (LIKELY(!useMemoryTagging<Config>(Options))) {
720+
MaxAllowedFragmentedPages =
721+
MaxUnreleasedCachePages + CachedBlock::MaxReleasedCachePages;
722+
723+
} else {
724+
MaxAllowedFragmentedPages = MaxUnreleasedCachePages;
725+
}
662726

663-
Entry = Cache.retrieve(Size, Alignment, getHeadersSize(), EntryHeaderPos);
727+
Entry = Cache.retrieve(MaxAllowedFragmentedPages, Size, Alignment,
728+
getHeadersSize(), EntryHeaderPos);
664729
if (!Entry.isValid())
665730
return nullptr;
666731

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,8 @@ struct MapAllocatorCacheTest : public Test {
281281
std::unique_ptr<CacheT> Cache = std::make_unique<CacheT>();
282282

283283
const scudo::uptr PageSize = scudo::getPageSizeCached();
284-
// The current test allocation size is set to the minimum size
285-
// needed for the scudo allocator to fall back to the secondary allocator
284+
// The current test allocation size is set to the maximum
285+
// cache entry size
286286
static constexpr scudo::uptr TestAllocSize =
287287
CacheConfig::getDefaultMaxEntrySize();
288288

@@ -327,7 +327,7 @@ TEST_F(MapAllocatorCacheTest, CacheOrder) {
327327
for (scudo::uptr I = CacheConfig::getEntriesArraySize(); I > 0; I--) {
328328
scudo::uptr EntryHeaderPos;
329329
scudo::CachedBlock Entry =
330-
Cache->retrieve(TestAllocSize, PageSize, 0, EntryHeaderPos);
330+
Cache->retrieve(0, TestAllocSize, PageSize, 0, EntryHeaderPos);
331331
EXPECT_EQ(Entry.MemMap.getBase(), MemMaps[I - 1].getBase());
332332
}
333333

@@ -336,6 +336,30 @@ TEST_F(MapAllocatorCacheTest, CacheOrder) {
336336
MemMap.unmap();
337337
}
338338

339+
TEST_F(MapAllocatorCacheTest, PartialChunkHeuristicRetrievalTest) {
340+
const scudo::uptr FragmentedPages =
341+
1 + scudo::CachedBlock::MaxReleasedCachePages;
342+
scudo::uptr EntryHeaderPos;
343+
scudo::CachedBlock Entry;
344+
scudo::MemMapT MemMap = allocate(PageSize + FragmentedPages * PageSize);
345+
Cache->store(Options, MemMap.getBase(), MemMap.getCapacity(),
346+
MemMap.getBase(), MemMap);
347+
348+
// FragmentedPages > MaxAllowedFragmentedPages so PageSize
349+
// cannot be retrieved from the cache
350+
Entry = Cache->retrieve(/*MaxAllowedFragmentedPages=*/0, PageSize, PageSize,
351+
0, EntryHeaderPos);
352+
EXPECT_FALSE(Entry.isValid());
353+
354+
// FragmentedPages == MaxAllowedFragmentedPages so PageSize
355+
// can be retrieved from the cache
356+
Entry =
357+
Cache->retrieve(FragmentedPages, PageSize, PageSize, 0, EntryHeaderPos);
358+
EXPECT_TRUE(Entry.isValid());
359+
360+
MemMap.unmap();
361+
}
362+
339363
TEST_F(MapAllocatorCacheTest, MemoryLeakTest) {
340364
std::vector<scudo::MemMapT> MemMaps;
341365
// Fill the cache above MaxEntriesCount to force an eviction
@@ -351,7 +375,7 @@ TEST_F(MapAllocatorCacheTest, MemoryLeakTest) {
351375
for (scudo::uptr I = CacheConfig::getDefaultMaxEntriesCount(); I > 0; I--) {
352376
scudo::uptr EntryHeaderPos;
353377
RetrievedEntries.push_back(
354-
Cache->retrieve(TestAllocSize, PageSize, 0, EntryHeaderPos));
378+
Cache->retrieve(0, TestAllocSize, PageSize, 0, EntryHeaderPos));
355379
EXPECT_EQ(MemMaps[I].getBase(), RetrievedEntries.back().MemMap.getBase());
356380
}
357381

0 commit comments

Comments
 (0)