[scudo] Reduce unsuccessful attempts of page releasing #110583
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We introduce a new strategy to track how many bytes are not released because of the contraint of release interval. This will change the `TryReleaseThreshold` adaptively so that we avoid releasing the same pages multiple times (and wasting time on the case of no pages to release). On Android, the number of release attempts decreases 33% (572 to 382) and the worst case drops from 251 to 33. At the same time, it maintains almost the same RSS usage (with some improvements as well). Note that in this CL, this is only applied to non small blocks. We will bring the strategy to all the size classes later.
llvmbot
added
compiler-rt
compiler-rt:scudo
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (ChiaHungDuan) ChangesWe introduce a new strategy to track how many bytes are not released because of the contraint of release interval. This will change the On Android, the number of release attempts decreases 33% (572 to 382) and the worst case drops from 251 to 33. At the same time, it maintains almost the same RSS usage (with some improvements as well). Note that in this CL, this is only applied to non small blocks. We will bring the strategy to all the size classes later. Full diff: https://github.com/llvm/llvm-project/pull/110583.diff 1 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index a3876479ec8158..2e50ba8142126b 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -532,6 +532,11 @@ template <typename Config> class SizeClassAllocator64 {
uptr BytesInFreeListAtLastCheckpoint;
uptr RangesReleased;
uptr LastReleasedBytes;
+ // The minimum size of pushed blocks to trigger page release.
+ uptr TryReleaseThreshold;
+ // The number of bytes not triggering `releaseToOSMaybe()` because of
+ // the length of release interval.
+ uptr PendingPushedBytesDelta;
u64 LastReleaseAtNs;
};
@@ -560,8 +565,6 @@ template <typename Config> class SizeClassAllocator64 {
u32 RandState GUARDED_BY(MMLock) = 0;
BlocksInfo FreeListInfo GUARDED_BY(FLLock);
PagesInfo MemMapInfo GUARDED_BY(MMLock);
- // The minimum size of pushed blocks to trigger page release.
- uptr TryReleaseThreshold GUARDED_BY(MMLock) = 0;
ReleaseToOsInfo ReleaseInfo GUARDED_BY(MMLock) = {};
bool Exhausted GUARDED_BY(MMLock) = false;
bool isPopulatingFreeList GUARDED_BY(FLLock) = false;
@@ -610,9 +613,8 @@ template <typename Config> class SizeClassAllocator64 {
return BlockSize < PageSize / 16U;
}
- ALWAYS_INLINE static bool isLargeBlock(uptr BlockSize) {
- const uptr PageSize = getPageSizeCached();
- return BlockSize > PageSize;
+ ALWAYS_INLINE uptr getMinReleaseAttemptSize(uptr BlockSize) {
+ return roundUp(BlockSize, getPageSizeCached());
}
ALWAYS_INLINE void initRegion(RegionInfo *Region, uptr ClassId,
@@ -631,12 +633,16 @@ template <typename Config> class SizeClassAllocator64 {
(getRandomModN(&Region->RandState, 16) + 1) * PageSize;
}
+ const uptr BlockSize = getSizeByClassId(ClassId);
// Releasing small blocks is expensive, set a higher threshold to avoid
// frequent page releases.
- if (isSmallBlock(getSizeByClassId(ClassId)))
- Region->TryReleaseThreshold = PageSize * SmallerBlockReleasePageDelta;
- else
- Region->TryReleaseThreshold = PageSize;
+ if (isSmallBlock(BlockSize)) {
+ Region->ReleaseInfo.TryReleaseThreshold =
+ PageSize * SmallerBlockReleasePageDelta;
+ } else {
+ Region->ReleaseInfo.TryReleaseThreshold =
+ getMinReleaseAttemptSize(BlockSize);
+ }
}
void pushBatchClassBlocks(RegionInfo *Region, CompactPtrT *Array, u32 Size)
@@ -1245,6 +1251,7 @@ template <typename Config> class SizeClassAllocator64 {
uptr BytesInFreeList;
const uptr AllocatedUserEnd =
Region->MemMapInfo.AllocatedUser + Region->RegionBeg;
+ uptr RegionPushedBytesDelta = 0;
SinglyLinkedList<BatchGroupT> GroupsToRelease;
{
@@ -1267,6 +1274,12 @@ template <typename Config> class SizeClassAllocator64 {
return 0;
}
+ // Given that we will unlock the freelist for block operations, cache the
+ // value here so that when we are adapting the `TryReleaseThreshold`
+ // later, we are using the right metric.
+ RegionPushedBytesDelta =
+ BytesInFreeList - Region->ReleaseInfo.BytesInFreeListAtLastCheckpoint;
+
// ==================================================================== //
// 2. Determine which groups can release the pages. Use a heuristic to
// gather groups that are candidates for doing a release.
@@ -1310,12 +1323,36 @@ template <typename Config> class SizeClassAllocator64 {
auto SkipRegion = [](UNUSED uptr RegionIndex) { return false; };
releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
if (Recorder.getReleasedRangesCount() > 0) {
+ // This is the case that we didn't hit the release threshold but it has
+ // been past a certain period of time. Thus we try to release some pages
+ // and if it does release some additional pages, it's hint that we are
+ // able to lower the threshold.
+ // TODO(chiahungduan): Apply the same adjustment strategy to small blocks.
+ if (!isSmallBlock(BlockSize)) {
+ if (RegionPushedBytesDelta < Region->ReleaseInfo.TryReleaseThreshold &&
+ Recorder.getReleasedBytes() >
+ Region->ReleaseInfo.LastReleasedBytes +
+ getMinReleaseAttemptSize(BlockSize)) {
+ Region->ReleaseInfo.TryReleaseThreshold =
+ Max(Region->ReleaseInfo.TryReleaseThreshold / 2,
+ getMinReleaseAttemptSize(BlockSize));
+ }
+ }
+
Region->ReleaseInfo.BytesInFreeListAtLastCheckpoint = BytesInFreeList;
Region->ReleaseInfo.RangesReleased += Recorder.getReleasedRangesCount();
Region->ReleaseInfo.LastReleasedBytes = Recorder.getReleasedBytes();
}
Region->ReleaseInfo.LastReleaseAtNs = getMonotonicTimeFast();
+ if (Region->ReleaseInfo.PendingPushedBytesDelta > 0) {
+ Region->ReleaseInfo.TryReleaseThreshold +=
+ Region->ReleaseInfo.PendingPushedBytesDelta / 2;
+ Region->ReleaseInfo.TryReleaseThreshold = Min<uptr>(
+ Region->ReleaseInfo.TryReleaseThreshold, (1UL << GroupSizeLog) / 2);
+ Region->ReleaseInfo.PendingPushedBytesDelta = 0;
+ }
+
// ====================================================================== //
// 5. Merge the `GroupsToRelease` back to the freelist.
// ====================================================================== //
@@ -1329,8 +1366,6 @@ template <typename Config> class SizeClassAllocator64 {
REQUIRES(Region->MMLock, Region->FLLock) {
DCHECK_GE(Region->FreeListInfo.PoppedBlocks,
Region->FreeListInfo.PushedBlocks);
- const uptr PageSize = getPageSizeCached();
-
// Always update `BytesInFreeListAtLastCheckpoint` with the smallest value
// so that we won't underestimate the releasable pages. For example, the
// following is the region usage,
@@ -1354,34 +1389,40 @@ template <typename Config> class SizeClassAllocator64 {
const uptr RegionPushedBytesDelta =
BytesInFreeList - Region->ReleaseInfo.BytesInFreeListAtLastCheckpoint;
- if (RegionPushedBytesDelta < PageSize)
- return false;
-
- // 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 (isSmallBlock(BlockSize) && ReleaseType == ReleaseToOS::Normal)
- if (RegionPushedBytesDelta < Region->TryReleaseThreshold)
- return false;
if (ReleaseType == ReleaseToOS::Normal) {
- const s32 IntervalMs = atomic_load_relaxed(&ReleaseToOsIntervalMs);
- if (IntervalMs < 0)
+ if (RegionPushedBytesDelta < Region->ReleaseInfo.TryReleaseThreshold / 2)
+ return false;
+
+ const u64 IntervalNs =
+ static_cast<u64>(atomic_load_relaxed(&ReleaseToOsIntervalMs)) *
+ 1000000;
+ if (IntervalNs < 0)
return false;
- // The constant 8 here is selected from profiling some apps and the number
- // of unreleased pages in the large size classes is around 16 pages or
- // more. Choose half of it as a heuristic and which also avoids page
- // release every time for every pushBlocks() attempt by large blocks.
- const bool ByPassReleaseInterval =
- isLargeBlock(BlockSize) && RegionPushedBytesDelta > 8 * PageSize;
- if (!ByPassReleaseInterval) {
- if (Region->ReleaseInfo.LastReleaseAtNs +
- static_cast<u64>(IntervalMs) * 1000000 >
- getMonotonicTimeFast()) {
- // Memory was returned recently.
+ const u64 CurTimeNs = getMonotonicTimeFast();
+ const u64 DiffSinceLastReleaseNs =
+ CurTimeNs - Region->ReleaseInfo.LastReleaseAtNs;
+
+ // When `RegionPushedBytesDelta` is more than half of
+ // `TryReleaseThreshold` and the last release was happened 2 release
+ // interval before, we will try to see if there's any chance to release
+ // some memory. If we do release additional memory, we decrease the
+ // threshold accodingly.
+ if (RegionPushedBytesDelta < Region->ReleaseInfo.TryReleaseThreshold) {
+ if (DiffSinceLastReleaseNs < 2 * IntervalNs)
return false;
- }
+ } else if (DiffSinceLastReleaseNs < IntervalNs) {
+ // `TryReleaseThreshold` will be adjusted according to how many bytes
+ // are not released due to the interval constraint and it will be
+ // increased so that we are likely to release them all in the next time.
+ // TODO(chiahungduan): Apply the same adjustment strategy to small
+ // blocks.
+ if (!isSmallBlock(BlockSize))
+ Region->ReleaseInfo.PendingPushedBytesDelta = RegionPushedBytesDelta;
+
+ // Memory was returned recently.
+ return false;
}
} // if (ReleaseType == ReleaseToOS::Normal)
@@ -1397,10 +1438,10 @@ template <typename Config> class SizeClassAllocator64 {
SinglyLinkedList<BatchGroupT> GroupsToRelease;
// We are examining each group and will take the minimum distance to the
- // release threshold as the next Region::TryReleaseThreshold(). Note that if
- // the size of free blocks has reached the release threshold, the distance
- // to the next release will be PageSize * SmallerBlockReleasePageDelta. See
- // the comment on `SmallerBlockReleasePageDelta` for more details.
+ // release threshold as the next `TryReleaseThreshold`. Note that if the
+ // size of free blocks has reached the release threshold, the distance to
+ // the next release will be PageSize * SmallerBlockReleasePageDelta. See the
+ // comment on `SmallerBlockReleasePageDelta` for more details.
uptr MinDistToThreshold = GroupSize;
for (BatchGroupT *BG = Region->FreeListInfo.BlockList.front(),
@@ -1548,7 +1589,7 @@ template <typename Config> class SizeClassAllocator64 {
// back to normal.
if (MinDistToThreshold == GroupSize)
MinDistToThreshold = PageSize * SmallerBlockReleasePageDelta;
- Region->TryReleaseThreshold = MinDistToThreshold;
+ Region->ReleaseInfo.TryReleaseThreshold = MinDistToThreshold;
}
return GroupsToRelease;
|
cferris1000
reviewed
Oct 1, 2024
| Region->ReleaseInfo.LastReleasedBytes + | ||
| getMinReleaseAttemptSize(BlockSize)) { | ||
| Region->ReleaseInfo.TryReleaseThreshold = | ||
| Max(Region->ReleaseInfo.TryReleaseThreshold / 2, |
There was a problem hiding this comment.
Can you add a comment about why the divide by 2 is used?
| Region->ReleaseInfo.BytesInFreeListAtLastCheckpoint = BytesInFreeList; | ||
| Region->ReleaseInfo.RangesReleased += Recorder.getReleasedRangesCount(); | ||
| Region->ReleaseInfo.LastReleasedBytes = Recorder.getReleasedBytes(); | ||
| } | ||
| Region->ReleaseInfo.LastReleaseAtNs = getMonotonicTimeFast(); | ||
|
|
||
| if (Region->ReleaseInfo.PendingPushedBytesDelta > 0) { | ||
| Region->ReleaseInfo.TryReleaseThreshold += |
There was a problem hiding this comment.
Can you add a comment here too about why these numbers were chosen?
| CurTimeNs - Region->ReleaseInfo.LastReleaseAtNs; | ||
|
|
||
| // When `RegionPushedBytesDelta` is more than half of | ||
| // `TryReleaseThreshold` and the last release was happened 2 release |
There was a problem hiding this comment.
This is a bit confusing and doesn't completely match the code below. I'm not sure you need this comment since it's mostly what's happening and the code seems relatively self-explanatory. However, it would be nice to know why you chose the 2 * IntervalNs value.
There was a problem hiding this comment.
I guess the code structure doesn't give the idea clear enough. I added more comments. Let me know if it looks better
Right, I was thinking the same thing. I'm thinking of some refactoring the use of |
cferris1000
reviewed
Oct 1, 2024
| CurTimeNs - Region->ReleaseInfo.LastReleaseAtNs; | ||
|
|
||
| // At here, `RegionPushedBytesDelta` is more than half of | ||
| // `TryReleaseThreshold`. If the last release was happened 2 release |
There was a problem hiding this comment.
last release was happened -> last release happened
| // release some memory even it doesn't exceed the threshold. | ||
| if (RegionPushedBytesDelta < Region->ReleaseInfo.TryReleaseThreshold) { | ||
| // We want the threshold to have a shorter response time to the variant | ||
| // memory usage patterns. By having some experiments (which was done |
There was a problem hiding this comment.
According to data collected during experiments (which were done with ....)
Or something similar.
| return false; | ||
| } | ||
| } else if (DiffSinceLastReleaseNs < IntervalNs) { | ||
| // In this case, we are over the threshold but we just did some page |
There was a problem hiding this comment.
we just did some page -> we just released some pages in the same release interval.
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
We introduce a new strategy to track how many bytes are not released because of the contraint of release interval. This will change the `TryReleaseThreshold` adaptively so that we avoid releasing the same pages multiple times (and wasting time on the case of no pages to release). On Android, the number of release attempts decreases 33% (572 to 382) and the worst case drops from 251 to 33. At the same time, it maintains almost the same RSS usage (with some improvements as well). Note that in this CL, this is only applied to non small blocks. We will bring the strategy to all the size classes later.
xgupta
pushed a commit
to xgupta/llvm-project
that referenced
this pull request
Oct 4, 2024
We introduce a new strategy to track how many bytes are not released because of the contraint of release interval. This will change the `TryReleaseThreshold` adaptively so that we avoid releasing the same pages multiple times (and wasting time on the case of no pages to release). On Android, the number of release attempts decreases 33% (572 to 382) and the worst case drops from 251 to 33. At the same time, it maintains almost the same RSS usage (with some improvements as well). Note that in this CL, this is only applied to non small blocks. We will bring the strategy to all the size classes later.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We introduce a new strategy to track how many bytes are not released because of the contraint of release interval. This will change the
TryReleaseThresholdadaptively so that we avoid releasing the same pages multiple times (and wasting time on the case of no pages to release).On Android, the number of release attempts decreases 33% (572 to 382) and the worst case drops from 251 to 33. At the same time, it maintains almost the same RSS usage (with some improvements as well).
Note that in this CL, this is only applied to non small blocks. We will bring the strategy to all the size classes later.