On Thu, 26 Sep 2024 17:29:38 GMT, Aleksey Shipilev <sh...@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 a... > > 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() `. Not every class in this table is inside vmSymbols, and I don't want to have to edit vmSymbols whenever a new class is added here (and having to do garbage collection in vmSymbols when a class is removed from here). I pushed a new version that's more table driven, and avoid most strcmp calls by filtering with the symbol length. > 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? We haven't done much testing with the other SCF BSMs, so I think it's better to do that in a follow-up REF. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21143#discussion_r1782287180 PR Review Comment: https://git.openjdk.org/jdk/pull/21143#discussion_r1782282918
