On Thu, 3 Apr 2025 21:40:48 GMT, Ioi Lam <ik...@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.

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

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

Reply via email to