On Mon, 23 Sep 2024 18:45:49 GMT, Ioi Lam <[email protected]> wrote:
> This is the 7th and final PR for [JEP 483: Ahead-of-Time Class Loading &
> Linking](https://bugs.openjdk.org/browse/JDK-8315737).
>
> This PR implements the AOT-linking of invokedynamic callsites:
> - We only link lambda expressions (`LambdaMetafactory::metafactory`) and
> string concat (`StringConcatFactory::makeConcatWithConstants()`) as the
> results of these bootstrap methods do not have environment dependencies and
> can be safely cached.
> - The resolved CallSites are represented as Java heap objects. Thus, this
> optimizations is supported only for the static CDS archive, which can store
> heap objects. The dynamic CDS archive is not supported.
>
> **Review Notes:**
>
> - Start with `AOTConstantPoolResolver::preresolve_indy_cp_entries()` -- it
> checks all indys that were linked during the training run, and aot-links only
> those for lambdas and string concats
> - `SystemDictionaryShared::find_all_archivable_classes()` finds all the
> hidden classes that are reachable from the indy CallSites
> - In `MetaspaceShared::preload_and_dump_impl()` we call
> `MethodType::createArchivedObjects()` to record all MethodTypes that were
> created in the assembly phase. This is necessary because the identity of
> MethodTypes is significant (they are compared with the `==` operator). Also
> see MethodType.java for the corresponding code that are used in the
> production run.
> - Afterwards, we enter the safepoint (`VM_PopulateDumpSharedSpace::doit()`).
> In this safepoint,
> `ConstantPoolCache::remove_resolved_indy_entries_if_non_deterministic()` is
> called to remove any resolved indy callsites that cannot be archived. (Such
> CallSites may be created as a side effect of Java code execution in the
> assembly phase. E.g., the bootstrap of the module system).
>
> **Rough Edges:**
>
> - Many archived CallSites references (directly or indirectly) the static
> fields of the classes listed under
> `AOTClassInitializer::can_archive_initialized_mirror()`, where the object
> identity of these static fields is significant. Therefore, we must preserve
> the initialized states of these classes. Otherwise, we might run into
> problems such as [JDK-8340836](https://bugs.openjdk.org/browse/JDK-8340836).
> Unfortunately, the list is created by trial-and-error, and may need to be
> updated to match changes in the `java.lang.invoke` and
> `jdk.internal.constant` packages. We expect Project Leyden to come with a
> general solution to this problem.
> - If the identity is significant for a static field in a complex class, but
> not all of the static fields of this cl...
test/lib/jdk/test/lib/cds/CDSTestUtils.java line 877:
> 875: }
> 876: }
> 877: if (lastMatch != null && lastMatch.equals("-XX:+" +
> optionName)) {
Suggestion:
if (lastMatch != null && lastMatch.equals("-XX:+" + optionName)) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21143#discussion_r1777059374
