8359436: AOTCompileEagerly should not be diagnostic

[shipilev]

A new AOTCompileEagerly flag introduced by JDK-8355003 is marked as diagnostic. However, this flag guards the experimental feature, that is, whether the existence of AOT profiles should trigger immediate JIT compilation. Therefore, this flag should be at least be "experimental", rather than "diagnostic".

I don't think it makes sense to elevate this flag to full product flag, since once AOT code caching arrives, this flag would default to true, and would cause AOT loads instead of JIT compilations. Disabling the flag by user choice would be significantly counter-productive then.

Additional testing:

• Linux x86_64 server fastdebug, runtime/cds

---

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

Issue

• JDK-8359436: AOTCompileEagerly should not be diagnostic (Enhancement - P3)

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer) Review applies to c00fef1b
• SendaoYan (@sendaoYan - Committer) Review applies to c00fef1b
• David Holmes (@dholmes-ora - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25799

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

8359436: AOTCompileEagerly should not be diagnostic

Reviewed-by: dholmes, kvn, syan

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

• f62f117: 8359701: G1: Move heap expansion time tracking of G1CollectedHeap:expand to call site
• a082082: 8354650: [PPC64] Try to reduce register definitions
• f7cd3fa: 8332623: Remove setTTL()/getTTL() methods from DatagramSocketImpl/MulticastSocket and MulticastSocket.send(DatagramPacket, byte)
• ... and 112 more: https://git.openjdk.org/jdk/compare/bb2611ad43a2feaebc2246fdbac2179a40115d67...master

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]

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

• hotspot-runtime

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.

[mlbridge]

Webrevs

• 01: Full - Incremental (c62f367f)
• 00: Full (c00fef1b)

[vnkozlov]

once AOT code caching arrives, this flag would default to true, and would cause AOT loads instead of JIT compilations. Disabling the flag by user choice would be significantly counter-productive then.

Why do you need to change it then?

[shipilev]

once AOT code caching arrives, this flag would default to true, and would cause AOT loads instead of JIT compilations. Disabling the flag by user choice would be significantly counter-productive then.

Why do you need to change it then?

Because in JDK 25, I can see people experimenting with this option, or even turning it on. It is inconvenient to turn on diagnostic option for production use.

[vnkozlov]

When AOT code caching is integrated do you plan to move it back to diagnostic? As you said - it does not make sense to touch it.

But then people will be confused if in JDK 25 and 26 they are different.

[shipilev]

When AOT code caching is integrated do you plan to move it back to diagnostic? As you said - it does not make sense to touch it.

Right. I think we would need to move it back to diagnostic once AOT code compilation would land. It would also reflect the status of the feature protected by the flag: in JDK 25, it is sensible to flip it back and forth as experimental performance flag; past that, the only sensible behavior is "on", and you only turn it off to diagnose bugs.

What I am trying to achieve with this PR: users in JDK 25 should not be unlocking diagnostic flags for their performance needs. At most, they need to unlock experimental ones.

[vnkozlov]

Okay. Then why not do this change in JDK 25 branch only and don't touch JDK 26?
You would need approval for JDK 25: https://openjdk.org/jeps/3#Late-Enhancement-Request-Process

[shipilev]

Okay. Then why not do this change in JDK 25 branch only and don't touch JDK 26?

Are we 100% sure AOT code cache would land in JDK 26? I don't think we can make that bet yet :) This is why it is for both mainline and JDK 25.

[vnkozlov]

Okay. Then why not do this change in JDK 25 branch only and don't touch JDK 26?

Are we 100% sure AOT code cache would land in JDK 26? I don't think we can make that bet yet :) This is why it is for both mainline and JDK 25.

That is the plan but we can't promise officially. "When it is ready".

[vnkozlov]

Okay, let's go with this. Don't forget to request approval for JDK 25.

Trivial.

[dholmes-ora]

Are there no tests that enable this flag and so would need updating to unlock it correctly?

[shipilev]

Are there no tests that enable this flag and so would need updating to unlock it correctly?

Yeah, there are no tests for this flag. The flag was a late Leyden addition, and it defaults to true in Leyden/premain, so we know it performs well enough. But for mainline, we should really have a sanity test, let me do it here.

[shipilev]

Yeah, there are no tests for this flag. The flag was a late Leyden addition, and it defaults to true in Leyden/premain, so we know it performs well enough. But for mainline, we should really have a sanity test, let me do it here.

Added a sanity test. Passes Linux x86_64 server fastdebug/release.

[dholmes-ora]

Sanity test seems okay. Thanks.

[vnkozlov]

Missing * @bug 8359436

[vnkozlov]

There is aotProfile directory for training data testing.

[vnkozlov]

May be modify aotProfile/AOTProfileFlags.java instead.