[compiler-rt] 1cf1b36 - Revert "[scudo] Support partial page releasing"

Kamau Bridgeman via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 14 13:12:27 PDT 2022


Author: Kamau Bridgeman
Date: 2022-10-14T15:11:44-05:00
New Revision: 1cf1b36112b3d475d443909881b02799b548e6b8

URL: https://github.com/llvm/llvm-project/commit/1cf1b36112b3d475d443909881b02799b548e6b8
DIFF: https://github.com/llvm/llvm-project/commit/1cf1b36112b3d475d443909881b02799b548e6b8.diff

LOG: Revert "[scudo] Support partial page releasing"

This reverts commit 9c26f51f5e178ac0fda98419e3a61d205d3b58b1.

Added: 
    

Modified: 
    compiler-rt/lib/scudo/standalone/local_cache.h
    compiler-rt/lib/scudo/standalone/primary32.h
    compiler-rt/lib/scudo/standalone/primary64.h
    compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/scudo/standalone/local_cache.h b/compiler-rt/lib/scudo/standalone/local_cache.h
index 6e5b09fdb709e..7f270472d70ff 100644
--- a/compiler-rt/lib/scudo/standalone/local_cache.h
+++ b/compiler-rt/lib/scudo/standalone/local_cache.h
@@ -65,13 +65,6 @@ template <class SizeClassAllocator> struct SizeClassAllocatorLocalCache {
     uptr GroupId;
     // Cache value of TransferBatch::getMaxCached()
     u16 MaxCachedPerBatch;
-    // Number of blocks pushed into this group. This is an increment-only
-    // counter.
-    uptr PushedBlocks;
-    // This is used to track how many blocks are pushed since last time we
-    // checked `PushedBlocks`. It's useful for page releasing to determine the
-    // usage of a BatchGroup.
-    uptr PushedBlocksAtLastCheckpoint;
     // Blocks are managed by TransferBatch in a list.
     SinglyLinkedList<TransferBatch> Batches;
   };

diff  --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h
index fb6cb53392358..b411a3bd837a0 100644
--- a/compiler-rt/lib/scudo/standalone/primary32.h
+++ b/compiler-rt/lib/scudo/standalone/primary32.h
@@ -421,8 +421,6 @@ template <typename Config> class SizeClassAllocator32 {
 
       BG->GroupId = GroupId;
       BG->Batches.push_front(TB);
-      BG->PushedBlocks = 0;
-      BG->PushedBlocksAtLastCheckpoint = 0;
       BG->MaxCachedPerBatch =
           TransferBatch::getMaxCached(getSizeByClassId(ClassId));
 
@@ -448,8 +446,6 @@ template <typename Config> class SizeClassAllocator32 {
         CurBatch->appendFromArray(&Array[I], AppendSize);
         I += AppendSize;
       }
-
-      BG->PushedBlocks += Size;
     };
 
     BatchGroup *Cur = Sci->FreeList.front();
