-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8328075: Shenandoah: Avoid forwarding when objects don't move in full-GC #18280
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
👋 Welcome back rkennke! A progress list of the required criteria for merging this PR into |
@rkennke This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 54 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
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
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.
A few nits. I think we do the same thing for humongous objects already (where it is really painful), no particular reason not to do the same for regular ones. Any performance tests for Full GC improve?
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.
Looks good after Aleksey's assertion comment fix. (FWIW, i've seen many back-to-back Full GCs that do not relocate any objects on some of the jtreg tests. Of course, these back-to-back Full GCs hopefully don't represent typical behavior of real workloads.)
Thanks! |
Going to push as commit 80ccc98.
Your commit was automatically rebased without conflicts. |
Currently, in Shenandoah's full-GC, we forward all marked objects (and preserve their headers), even if they don't move. This typically happens for a certain amount of 'sediment' that accumulates at the bottom of the heap. This results in wasted CPU cycles and memory accesses and usage. It can easily be avoided by not forwarding objects that don't move.
The fix is to simply not forward objects when they don't move.
Testing:
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18280/head:pull/18280
$ git checkout pull/18280
Update a local copy of the PR:
$ git checkout pull/18280
$ git pull https://git.openjdk.org/jdk.git pull/18280/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 18280
View PR using the GUI difftool:
$ git pr show -t 18280
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18280.diff
Webrev
Link to Webrev Comment