From 6e3a54a588ec0a19f002e6285ef432c203f4970e Mon Sep 17 00:00:00 2001 From: Chia-hung Duan Date: Tue, 10 Sep 2024 21:26:24 +0000 Subject: [PATCH] Add a DCHECK and some comments --- compiler-rt/lib/scudo/standalone/secondary.h | 25 +++++++++++++------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h index 7d85832d7fe714..46cfd53749a4c7 100644 --- a/compiler-rt/lib/scudo/standalone/secondary.h +++ b/compiler-rt/lib/scudo/standalone/secondary.h @@ -72,13 +72,16 @@ namespace { struct CachedBlock { static constexpr u16 CacheIndexMax = UINT16_MAX; static constexpr u16 InvalidEntry = CacheIndexMax; - // * MaxReleasedCachePages default is currently 4 - // - We arrived at this value after noticing that mapping - // in larger memory regions performs better than releasing - // memory and forcing a cache hit. According to the data, - // it suggests that beyond 4 pages, the release execution time is - // longer than the map execution time. In this way, the default - // is dependent on the platform. + // We allow a certain amount of fragmentation and part of the fragmented bytes + // will be released by `releaseAndZeroPagesToOS()`. This increases the chance + // of cache hit rate and reduce the overhead to the RSS at the same time. See + // more details in the `MapAllocatorCache::retrieve()` section. + // + // We arrived at this default value after noticing that mapping in larger + // memory regions performs better than releasing memory and forcing a cache + // hit. According to the data, it suggests that beyond 4 pages, the release + // execution time is longer than the map execution time. In this way, + // the default is dependent on the platform. static constexpr uptr MaxReleasedCachePages = 4U; uptr CommitBase = 0; @@ -725,8 +728,14 @@ MapAllocator::tryAllocateFromCache(const Options &Options, uptr Size, uptr EntryHeaderPos; uptr MaxAllowedFragmentedPages = MaxUnreleasedCachePages; - if (LIKELY(!useMemoryTagging(Options))) + if (LIKELY(!useMemoryTagging(Options))) { MaxAllowedFragmentedPages += CachedBlock::MaxReleasedCachePages; + } else { + // TODO: Enable MaxReleasedCachePages may result in pages for an entry being + // partially released and it erases the tag of those pages as well. To + // support this feature for MTE, we need to tag those pages again. + DCHECK_EQ(CachedBlock::MaxReleasedCachePages, 0U); + } Entry = Cache.retrieve(MaxAllowedFragmentedPages, Size, Alignment, getHeadersSize(), EntryHeaderPos);