On Thu, 24 Oct 2024 03:01:54 GMT, Ioi Lam <ik...@openjdk.org> wrote:
>> This is an implementation of [JEP 483: Ahead-of-Time Class Loading &
>> Linking](https://openjdk.org/jeps/483).
>>
>> ----
>> Note: this is a combined PR of the following individual PRs
>> - https://github.com/openjdk/jdk/pull/20516
>> - https://github.com/openjdk/jdk/pull/20517
>> - https://github.com/openjdk/jdk/pull/20843
>> - https://github.com/openjdk/jdk/pull/20958
>> - https://github.com/openjdk/jdk/pull/20959
>> - https://github.com/openjdk/jdk/pull/21049
>> - https://github.com/openjdk/jdk/pull/21143
>
> Ioi Lam has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - 8342907: Implement AOT testing mode for jtreg tests (authored by @katyapav)
> - disable test that fails with hotspot_runtime_non_cds_mode
src/hotspot/share/cds/aotConstantPoolResolver.cpp line 113:
> 111:
> 112: if (CDSConfig::is_dumping_aot_linked_classes()) {
> 113: // Need to call try_add_candidate instead of is_candidate, as this
> may be called
I think in this version of the code this method is not used before
`AOTClassLinker::add_candidates`. Can we switch back to `is_candidate` then?
src/hotspot/share/cds/heapShared.hpp line 298:
> 296: static SeenObjectsTable *_seen_objects_table;
> 297:
> 298: // The "special subgraph" contains all the all archived objects that
> are reachable
an extra `all` in the comment
src/hotspot/share/classfile/systemDictionaryShared.cpp line 685:
> 683: InstanceKlass* ik = InstanceKlass::cast(k);
> 684:
> 685: if (SafepointSynchronize::is_at_safepoint()) {
Why is this piece of block required?
It calls `is_excluded_class` which reads `DumpTimeClassInfo::_excluded` without
checking for `has_checked_exclusion`. That means it can return false (the
default value) even for classes that may later be marked for exclusion by
`check_for_exclusion(ik, p)`.
<del>On the same note, I think we should add an assert in
`DumpTimeClassInfo::is_excluded` that `has_checked_exclusion()` is true.<del>
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1816853423
PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1816996074
PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1817159936
