8372696: Allow boot classes to explicitly opt-in for final field trusting

[liach]

Currently, the hotspot compiler (as in ciField) trusts final fields in hidden classes, record classes, and selected jdk packages. Some classes in the JDK wish to be trusted, but they cannot apply package-wide opt-in due to other legacy classes in the package, such as java.util.

They currently can use @Stable as a workaround, but this is fragile because a stable final field may hold a trusted null, zero, or false value, which is currently treated as non-constant by ciField.

We should add an annotation to opt-in for a whole class, mainly for legacy packages. This would benefit greatly some of our classes already using a lot of Stable, such as java.util.Optional, whose empty instance is now constant-foldable, as demonstrated in a new IR test.

Paging @minborg who requested Optional folding for review.

I think we can remove redundant Stable in a few other java.util classes after this patch is integrated. I plan to do that in subsequent patches.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8372696: Allow boot classes to explicitly opt-in for final field trusting (Enhancement - P4)

Reviewers

• Jorn Vernee (@JornVernee - Reviewer) 🔄 Re-review required (review applies to f02b9da2)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28540

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back liach! 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]

@liach This change is no longer ready for integration - check the PR body for details.

[openjdk]

@liach The following labels will be automatically applied to this pull request:

• core-libs
• hotspot

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

[mlbridge]

Webrevs

• 03: Full - Incremental (d353bdbe)
• 02: Full - Incremental (7a1cfa4a)
• 01: Full - Incremental (712dbf1c)
• 00: Full (f02b9da2)

[liach]

/cc hotspot-compiler hotspot-runtime

This uses another one of the 16-bit instanceKlassFlags, which requires runtime engineers to agree. Need compiler review to check if such IR tests are the best way to ensure constant folding for core library classes.

[openjdk]

@liach
The hotspot-compiler label was successfully added.

The hotspot-runtime label was successfully added.

[JornVernee]

/reviewers 2

[openjdk]

@JornVernee
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[minborg]

I really like this one!

I wonder if we could enable the new annotation @TrustFinalFields on package level as well so we could get rid of all the special handing in ciField.spp. I am not sure this is the best way to do it but it would perhaps be possible to annotate the package-info.java file. For example in java.lang.invoke.package-info.java:

@TrustFinalFields
package java.lang.invoke;

Is there a better way to do it?

[AlanBateman]

I wonder if we could enable the new annotation @TrustFinalFields on package level as well so we could get rid of all the special handing in ciField.spp. I am not sure this is the best way to do it but it would perhaps be possible to annotate the package-info.java file. For example in java.lang.invoke.package-info.java:

The VM doesn't read/parse the package-info class. They are only read by APIs that want to get annotations. In any case, the long term goal needs to be to remove all special handling.

[AlanBateman]

I assume any write-up of HotSpot constant folding should move into src/hotspot tree, maybe a block comment in one of the source files?

[liach]

I intend this to be a user-oriented guide on constant folding. I should just call it constant folding.

[liach]

I intend this to be a user-oriented guide on constant folding. I should just call it constant folding.

[AlanBateman]

I think you can drop this section for now. It's okay to reference JEP 500 but it will be annoying to have to maintain this text as there are many steps to follow this one.

[coleenp]

With one small change, the runtime part of this change looks good.

[coleenp]

This is missing { } so not sure where it ends, especially that it encloses an if statement, and other code.

[DanHeidinga]

A bit of meta-question about this PR and JEP 500: does this trust need to be rescinded if the user explicitly adds --enable-final-field-mutation= for the modules that contain these classes marked with the annotation?

[liach]

This PR currently does not interact with JEP 500. However, as specified in Field.set, the result of setting a final field may be ignored, as Alan commented. So I don't think we need to rescind the current trusting even if users enable mutations.

In addition, @DanHeidinga I made the same fault as you when I first saw --enable-final-field-mutation= - this actually represents the callers, instead of the target, of Field.set. The target of mutation is specified via --add-opens, if the target field is not public.