-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[scudo] Apply the min release threshold to the group #112014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
For the block smaller than a page size, one block is unlikely to introduce more unused pages (at most 2 if it acrosses the page boundary and both touched pages are unused). So it's better to apply the threshold to reduce the time of scanning groups that can't release any new pages.
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (ChiaHungDuan) ChangesFor the block smaller than a page size, one block is unlikely to introduce more unused pages (at most 2 if it acrosses the page boundary and both touched pages are unused). So it's better to apply the threshold to reduce the time of scanning groups that can't release any new pages. Full diff: https://github.com/llvm/llvm-project/pull/112014.diff 1 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index 97188a5ac235cc..1865ed41368e3c 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -1492,6 +1492,8 @@ template <typename Config> class SizeClassAllocator64 {
}
const uptr PushedBytesDelta = BytesInBG - BG->BytesInBGAtLastCheckpoint;
+ if (PushedBytesDelta < getMinReleaseAttemptSize(BlockSize))
+ continue;
// 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
|
Even this is just a minor change, given that it may still have small chance to impact the RSS so I put it in an independent change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/6787 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/5002 Here is the relevant piece of the build log for the reference
|
This reverts commit 53c9553.
Reverts #112014 The change didn't update the iterator
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/4895 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/5106 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/11792 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/139/builds/4979 Here is the relevant piece of the build log for the reference
|
For the block smaller than a page size, one block is unlikely to introduce more unused pages (at most 2 if it acrosses the page boundary and both touched pages are unused). So it's better to apply the threshold to reduce the time of scanning groups that can't release any new pages.
…12252) Reverts llvm#112014 The change didn't update the iterator
For the block smaller than a page size, one block is unlikely to introduce more unused pages (at most 2 if it acrosses the page boundary and both touched pages are unused). So it's better to apply the threshold to reduce the time of scanning groups that can't release any new pages.