-
Notifications
You must be signed in to change notification settings - Fork 148
8354145: G1: UseCompressedOops boundary is calculated on maximum heap region size instead of maxiumum ergonomic heap region size #2497
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
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back cost0much! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
This backport pull request has now been updated with issue from the original commit. |
phohensee
left a comment
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.
What other testing have you done? We usually like to see at least jdk tier1, and tier2 if possible.
|
@phohensee Thanks for calling that out. I've updated the description, and since ran tests on internal pipelines confirming no regressions jtreg tier 1-4 for our supported platforms. |
phohensee
left a comment
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. Thanks!
|
|
|
/approval request Fixes UseCompressedOops ergonomics to use original limits for G1 GC: prior to JDK18, -Xmx32736m by default enables UseCompressedOops but in JDK21 -Xmx32736m default sets UseCompressedOops to false. Regarding only LTS versions, this creates a gap in expected behavior as 8-17 and 25 all share the same ergonomics, with only 21 being different; this greatly inconveniences users upgrading/migrating JDK versions. Does not apply cleanly due to JDK-8330694 but conflicts are trivial. Patched in JDK25 and no bugtail. Passes jtreg tier1-4 on multiple platforms; see PR for list. New test passes with change, fails without; GHA also passes. Risk is medium because hotspot change that affects G1. |
|
@cost0much |
Fixes UseCompressedOops ergonomics to use original limits for G1 GC; prior to JDK18,
-Xmx32736mby default enables UseCompressedOops but in JDK21-Xmx32736mdefault sets UseCompressedOops to false. New test passes with change, fails without. Change ran on internal Amazon pipeline and passes jtreg tier1-4 on platforms linux x64, aarch64, aarch32; windows x64; macos x64, aarch64. Passes GHA. Backport not clean due to JDK-8330694 renamingHeapRegiontoG1HeapRegion; modifications to the change are minimal as only had to "un-rename" the refactor.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/2497/head:pull/2497$ git checkout pull/2497Update a local copy of the PR:
$ git checkout pull/2497$ git pull https://git.openjdk.org/jdk21u-dev.git pull/2497/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2497View PR using the GUI difftool:
$ git pr show -t 2497Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/2497.diff
Using Webrev
Link to Webrev Comment