RFR: 8331497: Implement JEP 483: Ahead-of-Time Class Loading & Linking [v4]

Ioi Lam iklam at openjdk.org
Thu Oct 31 02:09:33 UTC 2024


On Fri, 25 Oct 2024 15:02:30 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:

>> 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/aotClassLinker.cpp line 117:
> 
>> 115: 
>> 116: void AOTClassLinker::add_candidate(InstanceKlass* ik) {
>> 117:   _candidates->put_when_absent(ik, true);
> 
> I just noticed the use of `put_when_absent` here. This means the caller should ensure that `ik` has not already been added to the `_candidates`. We do that currently (`try_add_candidate` calls `is_candidate` before calling `add_candidate`) but probably worth mentioning this contract explicitly in a comment somewhere for future readers.

I renamed it to `add_new_candidate()` and added comments so that it's clear what the contract is. I also added assert to check for thread safety.

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

In the future, these functions may be called before we enter the safepoint (e.g., to check if some constant pool entries can be resolved), so I think it's better to keep the `try_add_candidate()` call.

> 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

Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1823682059
PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1823684639
PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1823684779


More information about the serviceability-dev mailing list