[scudo] Release to OS if a large amount of memory is deallocated. #160621
Conversation
Before this change, if large amounts of memory are deallocated within a release interval, the release is put off until the release interval occurs. Unfortunately, for larger class sizes, this could mean that a lot of this memory accumulates and is never released since no more deallocations occur in that size class. To fix this, if `RegionPushedBytesDelta` grows larger than a group size, immediately do a release. This work was originally done by ChiaHungDuan.
llvmbot
added
compiler-rt
compiler-rt:scudo
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Christopher Ferris (cferris1000) ChangesBefore this change, if large amounts of memory are deallocated within a release interval, the release is put off until the release interval occurs. Unfortunately, for larger class sizes, this could mean that a lot of this memory accumulates and is never released since no more deallocations occur in that size class. To fix this, if This work was originally done by ChiaHungDuan. Full diff: https://github.com/llvm/llvm-project/pull/160621.diff 1 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index d08103008ef7c..747b1a2233d32 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -1565,6 +1565,13 @@ bool SizeClassAllocator64<Config>::hasChanceToReleasePages(
if (DiffSinceLastReleaseNs < 2 * IntervalNs)
return false;
} else if (DiffSinceLastReleaseNs < IntervalNs) {
+ // `TryReleaseThreshold` is capped by (1UL << GroupSizeLog) / 2). If
+ // RegionPushedBytesDelta grows to twice the threshold, it implies some
+ // huge deallocations have happened so we better try to release some
+ // pages. Note this tends to happen for larger block sizes.
+ if (RegionPushedBytesDelta > (1ULL << GroupSizeLog))
+ return true;
+
// In this case, we are over the threshold but we just did some page
// release in the same release interval. This is a hint that we may want
// a higher threshold so that we can release more memory at once.
|
|
See if my explanation makes sense, since this seems to be what the problem is. I verified this by running against the memory traces, running boot tests, running other benchmarks. I did see a slight increase in the time it took and a decrease in memory used. I expect that this will trip some performance benchmarks, so I want to get this in and see what happens. I did see some big memory savings in some cases, which probably means that we can probably tune the memory check better in the future to get even more savigns. |
…vm#160621) Before this change, if large amounts of memory are deallocated within a release interval, the release is put off until the release interval occurs. Unfortunately, for larger class sizes, this could mean that a lot of this memory accumulates and is never released since no more deallocations occur in that size class. To fix this, if `RegionPushedBytesDelta` grows larger than a group size, immediately do a release. This work was originally done by ChiaHungDuan.
…vm#160621) Before this change, if large amounts of memory are deallocated within a release interval, the release is put off until the release interval occurs. Unfortunately, for larger class sizes, this could mean that a lot of this memory accumulates and is never released since no more deallocations occur in that size class. To fix this, if `RegionPushedBytesDelta` grows larger than a group size, immediately do a release. This work was originally done by ChiaHungDuan.
Before this change, if large amounts of memory are deallocated within a release interval, the release is put off until the release interval occurs. Unfortunately, for larger class sizes, this could mean that a lot of this memory accumulates and is never released since no more deallocations occur in that size class.
To fix this, if
RegionPushedBytesDeltagrows larger than a group size, immediately do a release.This work was originally done by ChiaHungDuan.