[compiler-rt] daaef4c - Revert "Revert "Revert "[scudo] Only prepare PageMap entry for partial region"""
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 6 13:15:31 PST 2023
Probably worth editing the commit message in cases like this (anything
with more than one "Revert" especially) - trying to think through
N*"Revert" to figure out whether this is a reapplication or not -
probably including the commit hash series and a single "Revert" rather
than N*"Revert"
On Thu, Mar 2, 2023 at 12:39 PM Chia-hung Duan via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Chia-hung Duan
> Date: 2023-03-02T20:37:55Z
> New Revision: daaef4c49954cb04ea1831615e0876865a29a08a
>
> URL: https://github.com/llvm/llvm-project/commit/daaef4c49954cb04ea1831615e0876865a29a08a
> DIFF: https://github.com/llvm/llvm-project/commit/daaef4c49954cb04ea1831615e0876865a29a08a.diff
>
> LOG: Revert "Revert "Revert "[scudo] Only prepare PageMap entry for partial region"""
>
> ScudoReleaseTest.ReleaseFreeMemoryToOSAndroid failed on Fuchsia
>
> This reverts commit c6ef6bbd8d964028ee6c2f03441604d7a7ba5375.
>
> Added:
>
>
> Modified:
> compiler-rt/lib/scudo/standalone/primary32.h
> compiler-rt/lib/scudo/standalone/primary64.h
> compiler-rt/lib/scudo/standalone/release.h
> compiler-rt/lib/scudo/standalone/tests/release_test.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h
> index 0edc40d7e6c36..df4053a6dde4e 100644
> --- a/compiler-rt/lib/scudo/standalone/primary32.h
> +++ b/compiler-rt/lib/scudo/standalone/primary32.h
> @@ -762,8 +762,7 @@ template <typename Config> class SizeClassAllocator32 {
> compactPtrGroup(compactPtr(ClassId, Sci->CurrentRegion));
>
> ReleaseRecorder Recorder(Base);
> - PageReleaseContext Context(BlockSize, RegionSize, NumberOfRegions,
> - /*ReleaseSize=*/RegionSize);
> + PageReleaseContext Context(BlockSize, RegionSize, NumberOfRegions);
>
> auto DecompactPtr = [](CompactPtrT CompactPtr) {
> return reinterpret_cast<uptr>(CompactPtr);
>
> diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
> index 4caf8eb1751a8..c0e50218fb1d9 100644
> --- a/compiler-rt/lib/scudo/standalone/primary64.h
> +++ b/compiler-rt/lib/scudo/standalone/primary64.h
> @@ -794,34 +794,19 @@ template <typename Config> class SizeClassAllocator64 {
>
> const uptr GroupSize = (1U << GroupSizeLog);
> const uptr AllocatedUserEnd = Region->AllocatedUser + Region->RegionBeg;
> + ReleaseRecorder Recorder(Region->RegionBeg, &Region->Data);
> + PageReleaseContext Context(BlockSize, Region->AllocatedUser,
> + /*NumberOfRegions=*/1U);
> +
> const uptr CompactPtrBase = getCompactPtrBaseByClassId(ClassId);
> auto DecompactPtr = [CompactPtrBase](CompactPtrT CompactPtr) {
> return decompactPtrInternal(CompactPtrBase, CompactPtr);
> };
> -
> - // Instead of always preparing PageMap for the entire region, we only do it
> - // for the range of releasing groups. To do that, the free-block marking
> - // process includes visiting BlockGroups twice.
> -
> - // The first visit is to determine the range of BatchGroups we are going to
> - // release. And we will extract those BatchGroups out and push into
> - // `GroupToRelease`.
> - SinglyLinkedList<BatchGroup> GroupToRelease;
> - GroupToRelease.clear();
> -
> - // This is only used for debugging to ensure the consistency of the number
> - // of groups.
> - uptr NumberOfBatchGroups = Region->FreeList.size();
> -
> - for (BatchGroup *BG = Region->FreeList.front(), *Prev = nullptr;
> - BG != nullptr;) {
> + for (BatchGroup &BG : Region->FreeList) {
> const uptr PushedBytesDelta =
> - BG->PushedBlocks - BG->PushedBlocksAtLastCheckpoint;
> - if (PushedBytesDelta * BlockSize < PageSize) {
> - Prev = BG;
> - BG = BG->Next;
> + BG.PushedBlocks - BG.PushedBlocksAtLastCheckpoint;
> + if (PushedBytesDelta * BlockSize < PageSize)
> continue;
> - }
>
> // Group boundary does not necessarily have the same alignment as Region.
> // It may sit across a Region boundary. Which means that we may have the
> @@ -829,7 +814,7 @@ template <typename Config> class SizeClassAllocator64 {
> //
> // 1. Group boundary sits before RegionBeg.
> //
> - // (BatchGroupBase)
> + // (BatchGroupBeg)
> // batchGroupBase RegionBeg BatchGroupEnd
> // | | |
> // v v v
> @@ -839,7 +824,7 @@ template <typename Config> class SizeClassAllocator64 {
> //
> // 2. Group boundary sits after RegionBeg.
> //
> - // (BatchGroupBase)
> + // (BatchGroupBeg)
> // RegionBeg batchGroupBase BatchGroupEnd
> // | | |
> // v v v
> @@ -850,24 +835,21 @@ template <typename Config> class SizeClassAllocator64 {
> // Note that in the first case, the group range before RegionBeg is never
> // used. Therefore, while calculating the used group size, we should
> // exclude that part to get the correct size.
> - const uptr BatchGroupBase =
> - Max(batchGroupBase(CompactPtrBase, BG->GroupId), Region->RegionBeg);
> - DCHECK_GE(AllocatedUserEnd, BatchGroupBase);
> + const uptr BatchGroupBeg =
> + Max(batchGroupBase(CompactPtrBase, BG.GroupId), Region->RegionBeg);
> + DCHECK_GE(AllocatedUserEnd, BatchGroupBeg);
> const uptr BatchGroupEnd =
> - batchGroupBase(CompactPtrBase, BG->GroupId) + GroupSize;
> + batchGroupBase(CompactPtrBase, BG.GroupId) + GroupSize;
> const uptr AllocatedGroupSize = AllocatedUserEnd >= BatchGroupEnd
> - ? BatchGroupEnd - BatchGroupBase
> - : AllocatedUserEnd - BatchGroupBase;
> - if (AllocatedGroupSize == 0) {
> - Prev = BG;
> - BG = BG->Next;
> + ? BatchGroupEnd - BatchGroupBeg
> + : AllocatedUserEnd - BatchGroupBeg;
> + if (AllocatedGroupSize == 0)
> continue;
> - }
>
> // TransferBatches are pushed in front of BG.Batches. The first one may
> // not have all caches used.
> - const uptr NumBlocks = (BG->Batches.size() - 1) * BG->MaxCachedPerBatch +
> - BG->Batches.front()->getCount();
> + const uptr NumBlocks = (BG.Batches.size() - 1) * BG.MaxCachedPerBatch +
> + BG.Batches.front()->getCount();
> const uptr BytesInBG = NumBlocks * BlockSize;
> // Given the randomness property, we try to release the pages only if the
> // bytes used by free blocks exceed certain proportion of group size. Note
> @@ -875,92 +857,12 @@ template <typename Config> class SizeClassAllocator64 {
> // are allocated.
> if (CheckDensity && (BytesInBG * 100U) / AllocatedGroupSize <
> (100U - 1U - BlockSize / 16U)) {
> - Prev = BG;
> - BG = BG->Next;
> continue;
> }
>
> - // If `BG` is the first BatchGroup in the list, we only need to advance
> - // `BG` and call FreeList::pop_front(). No update is needed for `Prev`.
> - //
> - // (BG) (BG->Next)
> - // Prev Cur BG
> - // | | |
> - // v v v
> - // nil +--+ +--+
> - // |X | -> | | -> ...
> - // +--+ +--+
> - //
> - // Otherwise, `Prev` will be used to extract the `Cur` from the
> - // `FreeList`.
> - //
> - // (BG) (BG->Next)
> - // Prev Cur BG
> - // | | |
> - // v v v
> - // +--+ +--+ +--+
> - // | | -> |X | -> | | -> ...
> - // +--+ +--+ +--+
> - //
> - // After FreeList::extract(),
> - //
> - // Prev Cur BG
> - // | | |
> - // v v v
> - // +--+ +--+ +--+
> - // | |-+ |X | +->| | -> ...
> - // +--+ | +--+ | +--+
> - // +--------+
> - //
> - // Note that we need to advance before pushing this BatchGroup to
> - // GroupToRelease because it's a destructive operation.
> -
> - BatchGroup *Cur = BG;
> - BG = BG->Next;
> -
> - if (Prev != nullptr)
> - Region->FreeList.extract(Prev, Cur);
> - else
> - Region->FreeList.pop_front();
> - GroupToRelease.push_back(Cur);
> - }
> -
> - if (GroupToRelease.empty())
> - return 0;
> -
> - const uptr ReleaseBase =
> - Max(batchGroupBase(CompactPtrBase, GroupToRelease.front()->GroupId),
> - Region->RegionBeg);
> - const uptr LastGroupEnd =
> - Min(batchGroupBase(CompactPtrBase, GroupToRelease.back()->GroupId) +
> - GroupSize,
> - AllocatedUserEnd);
> - // The last block may straddle the group boundary. Rounding up to BlockSize
> - // to get the exact range.
> - const uptr ReleaseEnd =
> - roundUpSlow(LastGroupEnd - Region->RegionBeg, BlockSize) +
> - Region->RegionBeg;
> - const uptr ReleaseRangeSize = ReleaseEnd - ReleaseBase;
> -
> - ReleaseRecorder Recorder(ReleaseBase, &Region->Data);
> - PageReleaseContext Context(
> - BlockSize, Region->AllocatedUser, /*NumberOfRegions=*/1U,
> - ReleaseRangeSize, /*ReleaseOffset=*/ReleaseBase - Region->RegionBeg);
> -
> - for (BatchGroup &BG : GroupToRelease) {
> BG.PushedBlocksAtLastCheckpoint = BG.PushedBlocks;
>
> - // TODO(chiahungduan): Replace GroupId with BatchGroupBase to simplify the
> - // calculation here and above (where we determine the set of groups to
> - // release).
> - const uptr BatchGroupBase =
> - Max(batchGroupBase(CompactPtrBase, BG.GroupId), Region->RegionBeg);
> - const uptr BatchGroupEnd =
> - batchGroupBase(CompactPtrBase, BG.GroupId) + GroupSize;
> - const uptr AllocatedGroupSize = AllocatedUserEnd >= BatchGroupEnd
> - ? BatchGroupEnd - BatchGroupBase
> - : AllocatedUserEnd - BatchGroupBase;
> - const uptr BatchGroupUsedEnd = BatchGroupBase + AllocatedGroupSize;
> + const uptr BatchGroupUsedEnd = BatchGroupBeg + AllocatedGroupSize;
> const bool BlockAlignedWithUsedEnd =
> (BatchGroupUsedEnd - Region->RegionBeg) % BlockSize == 0;
>
> @@ -968,15 +870,12 @@ template <typename Config> class SizeClassAllocator64 {
> if (!BlockAlignedWithUsedEnd)
> ++MaxContainedBlocks;
>
> - const uptr NumBlocks = (BG.Batches.size() - 1) * BG.MaxCachedPerBatch +
> - BG.Batches.front()->getCount();
> -
> if (NumBlocks == MaxContainedBlocks) {
> for (const auto &It : BG.Batches)
> for (u16 I = 0; I < It.getCount(); ++I)
> DCHECK_EQ(compactPtrGroup(It.get(I)), BG.GroupId);
>
> - Context.markRangeAsAllCounted(BatchGroupBase, BatchGroupUsedEnd,
> + Context.markRangeAsAllCounted(BatchGroupBeg, BatchGroupUsedEnd,
> Region->RegionBeg);
> } else {
> DCHECK_LT(NumBlocks, MaxContainedBlocks);
> @@ -987,7 +886,8 @@ template <typename Config> class SizeClassAllocator64 {
> }
> }
>
> - DCHECK(Context.hasBlockMarked());
> + if (!Context.hasBlockMarked())
> + return 0;
>
> auto SkipRegion = [](UNUSED uptr RegionIndex) { return false; };
> releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
> @@ -999,55 +899,6 @@ template <typename Config> class SizeClassAllocator64 {
> Region->ReleaseInfo.LastReleasedBytes = Recorder.getReleasedBytes();
> }
> Region->ReleaseInfo.LastReleaseAtNs = getMonotonicTime();
> -
> - // Merge GroupToRelease back to the Region::FreeList. Note that both
> - // `Region->FreeList` and `GroupToRelease` are sorted.
> - for (BatchGroup *BG = Region->FreeList.front(), *Prev = nullptr;;) {
> - if (BG == nullptr || GroupToRelease.empty()) {
> - if (!GroupToRelease.empty())
> - Region->FreeList.append_back(&GroupToRelease);
> - break;
> - }
> -
> - DCHECK_NE(BG->GroupId, GroupToRelease.front()->GroupId);
> -
> - if (BG->GroupId < GroupToRelease.front()->GroupId) {
> - Prev = BG;
> - BG = BG->Next;
> - continue;
> - }
> -
> - // At here, the `BG` is the first BatchGroup with GroupId larger than the
> - // first element in `GroupToRelease`. We need to insert
> - // `GroupToRelease::front()` (which is `Cur` below) before `BG`.
> - //
> - // 1. If `Prev` is nullptr, we simply push `Cur` to the front of
> - // FreeList.
> - // 2. Otherwise, use `insert()` which inserts an element next to `Prev`.
> - //
> - // Afterwards, we don't need to advance `BG` because the order between
> - // `BG` and the new `GroupToRelease::front()` hasn't been checked.
> - BatchGroup *Cur = GroupToRelease.front();
> - GroupToRelease.pop_front();
> - if (Prev == nullptr)
> - Region->FreeList.push_front(Cur);
> - else
> - Region->FreeList.insert(Prev, Cur);
> - DCHECK_EQ(Cur->Next, BG);
> - Prev = Cur;
> - }
> -
> - DCHECK_EQ(Region->FreeList.size(), NumberOfBatchGroups);
> - (void)NumberOfBatchGroups;
> -
> - if (SCUDO_DEBUG) {
> - BatchGroup *Prev = Region->FreeList.front();
> - for (BatchGroup *Cur = Prev->Next; Cur != nullptr;
> - Prev = Cur, Cur = Cur->Next) {
> - CHECK_LT(Prev->GroupId, Cur->GroupId);
> - }
> - }
> -
> return Recorder.getReleasedBytes();
> }
> };
>
> diff --git a/compiler-rt/lib/scudo/standalone/release.h b/compiler-rt/lib/scudo/standalone/release.h
> index d29d1c1f53f10..733326db7b81f 100644
> --- a/compiler-rt/lib/scudo/standalone/release.h
> +++ b/compiler-rt/lib/scudo/standalone/release.h
> @@ -259,10 +259,10 @@ template <class ReleaseRecorderT> class FreePagesRangeTracker {
> };
>
> struct PageReleaseContext {
> - PageReleaseContext(uptr BlockSize, uptr RegionSize, uptr NumberOfRegions,
> - uptr ReleaseSize, uptr ReleaseOffset = 0)
> - : BlockSize(BlockSize), RegionSize(RegionSize),
> - NumberOfRegions(NumberOfRegions) {
> + PageReleaseContext(uptr BlockSize, uptr RegionSize, uptr NumberOfRegions) :
> + BlockSize(BlockSize),
> + RegionSize(RegionSize),
> + NumberOfRegions(NumberOfRegions) {
> PageSize = getPageSizeCached();
> if (BlockSize <= PageSize) {
> if (PageSize % BlockSize == 0) {
> @@ -294,20 +294,10 @@ struct PageReleaseContext {
> }
> }
>
> - // TODO: For multiple regions, it's more complicated to support partial
> - // region marking (which includes the complexity of how to handle the last
> - // block in a region). We may consider this after markFreeBlocks() accepts
> - // only free blocks from the same region.
> - if (NumberOfRegions != 1) {
> - DCHECK_EQ(ReleaseSize, RegionSize);
> - DCHECK_EQ(ReleaseOffset, 0U);
> - }
> -
> - PagesCount = roundUp(ReleaseSize, PageSize) / PageSize;
> + PagesCount = roundUp(RegionSize, PageSize) / PageSize;
> PageSizeLog = getLog2(PageSize);
> - RoundedRegionSize = roundUp(RegionSize, PageSize);
> + RoundedRegionSize = PagesCount << PageSizeLog;
> RoundedSize = NumberOfRegions * RoundedRegionSize;
> - ReleasePageOffset = ReleaseOffset >> PageSizeLog;
> }
>
> // PageMap is lazily allocated when markFreeBlocks() is invoked.
> @@ -374,7 +364,7 @@ struct PageReleaseContext {
> uptr NumBlocksInFirstPage =
> (FromInRegion + PageSize - FirstBlockInRange + BlockSize - 1) /
> BlockSize;
> - PageMap.incN(RegionIndex, getPageIndex(FromInRegion),
> + PageMap.incN(RegionIndex, FromInRegion >> PageSizeLog,
> NumBlocksInFirstPage);
> FromInRegion = roundUp(FromInRegion + 1, PageSize);
> }
> @@ -402,8 +392,8 @@ struct PageReleaseContext {
> // The last block is not aligned to `To`, we need to increment the
> // counter of `next page` by 1.
> if (LastBlockInRange + BlockSize != ToInRegion) {
> - PageMap.incRange(RegionIndex, getPageIndex(ToInRegion),
> - getPageIndex(LastBlockInRange + BlockSize - 1));
> + PageMap.incRange(RegionIndex, ToInRegion >> PageSizeLog,
> + (LastBlockInRange + BlockSize - 1) >> PageSizeLog);
> }
> } else {
> ToInRegion = RegionSize;
> @@ -412,8 +402,8 @@ struct PageReleaseContext {
> // After handling the first page and the last block, it's safe to mark any
> // page in between the range [From, To).
> if (FromInRegion < ToInRegion) {
> - PageMap.setAsAllCountedRange(RegionIndex, getPageIndex(FromInRegion),
> - getPageIndex(ToInRegion - 1));
> + PageMap.setAsAllCountedRange(RegionIndex, FromInRegion >> PageSizeLog,
> + (ToInRegion - 1) >> PageSizeLog);
> }
> }
>
> @@ -422,19 +412,6 @@ struct PageReleaseContext {
> DecompactPtrT DecompactPtr, uptr Base) {
> ensurePageMapAllocated();
>
> - const uptr LastBlockInRegion = ((RegionSize / BlockSize) - 1U) * BlockSize;
> -
> - // The last block in a region may not use the entire page, so if it's free,
> - // we mark the following "pretend" memory block(s) as free.
> - auto markLastBlock = [this, LastBlockInRegion](const uptr RegionIndex) {
> - uptr PInRegion = LastBlockInRegion + BlockSize;
> - while (PInRegion < RoundedRegionSize) {
> - PageMap.incRange(RegionIndex, getPageIndex(PInRegion),
> - getPageIndex(PInRegion + BlockSize - 1));
> - PInRegion += BlockSize;
> - }
> - };
> -
> // Iterate over free chunks and count how many free chunks affect each
> // allocated page.
> if (BlockSize <= PageSize && PageSize % BlockSize == 0) {
> @@ -446,14 +423,14 @@ struct PageReleaseContext {
> continue;
> const uptr RegionIndex = NumberOfRegions == 1U ? 0 : P / RegionSize;
> const uptr PInRegion = P - RegionIndex * RegionSize;
> - PageMap.inc(RegionIndex, getPageIndex(PInRegion));
> - if (PInRegion == LastBlockInRegion)
> - markLastBlock(RegionIndex);
> + PageMap.inc(RegionIndex, PInRegion >> PageSizeLog);
> }
> }
> } else {
> // In all other cases chunks might affect more than one page.
> DCHECK_GE(RegionSize, BlockSize);
> + const uptr LastBlockInRegion =
> + ((RegionSize / BlockSize) - 1U) * BlockSize;
> for (const auto &It : FreeList) {
> for (u16 I = 0; I < It.getCount(); I++) {
> const uptr P = DecompactPtr(It.get(I)) - Base;
> @@ -461,23 +438,26 @@ struct PageReleaseContext {
> continue;
> const uptr RegionIndex = NumberOfRegions == 1U ? 0 : P / RegionSize;
> uptr PInRegion = P - RegionIndex * RegionSize;
> - PageMap.incRange(RegionIndex, getPageIndex(PInRegion),
> - getPageIndex(PInRegion + BlockSize - 1));
> - if (PInRegion == LastBlockInRegion)
> - markLastBlock(RegionIndex);
> + PageMap.incRange(RegionIndex, PInRegion >> PageSizeLog,
> + (PInRegion + BlockSize - 1) >> PageSizeLog);
> + // The last block in a region might straddle a page, so if it's
> + // free, we mark the following "pretend" memory block(s) as free.
> + if (PInRegion == LastBlockInRegion) {
> + PInRegion += BlockSize;
> + while (PInRegion < RoundedRegionSize) {
> + PageMap.incRange(RegionIndex, PInRegion >> PageSizeLog,
> + (PInRegion + BlockSize - 1) >> PageSizeLog);
> + PInRegion += BlockSize;
> + }
> + }
> }
> }
> }
> }
>
> - uptr getPageIndex(uptr P) { return (P >> PageSizeLog) - ReleasePageOffset; }
> -
> uptr BlockSize;
> uptr RegionSize;
> uptr NumberOfRegions;
> - // For partial region marking, some pages in front are not needed to be
> - // counted.
> - uptr ReleasePageOffset;
> uptr PageSize;
> uptr PagesCount;
> uptr PageSizeLog;
> @@ -499,7 +479,6 @@ releaseFreeMemoryToOS(PageReleaseContext &Context,
> const uptr BlockSize = Context.BlockSize;
> const uptr PagesCount = Context.PagesCount;
> const uptr NumberOfRegions = Context.NumberOfRegions;
> - const uptr ReleasePageOffset = Context.ReleasePageOffset;
> const uptr FullPagesBlockCountMax = Context.FullPagesBlockCountMax;
> const bool SameBlockCountPerPage = Context.SameBlockCountPerPage;
> RegionPageMap &PageMap = Context.PageMap;
> @@ -537,10 +516,6 @@ releaseFreeMemoryToOS(PageReleaseContext &Context,
> }
> uptr PrevPageBoundary = 0;
> uptr CurrentBoundary = 0;
> - if (ReleasePageOffset > 0) {
> - PrevPageBoundary = ReleasePageOffset * PageSize;
> - CurrentBoundary = roundUpSlow(PrevPageBoundary, BlockSize);
> - }
> for (uptr J = 0; J < PagesCount; J++) {
> const uptr PageBoundary = PrevPageBoundary + PageSize;
> uptr BlocksPerPage = Pn;
> @@ -572,8 +547,7 @@ releaseFreeMemoryToOS(const IntrusiveList<TransferBatchT> &FreeList,
> uptr RegionSize, uptr NumberOfRegions, uptr BlockSize,
> ReleaseRecorderT &Recorder, DecompactPtrT DecompactPtr,
> SkipRegionT SkipRegion) {
> - PageReleaseContext Context(BlockSize, /*ReleaseSize=*/RegionSize, RegionSize,
> - NumberOfRegions);
> + PageReleaseContext Context(BlockSize, RegionSize, NumberOfRegions);
> Context.markFreeBlocks(FreeList, DecompactPtr, Recorder.getBase());
> releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
> }
>
> diff --git a/compiler-rt/lib/scudo/standalone/tests/release_test.cpp b/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
> index 2e7382147006b..3cc20c4621d2c 100644
> --- a/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
> +++ b/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
> @@ -138,18 +138,15 @@ TEST(ScudoReleaseTest, FreePagesRangeTracker) {
>
> class ReleasedPagesRecorder {
> public:
> - ReleasedPagesRecorder() = default;
> - explicit ReleasedPagesRecorder(scudo::uptr Base) : Base(Base) {}
> std::set<scudo::uptr> ReportedPages;
>
> void releasePageRangeToOS(scudo::uptr From, scudo::uptr To) {
> const scudo::uptr PageSize = scudo::getPageSizeCached();
> for (scudo::uptr I = From; I < To; I += PageSize)
> - ReportedPages.insert(I + getBase());
> + ReportedPages.insert(I);
> }
>
> - scudo::uptr getBase() const { return Base; }
> - scudo::uptr Base = 0;
> + scudo::uptr getBase() const { return 0; }
> };
>
> // Simplified version of a TransferBatch.
> @@ -222,8 +219,7 @@ template <class SizeClassMap> void testReleaseFreeMemoryToOS() {
> ReleasedPagesRecorder Recorder;
> scudo::PageReleaseContext Context(BlockSize,
> /*RegionSize=*/MaxBlocks * BlockSize,
> - /*NumberOfRegions=*/1U,
> - /*ReleaseSize=*/MaxBlocks * BlockSize);
> + /*NumberOfRegions=*/1U);
> ASSERT_FALSE(Context.hasBlockMarked());
> Context.markFreeBlocks(FreeList, DecompactPtr, Recorder.getBase());
> ASSERT_TRUE(Context.hasBlockMarked());
> @@ -329,9 +325,8 @@ template <class SizeClassMap> void testPageMapMarkRange() {
> const scudo::uptr GroupEnd = GroupBeg + GroupSize;
>
> scudo::PageReleaseContext Context(BlockSize, RegionSize,
> - /*NumberOfRegions=*/1U,
> - /*ReleaseSize=*/RegionSize);
> - Context.markRangeAsAllCounted(GroupBeg, GroupEnd, /*Base=*/0U);
> + /*NumberOfRegions=*/1U);
> + Context.markRangeAsAllCounted(GroupBeg, GroupEnd, /*Base=*/0);
>
> scudo::uptr FirstBlock =
> ((GroupBeg + BlockSize - 1) / BlockSize) * BlockSize;
> @@ -399,142 +394,13 @@ template <class SizeClassMap> void testPageMapMarkRange() {
>
> // Release the entire region. This is to ensure the last page is counted.
> scudo::PageReleaseContext Context(BlockSize, RegionSize,
> - /*NumberOfRegions=*/1U,
> - /*ReleaseSize=*/RegionSize);
> + /*NumberOfRegions=*/1U);
> Context.markRangeAsAllCounted(/*From=*/0U, /*To=*/RegionSize, /*Base=*/0);
> for (scudo::uptr Page = 0; Page < RoundedRegionSize / PageSize; ++Page)
> EXPECT_TRUE(Context.PageMap.isAllCounted(/*Region=*/0, Page));
> } // Iterate each size class
> }
>
> -template <class SizeClassMap> void testReleasePartialRegion() {
> - typedef FreeBatch<SizeClassMap> Batch;
> - const scudo::uptr PageSize = scudo::getPageSizeCached();
> - const scudo::uptr ReleaseBase = PageSize;
> - const scudo::uptr BasePageOffset = ReleaseBase / PageSize;
> -
> - for (scudo::uptr I = 1; I <= SizeClassMap::LargestClassId; I++) {
> - // In the following, we want to ensure the region includes at least 2 pages
> - // and we will release all the pages except the first one. The handling of
> - // the last block is tricky, so we always test the case that includes the
> - // last block.
> - const scudo::uptr BlockSize = SizeClassMap::getSizeByClassId(I);
> - const scudo::uptr RegionSize =
> - scudo::roundUpSlow(scudo::roundUp(BlockSize, PageSize) * 2, BlockSize) +
> - BlockSize;
> - const scudo::uptr RoundedRegionSize = scudo::roundUp(RegionSize, PageSize);
> -
> - scudo::SinglyLinkedList<Batch> FreeList;
> - FreeList.clear();
> -
> - // Skip the blocks in the first page and add the remaining.
> - std::vector<scudo::uptr> Pages(RoundedRegionSize / PageSize, 0);
> - for (scudo::uptr Block = scudo::roundUpSlow(PageSize, BlockSize);
> - Block + BlockSize <= RoundedRegionSize; Block += BlockSize) {
> - for (scudo::uptr Page = Block / PageSize;
> - Page <= (Block + BlockSize - 1) / PageSize; ++Page) {
> - ASSERT_LT(Page, Pages.size());
> - ++Pages[Page];
> - }
> - }
> -
> - // This follows the logic how we count the last page. It should be
> - // consistent with how markFreeBlocks() handles the last block.
> - if (RoundedRegionSize % BlockSize != 0)
> - ++Pages.back();
> -
> - Batch *CurrentBatch = nullptr;
> - for (scudo::uptr Block = scudo::roundUpSlow(PageSize, BlockSize);
> - Block < RegionSize; Block += BlockSize) {
> - if (CurrentBatch == nullptr ||
> - CurrentBatch->getCount() == Batch::MaxCount) {
> - CurrentBatch = new Batch;
> - CurrentBatch->clear();
> - FreeList.push_back(CurrentBatch);
> - }
> - CurrentBatch->add(Block);
> - }
> -
> - auto VerifyReleaseToOs = [&](scudo::PageReleaseContext &Context) {
> - auto SkipRegion = [](UNUSED scudo::uptr RegionIndex) { return false; };
> - ReleasedPagesRecorder Recorder(ReleaseBase);
> - releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
> - const scudo::uptr FirstBlock = scudo::roundUpSlow(PageSize, BlockSize);
> -
> - for (scudo::uptr P = 0; P < RoundedRegionSize; P += PageSize) {
> - if (P < FirstBlock) {
> - // If FirstBlock is not aligned with page boundary, the first touched
> - // page will not be released either.
> - EXPECT_TRUE(Recorder.ReportedPages.find(P) ==
> - Recorder.ReportedPages.end());
> - } else {
> - EXPECT_TRUE(Recorder.ReportedPages.find(P) !=
> - Recorder.ReportedPages.end());
> - }
> - }
> - };
> -
> - // Test marking by visiting each block.
> - {
> - auto DecompactPtr = [](scudo::uptr P) { return P; };
> - scudo::PageReleaseContext Context(
> - BlockSize, RegionSize, /*NumberOfRegions=*/1U,
> - /*ReleaseSize=*/RegionSize - PageSize, ReleaseBase);
> - Context.markFreeBlocks(FreeList, DecompactPtr, /*Base=*/0U);
> - for (const Batch &It : FreeList) {
> - for (scudo::u16 I = 0; I < It.getCount(); I++) {
> - scudo::uptr Block = It.get(I);
> - for (scudo::uptr Page = Block / PageSize;
> - Page <= (Block + BlockSize - 1) / PageSize; ++Page) {
> - EXPECT_EQ(Pages[Page], Context.PageMap.get(/*Region=*/0U,
> - Page - BasePageOffset));
> - }
> - }
> - }
> -
> - VerifyReleaseToOs(Context);
> - }
> -
> - // Test range marking.
> - {
> - scudo::PageReleaseContext Context(
> - BlockSize, RegionSize, /*NumberOfRegions=*/1U,
> - /*ReleaseSize=*/RegionSize - PageSize, ReleaseBase);
> - Context.markRangeAsAllCounted(ReleaseBase, RegionSize, /*Base=*/0U);
> - for (scudo::uptr Page = ReleaseBase / PageSize;
> - Page < RoundedRegionSize / PageSize; ++Page) {
> - if (Context.PageMap.get(/*Region=*/0, Page - BasePageOffset) !=
> - Pages[Page]) {
> - EXPECT_TRUE(Context.PageMap.isAllCounted(/*Region=*/0,
> - Page - BasePageOffset));
> - }
> - }
> -
> - VerifyReleaseToOs(Context);
> - }
> -
> - // Check the buffer size of PageMap.
> - {
> - scudo::PageReleaseContext Full(BlockSize, RegionSize,
> - /*NumberOfRegions=*/1U,
> - /*ReleaseSize=*/RegionSize);
> - Full.ensurePageMapAllocated();
> - scudo::PageReleaseContext Partial(
> - BlockSize, RegionSize, /*NumberOfRegions=*/1U,
> - /*ReleaseSize=*/RegionSize - PageSize, ReleaseBase);
> - Partial.ensurePageMapAllocated();
> -
> - EXPECT_GE(Full.PageMap.getBufferSize(), Partial.PageMap.getBufferSize());
> - }
> -
> - while (!FreeList.empty()) {
> - CurrentBatch = FreeList.front();
> - FreeList.pop_front();
> - delete CurrentBatch;
> - }
> - } // Iterate each size class
> -}
> -
> TEST(ScudoReleaseTest, ReleaseFreeMemoryToOSDefault) {
> testReleaseFreeMemoryToOS<scudo::DefaultSizeClassMap>();
> }
> @@ -553,10 +419,3 @@ TEST(ScudoReleaseTest, PageMapMarkRange) {
> testPageMapMarkRange<scudo::FuchsiaSizeClassMap>();
> testPageMapMarkRange<scudo::SvelteSizeClassMap>();
> }
> -
> -TEST(ScudoReleaseTest, ReleasePartialRegion) {
> - testReleasePartialRegion<scudo::DefaultSizeClassMap>();
> - testReleasePartialRegion<scudo::AndroidSizeClassMap>();
> - testReleasePartialRegion<scudo::FuchsiaSizeClassMap>();
> - testReleasePartialRegion<scudo::SvelteSizeClassMap>();
> -}
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list