RFR: 8353597: Refactor handling VM options for AOT cache input and output [v2]

[Ioi Lam]

On Thu, 3 Apr 2025 21:40:48 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> @iklam one annoying thing in current ergonomic setting for AOTCode flags in mainline is checking which phase we are executing. We agreed before that we should only save/load AOT code when `AOTClassLinking` is on because AOT code needs classes to be preloaded.
>> 
>> I have to do next checks to enable AOTCode in `CDSConfig::check_vm_args_consistency()`:
>> 
>>   if (AOTClassLinking && is_using_archive() && !is_dumping_archive() && !FLAG_IS_DEFAULT(AOTCache)) {
>>     FLAG_SET_ERGO_IF_DEFAULT(LoadAOTCode, true);
>> ...
>>   if (AOTClassLinking && is_dumping_final_static_archive()) {
>>     FLAG_SET_ERGO_IF_DEFAULT(StoreAOTCode, true);
>> 
>> 
>> First, I am not sure these conditions are correct.
>> 
>> Second, it would be nice to have simple checks instead: `is_dumping_aot_archive()` and `is_using_aot_archive()`.
>> 
>> May be also consider it is error if both conditions are true (we don't support updating archive yet).
>
>> @iklam one annoying thing in current ergonomic setting for AOTCode flags in mainline is checking which phase we are executing. We agreed before that we should only save/load AOT code when `AOTClassLinking` is on because AOT code needs classes to be preloaded.
>> 
>> I have to do next checks to enable AOTCode in `CDSConfig::check_vm_args_consistency()`:
>> 
>> ```
>>   if (AOTClassLinking && is_using_archive() && !is_dumping_archive() && !FLAG_IS_DEFAULT(AOTCache)) {
>>     FLAG_SET_ERGO_IF_DEFAULT(LoadAOTCode, true);
>> ...
>>   if (AOTClassLinking && is_dumping_final_static_archive()) {
>>     FLAG_SET_ERGO_IF_DEFAULT(StoreAOTCode, true);
>> ```
>> 
>> First, I am not sure these conditions are correct.
>> 
>> Second, it would be nice to have simple checks instead: `is_dumping_aot_archive()` and `is_using_aot_archive()`.
>> 
>> May be also consider it is error if both conditions are true (we don't support updating archive yet).
> 
> There are a lot of dependencies between different AOT capabilities, and it's hard to control that using global variables. At the point of `CDSConfig::check_vm_args_consistency()`, we don't have complete knowledge whether the AOT cache exists, or whether the cache contains AOT code, or whether the GC compressed oops settings are compatible with the AOT code.
> 
> In the handling of such "AOT capability flags", I have been using the following pattern:
> 
> In `CDSConfig::check_vm_args_consistency()` we update the default values of the flags according to their dependencies on other flags. E.g., by specifying `-XX:AOTMode=create`, `AOTClassLinking` and `AOTInvokeDynamicLinking` are enabled by default.
> 
> 
>   if (!FLAG_IS_DEFAULT(AOTMode)) {
>     // Using any form of the new AOTMode switch enables enhanced optimizations.
>     FLAG_SET_ERGO_IF_DEFAULT(AOTClassLinking, true);
>   }
> 
>   if (AOTClassLinking) {
>     // If AOTClassLinking is specified, enable all AOT optimizations by default.
>     FLAG_SET_ERGO_IF_DEFAULT(AOTInvokeDynamicLinking, true);
>   } else {
>     // AOTInvokeDynamicLinking depends on AOTClassLinking.
>     FLAG_SET_ERGO(AOTInvokeDynamicLinking, false);
>   }
> 
> 
> However, the values of these flags are just advisory. Even if a flag is enabled, the underlying capability may be disabled. For example, `AOTClassLinking` requires the ability of dumping heap objects, which is not available if ZGC is used.
> 
> Because the dependencies are complex, it's difficult to resolve them statically and set a global boolean variable for each capability. Instead, I have been expres...

> Thank you @iklam for explanation. I can do final adjustment to `Store|LoadAOTCode` flags values in `StoreAOTCode::initialize()` which is called from `initialize_shared_spaces()`:
> 
> ```
> MetaspaceShared::initialize_shared_spaces() {
> ...
>   static_mapinfo->patch_heap_embedded_pointers();
>   ArchiveHeapLoader::finish_initialization();
>   Universe::load_archived_object_instances();
> +  AOTCodeCache::initialize();
> ```
> 
> The question: at this place are all CDS AOT flags are final (flags compatibility and cache presence are verified)?
> 
> Note, `Store|LoadAOTCode` flags are diagnostic and disabled by default. I need to set them to `true` somewhere.

Yes, at this point all configuration related to AOT should be final. You can set the final values for the `Store|LoadAOTCode` flags here.

`StoreAOTCode` should be true only if `CDSConfig::is_dumping_final_static_archive()` is true. `LoadAOTCode` should be true only if `CDSConfig::is_loading_archive()` is true and the archive contains AOT code.

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

PR Comment: https://git.openjdk.org/jdk/pull/24401#issuecomment-2781681413