RFR: 8344140: Refactor the discovery of AOT cache artifacts [v5]

[Ioi Lam]

On Mon, 13 Jan 2025 21:27:17 GMT, Calvin Cheung <ccheung 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 11 commits:
>> 
>>  - Updated copyright
>>  - Merge branch 'master' into 8344140-consolidate-aot-artifact-gathering-discovery
>>  - @ashu-mehra comments
>>  - Merge branch 'master' into 8344140-consolidate-aot-artifact-gathering-discovery
>>  - Merge branch 'master' of https://github.com/openjdk/jdk into 8344140-consolidate-aot-artifact-gathering-discovery
>>  - Fixe 32-bit builds; removed unused function
>>  - more clean up
>>  - tmp
>>  - More clean up
>>  - Removed unnecessary code
>>  - ... and 1 more: https://git.openjdk.org/jdk/compare/31452788...e246276c
>
> src/hotspot/share/cds/aotArtifactFinder.hpp line 71:
> 
>> 69: // Note1: See TODO comments in HeapShared::archive_object() for exceptions to this rule.
>> 70: //
>> 71: // Note2: The scanning of Java objects is done in heapShared.cpp. Please see calls into the HeapShared class
> 
> Note2 isn't referenced by the comment above. Perhaps line 62 should reference it?

Fixed.

> src/hotspot/share/cds/heapShared.cpp line 320:
> 
>> 318:     if (k->is_instance_klass()) {
>> 319:       // Whenever we see a non-array Java object of type X, we mark X to be aot-initialized.
>> 320:       // This ensures that during the production run, whenever Java code seens a cached object
> 
> typo: s/seens/sees

Fixed.

> src/hotspot/share/cds/heapShared.cpp line 327:
> 
>> 325:         // we must store them as AOT-initialized.
>> 326:         AOTArtifactFinder::add_aot_inited_class(InstanceKlass::cast(k));
>> 327:       } else if (subgraph_info == _dump_time_special_subgraph) {
> 
> Can this be an "or" condition rather than "else if" since the same function is called for either on of the conditions?

Fixed.

> src/hotspot/share/cds/heapShared.cpp line 784:
> 
>> 782: 
>> 783:   if (orig_k->is_instance_klass()) {
>> 784:     InstanceKlass* ik = InstanceKlass::cast(orig_k);
> 
> Should this be inside a "#if ASSERT" since `ik` is used only in the `assert` statements?

Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22291#discussion_r1913998325
PR Review Comment: https://git.openjdk.org/jdk/pull/22291#discussion_r1913998305
PR Review Comment: https://git.openjdk.org/jdk/pull/22291#discussion_r1913998270
PR Review Comment: https://git.openjdk.org/jdk/pull/22291#discussion_r1913998258