8329706: Implement -XX:+AOTClassLinking

[iklam]

This is the 3rd PR for JEP 483: Ahead-of-Time Class Loading & Linking.

Overview

• A new -XX:+AOTClassLinking flag is added. See JEP 498 and the CSR for a discussion of this command-line option, its default value, and its impact on compatibility.
• When this flag is enabled during the creation of an AOT cache (aka CDS archive), an AOTLinkedClassTable is added to the cache to include all classes that are AOT-linked. For this PR, only classes for the boot/platform/application loaders are eligible. The main logic is in aotClassLinker.cpp.
• When an AOT archive is loaded in a production run, all classes in the AOTLinkedClassTable are loaded into their respective class loaders at the earliest opportunity. The main logic is in aotLinkedClassBulkLoader.cpp.
  • The boot classes are loaded as part of vmClasses::resolve_all()
  • The platform/application classes are loaded after the module graph is restored (see changes in threads.cpp).
• Since all classes in a AOTLinkedClassTable are loaded before any user-code is executed, we can resolve constant pool entries that refer to these classes during AOT cache creation. See changes in AOTConstantPoolResolver::is_class_resolution_deterministic().

All-or-nothing Loading

• Because AOT-linked classes can refer to each other, using direct C++ pointers, all AOT-linked classes must be loaded together. Otherwise we will have dangling C++ pointers in the class metadata structures.
• During a production run, we check during VM start-up for incompatible VM options that would prevent some of the AOT-linked classes from being loaded. For example:
  • If the VM is started with an JVMTI agent that has ClassFileLoadHook capabilities, it could replace some of the AOT-linked classes with alternative versions.
  • If the VM is started with certain module options, such as --patch-module or --module, some AOT-linked classes may be replaced with patched versions, or may become invisible and cannot be loaded into the JVM.
• When incompatible VM options are detected, the JVM will refuse to load an AOT cache that has AOT-linked classes. See FileMapInfo::validate_aot_class_linking().
  • For simplfication, FileMapInfo::validate_aot_class_linking() requires CDSConfig::is_using_full_module_graph() to be true. This means that the exact same set of modules are visible when the AOT cache was created, and when the AOT cache is used.

Testing

• New test cases are added to test specific behaviors of the -XX:+AOTClassLinking flag
• A new test group hotspot_aot_classlinking is created for running existing CDS tests with the -XX:+AOTClassLinking flag. Note that this group filters out test cases that are incompatible with -XX:+AOTClassLinking. E.g., classes that use JVMTI ClassFileLoadHook or class redefinition.
• We will also modify some of our internal test cases (e.g., with large applications) to also run with the -XX:+AOTClassLinking flag.

---

See here for the sequence of dependent RFEs for implementing JEP 483.

---

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-8339506 to be approved

Integration blocker

 ⚠️ Dependency #20517 must be integrated first

Issues

• JDK-8329706: Implement -XX:+AOTClassLinking (Enhancement - P4)
• JDK-8339506: Implement -XX:+AOTClassLinking (CSR)

Reviewers

• David Holmes (@dholmes-ora - Reviewer) Review applies to 6bb38cb9
• Andrew Dinn (@adinn - Reviewer) Review applies to 0441aef0
• Ashutosh Mehra (@ashu-mehra - Committer) Review applies to bcddf963
• Calvin Cheung (@calvinccheung - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 20843

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

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

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

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

• hotspot
• serviceability

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

• 16: Full - Incremental (9d3d8bd9)
• 15: Full - Incremental (6bb38cb9)
• 14: Full - Incremental (22c47d33)
• 13: Full (02ac6d6e)
• 12: Full (3cdc7634)
• 11: Full (6029b35f)
• 10: Full - Incremental (dd5a5ba6)
• 09: Full - Incremental (3215c002)
• 08: Full - Incremental (be1d0ef1)
• 07: Full - Incremental (bedf9a26)
• 06: Full - Incremental (4e9668a0)
• 05: Full - Incremental (bcddf963)
• 04: Full - Incremental (66a4ff41)
• 03: Full - Incremental (5bba4ad4)
• 02: Full - Incremental (0441aef0)
• 01: Full - Incremental (ac1ed798)
• 00: Full (a5e7eb51)

[dholmes-ora]

I've taken an initial look through but there is an awful lot to try and digest here. I've flagged numerous typos and minor nits.

One general query: does this stuff work if the user defines their own initial application classloader?

[AlanBateman]

One general query: does this stuff work if the user defines their own initial application classloader?

Just to say that there isn't any support in the JDK for replacing any of the 3 built-in class loaders. In normal setup, the system class loader == application class loader.

It is possible to run -Djava.system.class.loader=.. to set your own system class loader, it gets created with the built-in application class loader as its parent. In that setup you start out with 4 class loaders. It's not a widely used feature and I assume goes into the "user-defined class loaders" non-goal bucket. In any case, loading from the class path or module system continues to use the built-in application class loader in that configuration so it might not be totally hostile to the AOT cache. I suspect there are tests already for this setup.

[dholmes-ora]

It is possible to run -Djava.system.class.loader=.. to set your own system class loader, it gets created with the built-in application class loader as its parent. In that setup you start out with 4 class loaders.

Yes that is what I was referring to.

[dholmes-ora]

I have taken another pass through. A few queries and small items.

Thanks

[mlbridge]

Mailing list message from John Rose on hotspot-dev:

On 18 Sep 2024, at 21:11, Ioi Lam wrote:

On Wed, 18 Sep 2024 05:07:33 GMT, David Holmes <dholmes at openjdk.org> wrote:
?

src/hotspot/share/cds/aotLinkedClassBulkLoader.cpp line 170:

168: log_error(cds)("Unable to resolve %s class from CDS archive: %s", category_name, ik->external_name());
169: log_error(cds)("Expected: " INTPTR_FORMAT ", actual: " INTPTR_FORMAT, p2i(ik), p2i(actual));
170: log_error(cds)("JVMTI class retransformation is not supported when archive was generated with -XX:+AOTClassLinking.");

Nit: use a `logStream` instead of the three separate calls.

Why?

If this were running millions of times a second, and
it were a debug or trace log message, using a log stream
might batch up the gating logic instead of executing
it three times, and it might make for a more efficient
output, with three lines grouped cleanly. But for
a rare error message, those reasons are less important.
Maybe the code would be more readable with a log stream?
But I find this code readable enough. YMMV

[dholmes-ora]

Nothing further from me.

Thanks

[ashu-mehra]

LGTM

[calvinccheung]

Spotted a few nits.

[dholmes-ora]

Still good for me - but seems to be jcheck whitespace issues.

[iklam]

Still good for me - but seems to be jcheck whitespace issues.

Thanks for noticing it. I've fixed the whitespaces (and need to propagate that to all subsequent PRs in this series).

[iklam]

Closing this PR as its changes have been combined into #21642.