RFR: 8348322: AOT cache creation crashes with "All cached hidden classes must be aot-linkable" when AOTInvokeDynamicLinking is disabled [v2]

[Ioi Lam]

On Fri, 28 Feb 2025 22:59:22 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> src/hotspot/share/cds/cdsConfig.cpp line 618:
>> 
>>> 616: // Archived MethodHandles are required for higher-level optimizations such as AOT resolution of invokedynamic
>>> 617: // and dynamic proxies.
>>> 618: bool CDSConfig::is_dumping_method_handles() {
>> 
>> This looks like it does the same check as `is_initing_classes_at_dump_time() ` and is very similar to `is_dumping_invokedynamic()`. Maybe you can combine these methods to avoid duplicate code?
>
> I removed the `CDSConfig::is_initing_classes_at_dump_time()` since the conditions are the same as `CDSConfig::is_dumping_method_handles()`. All the calls to `CDSConfig::is_initing_classes_at_dump_time()` have been updated accordingly.

I think we should keep `is_initing_classes_at_dump_time()`, because it's used as a precondition for archiving initialized Java mirrors. Putting such code inside `is_dumping_method_handles()` would not make sense.

In fact, archived method handles require the ability to store initialized mirrors (for the hidden classes). So I think we should change `is_dumping_method_handles()` to this:


// When we can dump initialized classes mirrors, we automatically enable the archiving of MethodHandles.
// This will in turn enable the archiving of MethodTypes and hidden classes that are used in the
// implementation of MethodHandles. Note: these hidden classes need to be stored in the initialized state,
// as we don't have a mechanism to trigger their initialization on-demand.
// Archived MethodHandles are required for higher-level optimizations such as AOT resolution of invokedynamic
// and dynamic proxies.
bool CDSConfig::is_dumping_method_handles() {
  return is_initing_classes_at_dump_time();
}

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23546#discussion_r1976136046