On Mon, 7 Oct 2024 16:09:30 GMT, Andrew Dinn <ad...@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/aotConstantPoolResolver.cpp line 450: > >> 448: >> 449: Symbol* mh_sig = cp->method_handle_signature_ref_at(mh_index); >> 450: if (!check_lambda_metafactory_signature(cp, mh_sig, false)) { > > This appears to be rerunning the previous check without a log message -- or > am I missing something here? It was a cut-and-paste mistake. I've removed this line in [195303f](https://github.com/openjdk/jdk/pull/21143/commits/195303f2c6939b93b6152c6610bd38faff5ae3c7) , where I also fixed some minor bugs with `AOTConstantPoolResolver::is_indy_resolution_deterministic()` and added more comments in the C++ code about what checks are being made. I also added tests code in AOTLinkedLambdas.java to cover all the type checks that happen inside `is_indy_resolution_deterministic()`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21143#discussion_r1792594084
