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.
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
