On Mon, 23 Sep 2024 18:45:49 GMT, Ioi Lam <ik...@openjdk.org> 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...
Cursory review:
src/hotspot/share/cds/aotClassInitializer.cpp line 47:
> 45:
> 46: Symbol* name = ik->name();
> 47: if (name->equals("jdk/internal/constant/PrimitiveClassDescImpl") ||
Could we / should we start using `vmSymbols` constants for these? I think you
can directly compare the `Symbol*`-s then, like `name ==
vmSymbols::jdk_internal_constant_PrimitiveClassDescImpl_Holder() `.
src/hotspot/share/cds/aotConstantPoolResolver.cpp line 476:
> 474: if (bsm_klass->equals("java/lang/invoke/StringConcatFactory") &&
> 475: bsm_name->equals("makeConcatWithConstants")) {
> 476: return true;
I think all BSM entries in SCF are deterministic, really. So we can just check
for the class?
-------------
PR Review: https://git.openjdk.org/jdk/pull/21143#pullrequestreview-2331919072
PR Review Comment: https://git.openjdk.org/jdk/pull/21143#discussion_r1777490992
PR Review Comment: https://git.openjdk.org/jdk/pull/21143#discussion_r1777474219