@@ -656,13 +652,16 @@ template <typename Config> class SizeClassAllocator32 {
     if (BytesPushed < PageSize)
       return 0; // Nothing new to release.
 
-    const bool CheckDensity = BlockSize < PageSize / 16U;
     // Releasing smaller blocks is expensive, so we want to make sure that a
     // significant amount of bytes are free, and that there has been a good
     // amount of batches pushed to the freelist before attempting to release.
-    if (CheckDensity) {
+    if (BlockSize < PageSize / 16U) {
       if (!Force && BytesPushed < Sci->AllocatedUser / 16U)
         return 0;
+      // We want 8x% to 9x% free bytes (the larger the block, the lower the %).
+      if ((BytesInFreeList * 100U) / Sci->AllocatedUser <
+          (100U - 1U - BlockSize / 16U))
+        return 0;
     }
 
     if (!Force) {
@@ -683,47 +682,17 @@ template <typename Config> class SizeClassAllocator32 {
     uptr TotalReleasedBytes = 0;
     const uptr Base = First * RegionSize;
     const uptr NumberOfRegions = Last - First + 1U;
-    const uptr GroupSize = (1U << GroupSizeLog);
-    const uptr CurRegionGroupId =
-        compactPtrGroup(compactPtr(ClassId, Sci->CurrentRegion));
-
     ReleaseRecorder Recorder(Base);
-    PageReleaseContext Context(BlockSize, RegionSize, NumberOfRegions);
-
+    auto SkipRegion = [this, First, ClassId](uptr RegionIndex) {
+      return (PossibleRegions[First + RegionIndex] - 1U) != ClassId;
+    };
     auto DecompactPtr = [](CompactPtrT CompactPtr) {
       return reinterpret_cast<uptr>(CompactPtr);
     };
-    for (BatchGroup &BG : Sci->FreeList) {
-      const uptr PushedBytesDelta =
-          BG.PushedBlocks - BG.PushedBlocksAtLastCheckpoint;
-      if (PushedBytesDelta * BlockSize < PageSize)
-        continue;
-
-      uptr AllocatedGroupSize =
-          BG.GroupId == CurRegionGroupId ? Sci->CurrentRegionAllocated :
-                                           GroupSize;
-      if (AllocatedGroupSize == 0) continue;
-
-      const uptr NumBlocks = (BG.Batches.size() - 1) * BG.MaxCachedPerBatch +
-          BG.Batches.back()->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 allocated
-      // spaces.
-      if (CheckDensity && (BytesInBG * 100U) / AllocatedGroupSize <
-          (100U - 1U - BlockSize / 16U)) {
-        continue;
-      }
-
-      BG.PushedBlocksAtLastCheckpoint = BG.PushedBlocks;
-      // Note that we don't always visit blocks in each BatchGroup so that we
-      // may miss the chance of releasing certain pages that cross BatchGroups.
+    PageReleaseContext Context(BlockSize, RegionSize, NumberOfRegions);
+    for (BatchGroup &BG : Sci->FreeList)
       Context.markFreeBlocks(BG.Batches, DecompactPtr, Base);
-    }
 
-    auto SkipRegion = [this, First, ClassId](uptr RegionIndex) {
-      return (PossibleRegions[First + RegionIndex] - 1U) != ClassId;
-    };
     releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
 
     if (Recorder.getReleasedRangesCount() > 0) {

diff  --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index 2b6e71c8584ab..84b811a8b399d 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -370,9 +370,6 @@ template <typename Config> class SizeClassAllocator64 {
   static uptr compactPtrGroup(CompactPtrT CompactPtr) {
     return CompactPtr >> (GroupSizeLog - CompactPtrScale);
   }
-  static uptr batchGroupBase(uptr Base, uptr GroupId) {
-    return (GroupId << GroupSizeLog) + Base;
-  }
 
   // Push the blocks to their batch group. The layout will be like,
   //
@@ -427,8 +424,6 @@ template <typename Config> class SizeClassAllocator64 {
 
       BG->GroupId = GroupId;
       BG->Batches.push_front(TB);
-      BG->PushedBlocks = 0;
-      BG->PushedBlocksAtLastCheckpoint = 0;
       BG->MaxCachedPerBatch =
           TransferBatch::getMaxCached(getSizeByClassId(ClassId));
 
@@ -455,8 +450,6 @@ template <typename Config> class SizeClassAllocator64 {
         CurBatch->appendFromArray(&Array[I], AppendSize);
         I += AppendSize;
       }
-
-      BG->PushedBlocks += Size;
     };
 
     BatchGroup *Cur = Region->FreeList.front();
@@ -668,13 +661,16 @@ template <typename Config> class SizeClassAllocator64 {
     if (BytesPushed < PageSize)
       return 0; // Nothing new to release.
 
-    bool CheckDensity = BlockSize < PageSize / 16U;
     // Releasing smaller blocks is expensive, so we want to make sure that a
     // significant amount of bytes are free, and that there has been a good
     // amount of batches pushed to the freelist before attempting to release.
-    if (CheckDensity) {
+    if (BlockSize < PageSize / 16U) {
       if (!Force && BytesPushed < Region->AllocatedUser / 16U)
         return 0;
+      // We want 8x% to 9x% free bytes (the larger the block, the lower the %).
+      if ((BytesInFreeList * 100U) / Region->AllocatedUser <
+          (100U - 1U - BlockSize / 16U))
+        return 0;
     }
 
     if (!Force) {
@@ -688,46 +684,16 @@ 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, RegionSize, /*NumberOfRegions=*/1U);
-
     const uptr CompactPtrBase = getCompactPtrBaseByClassId(ClassId);
     auto DecompactPtr = [CompactPtrBase](CompactPtrT CompactPtr) {
       return decompactPtrInternal(CompactPtrBase, CompactPtr);
     };
-    for (BatchGroup &BG : Region->FreeList) {
-      const uptr PushedBytesDelta =
-          BG.PushedBlocks - BG.PushedBlocksAtLastCheckpoint;
-      if (PushedBytesDelta * BlockSize < PageSize)
-        continue;
-      const uptr BatchGroupEnd =
-          batchGroupBase(BG.GroupId, CompactPtrBase) + GroupSize;
-      const uptr AllocatedGroupSize =
-          AllocatedUserEnd >= BatchGroupEnd ? GroupSize :
-                                              AllocatedUserEnd - BatchGroupEnd;
-      if (AllocatedGroupSize == 0) continue;
-
-      const uptr NumBlocks = (BG.Batches.size() - 1) * BG.MaxCachedPerBatch +
-          BG.Batches.back()->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
-      // that this heuristic only applies when all the spaces in a BatchGroup
-      // are allocated.
-      if (CheckDensity && (BytesInBG * 100U) / AllocatedGroupSize <
-          (100U - 1U - BlockSize / 16U)) {
-        continue;
-      }
-
-      BG.PushedBlocksAtLastCheckpoint = BG.PushedBlocks;
-      // Note that we don't always visit blocks in each BatchGroup so that we
-      // may miss the chance of releasing certain pages that cross BatchGroups.
+    auto SkipRegion = [](UNUSED uptr RegionIndex) { return false; };
+    PageReleaseContext Context(BlockSize, RegionSize, /*NumberOfRegions=*/1U);
+    for (BatchGroup &BG : Region->FreeList)
       Context.markFreeBlocks(BG.Batches, DecompactPtr, Region->RegionBeg);
-    }
 
-    auto SkipRegion = [](UNUSED uptr RegionIndex) { return false; };
     releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
 
     if (Recorder.getReleasedRangesCount() > 0) {

diff  --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index 02a6e04cb758c..5b9d47151c7f5 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -506,7 +506,7 @@ struct DeathSizeClassConfig {
   static const scudo::uptr MinSizeLog = 10;
   static const scudo::uptr MidSizeLog = 10;
   static const scudo::uptr MaxSizeLog = 13;
-  static const scudo::u16 MaxNumCachedHint = 8;
+  static const scudo::u16 MaxNumCachedHint = 4;
   static const scudo::uptr MaxBytesCachedLog = 12;
   static const scudo::uptr SizeDelta = 0;
 };


        


More information about the llvm-commits mailing list