On Thu, 3 Apr 2025 22:02:01 GMT, Vladimir Kozlov <[email protected]> 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).
>>
>> 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 globa...
>
> 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.
Thanks @vnkozlov @ashu-mehra @DanHeidinga @lmesnik for the review
-------------
PR Comment: https://git.openjdk.org/jdk/pull/24401#issuecomment-2790037592
