RFR: 8348426: Generate binary file for -XX:AOTMode=record -XX:AOTConfiguration=file [v8]

[Ioi Lam]

On Tue, 25 Feb 2025 05:56:59 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:

>> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
>> 
>>  - all tests in runtime/cds/appcds/aotClassLinking should be excluded for hotspot_appcds_dynamic testing
>>  - @ashu-mehra comment - simplified _archived_cpp_vtptrs; also fixed old comments near by
>>  - Merge branch 'master' into 8348426-binary-aot-config-file
>>  - Merge branch 'master' into 8348426-binary-aot-config-file
>>  - @ashu-mehra comments
>>  - @calvinccheung comments
>>  - Improved JTREG_AOT_JDK=true so we do not need to add test code into the JDK itself
>>  - Improve error message when AOTMode=create has an incompatible classpath
>>  - Fixed test cases @vnkozlov
>>  - Update "make test JTREG_AOT_JDK=true ..." to work with binary AOT configuration
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/990d40e9...1ec67c11
>
> src/hotspot/share/cds/cppVtables.cpp line 293:
> 
>> 291:     for (kind = 0; kind < _num_cloned_vtable_kinds; kind ++) {
>> 292:       if (vtable_of((Metadata*)obj) == _orig_cpp_vtptrs[kind] ||
>> 293:           vtable_of((Metadata*)obj) == _archived_cpp_vtptrs[kind]) {
> 
> I think we should check these conditions only in the mode where applicable. That would make it easier to understand the code in future. So my suggestion is to update it as:
> 
> 
>     for (kind = 0; kind < _num_cloned_vtable_kinds; kind ++) {
>       intptr_t* vtable_ptr;
>       if (CDSConfig::is_dumping_final_static_archive()) {
>         vtable_ptr = _archived_cpp_vtptrs[kind];
>       } else {
>         vtable_ptr = _orig_cpp_vtptrs[kind];
>       }
>       if (vtable_of((Metadata*)obj) == vtable_ptr) {
>         break;
>       }
>     }

During the final archive dump, we can have both archived classes from the preimage as well as dynamically loaded classes. For example, hidden classes are generated when linking lambdas.

> src/hotspot/share/cds/cppVtables.cpp line 322:
> 
>> 320:   assert(MetaspaceShared::is_in_shared_metaspace(m), "must be");
>> 321:   return vtable_of(m) == _index[Method_Kind]->cloned_vtable() ||
>> 322:          vtable_of(m) == _archived_cpp_vtptrs[Method_Kind];
> 
> I am not sure if this needs any fixing. If `m` is in the archive (as the above assert says), then its vtable should always be `_index[Method_Kind]->cloned_vtable()`.

`_index` is updated to use the runtime vtables after `CppVtables::dumptime_init()`, so we must check `_archived_cpp_vtptrs` as well. This is a new condition that can happen when dumping the final archive.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23484#discussion_r1969000603
PR Review Comment: https://git.openjdk.org/jdk/pull/23484#discussion_r1969000378