Skip to content

Commit 043de74

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 043de74

File tree

2 files changed

+130
-34
lines changed

2 files changed

+130
-34
lines changed

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

Lines changed: 102 additions & 30 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 = 0U;
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,112 @@ 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;
344355
CachedBlock Entry;
345356
EntryHeaderPos = 0;
346357
{
347358
ScopedLock L(Mutex);
348359
CallsToRetrieve++;
349360
if (EntriesCount == 0)
350361
return {};
351-
u32 OptimalFitIndex = 0;
362+
u16 RetrievedIndex = CachedBlock::InvalidEntry;
352363
uptr MinDiff = UINTPTR_MAX;
353-
for (u32 I = LRUHead; I != CachedBlock::InvalidEntry;
364+
365+
// Since allocation sizes don't always match cached memory chunk sizes
366+
// we allow some memory to be unused (called fragmented bytes). The
367+
// amount of unused bytes is exactly EntryHeaderPos - CommitBase.
368+
//
369+
// CommitBase CommitBase + CommitSize
370+
// V V
371+
// +---+------------+-----------------+---+
372+
// | | | | |
373+
// +---+------------+-----------------+---+
374+
// ^ ^ ^
375+
// Guard EntryHeaderPos Guard-page-end
376+
// page-begin
377+
//
378+
// [EntryHeaderPos, CommitBase + CommitSize) contains the user data as
379+
// well as the header metadata. If EntryHeaderPos - CommitBase exceeds
380+
// MaxAllowedFragmentedPages * PageSize, the cached memory chunk is
381+
// not considered valid for retrieval.
382+
for (u16 I = LRUHead; I != CachedBlock::InvalidEntry;
354383
I = Entries[I].Next) {
355384
const uptr CommitBase = Entries[I].CommitBase;
356385
const uptr CommitSize = Entries[I].CommitSize;
357386
const uptr AllocPos =
358387
roundDown(CommitBase + CommitSize - Size, Alignment);
359388
const uptr HeaderPos = AllocPos - HeadersSize;
389+
const uptr MaxAllowedFragmentedBytes =
390+
MaxAllowedFragmentedPages * PageSize;
360391
if (HeaderPos > CommitBase + CommitSize)
361392
continue;
362393
if (HeaderPos < CommitBase ||
363-
AllocPos > CommitBase + PageSize * MaxUnusedCachePages) {
394+
AllocPos > CommitBase + MaxAllowedFragmentedBytes) {
364395
continue;
365396
}
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 =
371-
(CommitBase + CommitSize - HeaderPos) / FragmentedBytesDivisor;
372-
if (Diff <= MaxAllowedFragmentedBytes) {
373-
OptimalFitIndex = I;
374-
EntryHeaderPos = HeaderPos;
375-
break;
376-
}
377-
// keep track of the smallest cached block
397+
398+
const uptr Diff = roundDown(HeaderPos, PageSize) - CommitBase;
399+
400+
// Keep track of the smallest cached block
378401
// that is greater than (AllocSize + HeaderSize)
379-
if (Diff > MinDiff)
402+
if (Diff >= MinDiff)
380403
continue;
381-
OptimalFitIndex = I;
404+
382405
MinDiff = Diff;
406+
RetrievedIndex = I;
383407
EntryHeaderPos = HeaderPos;
408+
409+
// Immediately use a cached block if its size is close enough to the
410+
// requested size
411+
const uptr OptimalFitThesholdBytes =
412+
(CommitBase + CommitSize - HeaderPos) / FragmentedBytesDivisor;
413+
if (Diff <= OptimalFitThesholdBytes)
414+
break;
384415
}
385-
if (Found) {
386-
Entry = Entries[OptimalFitIndex];
387-
remove(OptimalFitIndex);
416+
if (RetrievedIndex != CachedBlock::InvalidEntry) {
417+
Entry = Entries[RetrievedIndex];
418+
remove(RetrievedIndex);
388419
SuccessfulRetrieves++;
389420
}
390421
}
391422

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

@@ -659,8 +721,18 @@ MapAllocator<Config>::tryAllocateFromCache(const Options &Options, uptr Size,
659721
FillContentsMode FillContents) {
660722
CachedBlock Entry;
661723
uptr EntryHeaderPos;
724+
uptr MaxAllowedFragmentedPages;
725+
726+
if (LIKELY(!useMemoryTagging<Config>(Options))) {
727+
MaxAllowedFragmentedPages =
728+
MaxUnreleasedCachePages + CachedBlock::MaxReleasedCachePages;
729+
730+
} else {
731+
MaxAllowedFragmentedPages = MaxUnreleasedCachePages;
732+
}
662733

663-
Entry = Cache.retrieve(Size, Alignment, getHeadersSize(), EntryHeaderPos);
734+
Entry = Cache.retrieve(MaxAllowedFragmentedPages, Size, Alignment,
735+
getHeadersSize(), EntryHeaderPos);
664736
if (!Entry.isValid())
665737
return nullptr;
666738

‎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)