RFR: 8317269: Store old classes in linked state in AOT cache [v2]

[Coleen Phillimore]

On Wed, 13 Aug 2025 06:22:15 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> During the assembly phase of the AOT cache, we link and verify all classes that were loaded during the training run. When verifying a class like this:
>> 
>> 
>> class X {
>>     A getA() { return new B(); } // Verifier requires B to be a subtype of A.
>> }
>> 
>> 
>> We remember `A` and `B` as the "verification dependencies" of `X`. A class will be excluded from the AOT cache if any of its verification dependencies are excluded. For example, `X` will be excluded if
>> 
>> - `A` fails verification
>> - `B` is a signed class, which is always excluded from the AOT cache
>> 
>> Conversely, if both `A` and `B`  are included in the AOT cache, in the production run, they will be unconditionally loaded during VM bootstrap. Therefore, we can guarantee that the verification result computed for `X` will remain valid during the production run.
>> 
>> Notes for reviewers:
>> 
>> - The checks for verification dependencies are done inside `SystemDictionaryShared::check_exclusion_for_self_and_dependencies()`. These checks are done for both old and new classes. Since the dependencies can form a cyclic graph, the checks cannot be implemented with a simple recursion. See "Algorithm notes" in this function for details. 
>> - The verification dependencies for "new" classes are already stored in `DumpTimeClassInfo::_verifier_constraints`.
>> - This PR adds code to record the verification dependencies for "old" classes into `DumpTimeClassInfo::_old_verifier_dependencies`, by intercepting `JVM_FindClassFromCaller()`.
>> - This new functionality (store old classes in linked state) is available only when using the AOT cache. For simplicity, this functionality is not available with the old CDS workflows. See comments in `CDSConfig::is_preserving_verification_dependencies()`.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed bugs found in JCK testing

I have a few initial comments and only skimmed the rest.  I didn't imagine there'd be so much code for this.

src/hotspot/share/cds/dumpTimeClassInfo.cpp line 90:

> 88:     _old_verifier_dependencies = new (mtClass) GrowableArray<Symbol*>(4, mtClass);
> 89:   }
> 90:   _old_verifier_dependencies->append(name);

I wonder why there's append and push that do the same thing in GrowableArray.  I'm used to seeing push().

src/hotspot/share/cds/metaspaceShared.cpp line 636:

> 634: 
> 635: void VM_PopulateDumpSharedSpace::doit() {
> 636:   CDSConfig::set_is_at_cds_safepoint(true);

Can you just use SafepointSynchronize::is_at_safepoint()?  Why new flag?

src/hotspot/share/classfile/systemDictionaryShared.cpp line 264:

> 262:     }
> 263: 
> 264: 

extra blank lines.

src/hotspot/share/runtime/mutexLocker.cpp line 309:

> 307:   MUTEX_DEFN(DumpTimeTable_lock              , PaddedMutex  , nosafepoint);
> 308:   MUTEX_DEFN(CDSLambda_lock                  , PaddedMutex  , nosafepoint);
> 309:   MUTEX_DEFN(DumpRegion_lock                 , PaddedMutex  , nosafepoint-1); // Holds DumpTimeTable_lock

There's a MUTEX_DEFL where you can define the mutex in terms of the locks that it takes out.

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

PR Review: https://git.openjdk.org/jdk/pull/26754#pullrequestreview-3129452422
PR Review Comment: https://git.openjdk.org/jdk/pull/26754#discussion_r2286327664
PR Review Comment: https://git.openjdk.org/jdk/pull/26754#discussion_r2286332210
PR Review Comment: https://git.openjdk.org/jdk/pull/26754#discussion_r2286336645
PR Review Comment: https://git.openjdk.org/jdk/pull/26754#discussion_r2286347290