[scudo] Add the record of number of attempted page release #120497
Merged
Conversation
This file contains hidden or 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
This also removes the `RangesReleased` which doesn't give much insight to whether we should adjust the heuristic of doing page release.
llvmbot
added
compiler-rt
compiler-rt:scudo
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (ChiaHungDuan) ChangesThis also removes the Full diff: https://github.com/llvm/llvm-project/pull/120497.diff 3 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h
index 654b129d9f5471..2f37cc96c0d406 100644
--- a/compiler-rt/lib/scudo/standalone/primary32.h
+++ b/compiler-rt/lib/scudo/standalone/primary32.h
@@ -387,7 +387,7 @@ template <typename Config> class SizeClassAllocator32 {
struct ReleaseToOsInfo {
uptr BytesInFreeListAtLastCheckpoint;
- uptr RangesReleased;
+ uptr NumReleaseAttempted;
uptr LastReleasedBytes;
u64 LastReleaseAtNs;
};
@@ -880,14 +880,14 @@ template <typename Config> class SizeClassAllocator32 {
BytesInFreeList - Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint;
}
const uptr AvailableChunks = Sci->AllocatedUser / BlockSize;
- Str->append(" %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu "
- "inuse: %6zu avail: %6zu releases: %6zu last released: %6zuK "
- "latest pushed bytes: %6zuK\n",
- ClassId, getSizeByClassId(ClassId), Sci->AllocatedUser >> 10,
- Sci->FreeListInfo.PoppedBlocks, Sci->FreeListInfo.PushedBlocks,
- InUse, AvailableChunks, Sci->ReleaseInfo.RangesReleased,
- Sci->ReleaseInfo.LastReleasedBytes >> 10,
- PushedBytesDelta >> 10);
+ Str->append(
+ " %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu "
+ "inuse: %6zu avail: %6zu release count: %6zu last released: %6zuK "
+ "latest pushed bytes: %6zuK\n",
+ ClassId, getSizeByClassId(ClassId), Sci->AllocatedUser >> 10,
+ Sci->FreeListInfo.PoppedBlocks, Sci->FreeListInfo.PushedBlocks, InUse,
+ AvailableChunks, Sci->ReleaseInfo.NumReleaseAttempted,
+ Sci->ReleaseInfo.LastReleasedBytes >> 10, PushedBytesDelta >> 10);
}
void getSizeClassFragmentationInfo(SizeClassInfo *Sci, uptr ClassId,
@@ -972,6 +972,10 @@ template <typename Config> class SizeClassAllocator32 {
const uptr Base = First * RegionSize;
const uptr NumberOfRegions = Last - First + 1U;
+ // The following steps contribute to the majority time spent in page
+ // releasing thus we increment the counter here.
+ ++Sci->ReleaseInfo.NumReleaseAttempted;
+
// ==================================================================== //
// 2. Mark the free blocks and we can tell which pages are in-use by
// querying `PageReleaseContext`.
@@ -991,9 +995,8 @@ template <typename Config> class SizeClassAllocator32 {
};
releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
- if (Recorder.getReleasedRangesCount() > 0) {
+ if (Recorder.getReleasedBytes() > 0) {
Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint = BytesInFreeList;
- Sci->ReleaseInfo.RangesReleased += Recorder.getReleasedRangesCount();
Sci->ReleaseInfo.LastReleasedBytes = Recorder.getReleasedBytes();
TotalReleasedBytes += Sci->ReleaseInfo.LastReleasedBytes;
}
diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index e382e01f5d8e54..c99463d3946b0c 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -530,7 +530,7 @@ template <typename Config> class SizeClassAllocator64 {
struct ReleaseToOsInfo {
uptr BytesInFreeListAtLastCheckpoint;
- uptr RangesReleased;
+ uptr NumReleaseAttempted;
uptr LastReleasedBytes;
// The minimum size of pushed blocks to trigger page release.
uptr TryReleaseThreshold;
@@ -1141,17 +1141,18 @@ template <typename Config> class SizeClassAllocator64 {
BytesInFreeList - Region->ReleaseInfo.BytesInFreeListAtLastCheckpoint;
}
const uptr TotalChunks = Region->MemMapInfo.AllocatedUser / BlockSize;
- Str->append(
- "%s %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu "
- "inuse: %6zu total: %6zu releases: %6zu last "
- "released: %6zuK latest pushed bytes: %6zuK region: 0x%zx (0x%zx)\n",
- Region->Exhausted ? "E" : " ", ClassId, getSizeByClassId(ClassId),
- Region->MemMapInfo.MappedUser >> 10, Region->FreeListInfo.PoppedBlocks,
- Region->FreeListInfo.PushedBlocks, InUseBlocks, TotalChunks,
- Region->ReleaseInfo.RangesReleased,
- Region->ReleaseInfo.LastReleasedBytes >> 10,
- RegionPushedBytesDelta >> 10, Region->RegionBeg,
- getRegionBaseByClassId(ClassId));
+ Str->append("%s %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu "
+ "inuse: %6zu total: %6zu releases: %6zu last "
+ "release count: %6zuK latest pushed bytes: %6zuK region: 0x%zx "
+ "(0x%zx)\n",
+ Region->Exhausted ? "E" : " ", ClassId,
+ getSizeByClassId(ClassId), Region->MemMapInfo.MappedUser >> 10,
+ Region->FreeListInfo.PoppedBlocks,
+ Region->FreeListInfo.PushedBlocks, InUseBlocks, TotalChunks,
+ Region->ReleaseInfo.NumReleaseAttempted,
+ Region->ReleaseInfo.LastReleasedBytes >> 10,
+ RegionPushedBytesDelta >> 10, Region->RegionBeg,
+ getRegionBaseByClassId(ClassId));
}
void getRegionFragmentationInfo(RegionInfo *Region, uptr ClassId,
@@ -1322,7 +1323,7 @@ template <typename Config> class SizeClassAllocator64 {
Context.getReleaseOffset());
auto SkipRegion = [](UNUSED uptr RegionIndex) { return false; };
releaseFreeMemoryToOS(Context, Recorder, SkipRegion);
- if (Recorder.getReleasedRangesCount() > 0) {
+ if (Recorder.getReleasedBytes() > 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
@@ -1342,7 +1343,6 @@ template <typename Config> class SizeClassAllocator64 {
}
Region->ReleaseInfo.BytesInFreeListAtLastCheckpoint = BytesInFreeList;
- Region->ReleaseInfo.RangesReleased += Recorder.getReleasedRangesCount();
Region->ReleaseInfo.LastReleasedBytes = Recorder.getReleasedBytes();
}
Region->ReleaseInfo.LastReleaseAtNs = getMonotonicTimeFast();
diff --git a/compiler-rt/lib/scudo/standalone/release.h b/compiler-rt/lib/scudo/standalone/release.h
index 6353dafde31609..7a4912ec0b5a6f 100644
--- a/compiler-rt/lib/scudo/standalone/release.h
+++ b/compiler-rt/lib/scudo/standalone/release.h
@@ -22,8 +22,6 @@ template <typename MemMapT> class RegionReleaseRecorder {
RegionReleaseRecorder(MemMapT *RegionMemMap, uptr Base, uptr Offset = 0)
: RegionMemMap(RegionMemMap), Base(Base), Offset(Offset) {}
- uptr getReleasedRangesCount() const { return ReleasedRangesCount; }
-
uptr getReleasedBytes() const { return ReleasedBytes; }
uptr getBase() const { return Base; }
@@ -33,12 +31,10 @@ template <typename MemMapT> class RegionReleaseRecorder {
void releasePageRangeToOS(uptr From, uptr To) {
const uptr Size = To - From;
RegionMemMap->releasePagesToOS(getBase() + Offset + From, Size);
- ReleasedRangesCount++;
ReleasedBytes += Size;
}
private:
- uptr ReleasedRangesCount = 0;
uptr ReleasedBytes = 0;
MemMapT *RegionMemMap = nullptr;
uptr Base = 0;
@@ -52,8 +48,6 @@ class ReleaseRecorder {
ReleaseRecorder(uptr Base, uptr Offset = 0, MapPlatformData *Data = nullptr)
: Base(Base), Offset(Offset), Data(Data) {}
- uptr getReleasedRangesCount() const { return ReleasedRangesCount; }
-
uptr getReleasedBytes() const { return ReleasedBytes; }
uptr getBase() const { return Base; }
@@ -62,12 +56,10 @@ class ReleaseRecorder {
void releasePageRangeToOS(uptr From, uptr To) {
const uptr Size = To - From;
releasePagesToOS(Base, From + Offset, Size, Data);
- ReleasedRangesCount++;
ReleasedBytes += Size;
}
private:
- uptr ReleasedRangesCount = 0;
uptr ReleasedBytes = 0;
// The starting address to release. Note that we may want to combine (Base +
// Offset) as a new Base. However, the Base is retrieved from
|
cferris1000
reviewed
Dec 19, 2024
| PushedBytesDelta >> 10); | ||
| Str->append( | ||
| " %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu " | ||
| "inuse: %6zu avail: %6zu release count: %6zu last released: %6zuK " |
There was a problem hiding this comment.
Should this be change to releases attempted?
| @@ -387,7 +387,7 @@ template <typename Config> class SizeClassAllocator32 { | |||
|
|
|||
| struct ReleaseToOsInfo { | |||
| uptr BytesInFreeListAtLastCheckpoint; | |||
| uptr RangesReleased; | |||
| uptr NumReleaseAttempted; | |||
There was a problem hiding this comment.
This is such a nit, but could this be NumReleasesAttempted?
cferris1000
reviewed
Dec 19, 2024
| PushedBytesDelta >> 10); | ||
| Str->append( | ||
| " %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu " | ||
| "inuse: %6zu avail: %6zu release attempted: %6zu last released: %6zuK " |
| @@ -530,7 +530,7 @@ template <typename Config> class SizeClassAllocator64 { | |||
|
|
|||
| struct ReleaseToOsInfo { | |||
| uptr BytesInFreeListAtLastCheckpoint; | |||
| uptr RangesReleased; | |||
| uptr NumReleaseAttempted; | |||
There was a problem hiding this comment.
Can you change the name to NumReleasesAttempted here too?
| @@ -1144,11 +1144,12 @@ template <typename Config> class SizeClassAllocator64 { | |||
| Str->append( | |||
| "%s %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu " | |||
| "inuse: %6zu total: %6zu releases: %6zu last " | |||
| "released: %6zuK latest pushed bytes: %6zuK region: 0x%zx (0x%zx)\n", | |||
| "release attempted: %6zuK latest pushed bytes: %6zuK region: 0x%zx " | |||
ChiaHungDuan
commented
Dec 19, 2024
| PushedBytesDelta >> 10); | ||
| Str->append( | ||
| " %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu " | ||
| "inuse: %6zu avail: %6zu release attempted: %6zu last released: %6zuK " |
| @@ -530,7 +530,7 @@ template <typename Config> class SizeClassAllocator64 { | |||
|
|
|||
| struct ReleaseToOsInfo { | |||
| uptr BytesInFreeListAtLastCheckpoint; | |||
| uptr RangesReleased; | |||
| uptr NumReleaseAttempted; | |||
| @@ -1144,11 +1144,12 @@ template <typename Config> class SizeClassAllocator64 { | |||
| Str->append( | |||
| "%s %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu " | |||
| "inuse: %6zu total: %6zu releases: %6zu last " | |||
| "released: %6zuK latest pushed bytes: %6zuK region: 0x%zx (0x%zx)\n", | |||
| "release attempted: %6zuK latest pushed bytes: %6zuK region: 0x%zx " | |||
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.
This also removes the
RangesReleasedwhich doesn't give much insight to whether we should adjust the heuristic of doing page release.