8326957: Implement JEP 474: ZGC: Generational Mode by Default

[xmas92]

This is the implementation task for JEP 474: ZGC: Generational Mode by Default. See the JEP for details. JDK-8326667

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires CSR request JDK-8326958 to be approved

Issues

• JDK-8326957: Implement JEP 474: ZGC: Generational Mode by Default (Sub-task - P4)
• JDK-8326958: Implement JEP 474: ZGC: Generational Mode by Default (CSR)

Reviewers

• Stefan Karlsson (@stefank - Reviewer)
• Erik Österlund (@fisk - Reviewer) ⚠️ Review applies to ca9492d9

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18393/head:pull/18393
$ git checkout pull/18393

Update a local copy of the PR:
$ git checkout pull/18393
$ git pull https://git.openjdk.org/jdk.git pull/18393/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18393

View PR using the GUI difftool:
$ git pr show -t 18393

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18393.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back aboldtch! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@xmas92 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:

8326957: Implement JEP 474: ZGC: Generational Mode by Default

Reviewed-by: stefank, eosterlund

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 3 new commits pushed to the master branch:

• ea5eb74: 8326404: Assertion error when trying to compile switch with fallthrough with pattern
• beea530: 8331907: BigInteger and BigDecimal should use optimized division
• 440782e: 8331466: Problemlist serviceability/dcmd/gc/RunFinalizationTest.java on generic-all

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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 master branch, type /integrate in a new comment.

[openjdk]

@xmas92 The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[openjdk]

@xmas92 this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8326957
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[mlbridge]

Webrevs

• 05: Full (60460dce)
• 04: Full - Incremental (4de30da2)
• 03: Full (8fac84cc)
• 02: Full (6e2edc9b)
• 01: Full - Incremental (e80424c6)
• 00: Full (ca9492d9)

[stefank]

Looks good.

[fisk]

Looks good. One nit in terminology, don't need to see the new version.

[tkrodriguez]

Graal still doesn't support generational ZGC though I'm actively working on it. I'm hoping to have it in before rampdown but at least until that time JVMCI needs to default to non-generational ZGC.

[stefank]

Graal still doesn't support generational ZGC though I'm actively working on it. I'm hoping to have it in before rampdown but at least until that time JVMCI needs to default to non-generational ZGC.

Great that you are working on support for Generational ZGC!

The intent of this JEP is to make it so that you get Generational ZGC when you specify -XX:+UseZGC, and then warn that non-generational ZGC is deprecated and warn if the user explicitly turns it on. I don't want us to change the meaning of -XX:+UseZGC when the Graal JIT is enabled. Doing so would be inconsistent, cause confusion, and could be misleading to our users.

In fact, we have already seen instances of a somewhat opposite problem. Today if you try to run Generational ZGC with the Graal JIT, you get Generational ZGC but revert back to using C2. We print a warning when that happens, but we have seen users not realizing that they ran C2 instead of the Graal JIT.

My proposal is to simply refuse to start the JVM and print an error message when we have conflicting GC and JIT compiler flags.

With this we would have:

• -XX:+UseZGC -XX: => Refuse to start with error message
• -XX:+UseZGC -XX:-ZGenerational -XX: => Same as above
• -XX:+UseZGC -XX:-ZGenerational -XX: => Start (with message about deprecation of non-generational ZGC)

With this we would both get rid of the confusing situation where the user asks for Graal JIT but get C2 and the other confusing situation where the user asks for Generational ZGC but gets non-generational ZGC. The user will have to make a decision on what combination they want to run.

[stefank]

This is my proposal:

For this JEP, I propose that we stick with the current plan that -XX:+UseZGC always mean Generational ZGC. If the user try to enable the Graal JIT when running Generational ZGC, we'll print a warning and switch over to C2, just like we're doing today.

Then, as a follow-up, we can figure out what the correct behavior should be if the user specifies an incompatible GC and compiler combination.

@tkrodriguez Is this an OK action plan for you?

[tkrodriguez]

Sorry I didn't reply sooner. That sounds like a fine interim solution. I agree that automatically selecting something different can be very confusing. Anyway, hopefully this kind of problem will be a transient one.

[stefank]

Thanks!

[dougxc]

After discussing with Tom offline, here's one more change you could make in this PR:

• Change the warning in check_jvmci_supported_gc to a hard error when an unsupported GC + compiler combination is requested.

Either way, as soon we have Gen ZGC implemented for Graal, we will:

1. Update gc_supports_jvmci.
2. Update check_jvmci_supported_gc to no longer warn and switch compilers but to exit the VM when an unsupported combination is requested.

[xmas92]

Thanks for the reviews.
/integrate

[openjdk]

Going to push as commit 4ba7447.
Since your change was applied there have been 4 commits pushed to the master branch:

• 7ce4a13: 8332130: RISC-V: remove wrong instructions of Vector Crypto Extension
• ea5eb74: 8326404: Assertion error when trying to compile switch with fallthrough with pattern
• beea530: 8331907: BigInteger and BigDecimal should use optimized division
• 440782e: 8331466: Problemlist serviceability/dcmd/gc/RunFinalizationTest.java on generic-all

Your commit was automatically rebased without conflicts.

[openjdk]

@xmas92 Pushed as commit 4ba7447.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.