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

[Ioi Lam]

On Fri, 29 Aug 2025 21:48:54 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains nine additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8317269-store-old-classes-in-linked-state-in-aot-cache
>>  - @matias9927 comments: fixed typos
>>  - clean up; removed unrelated change
>>  - Cleaned up iterate_verification_dependency_names so a return value of false from <func> means early termination (similar to HashTable::iterate())
>>  - Merge branch 'master' into 8317269-store-old-classes-in-linked-state-in-aot-cache
>>  - @coleenp comments
>>  - More bug fixes from JCK testing
>>  - Fixed bugs found in JCK testing
>>  - 8317269: Store old classes in linked state in AOT cache
>
> src/hotspot/share/classfile/systemDictionaryShared.cpp line 469:
> 
>> 467: 
>> 468: // Returns true if the DumpTimeClassInfo::is_excluded() is true for at least one of k's exclusion dependencies.
>> 469: bool SystemDictionaryShared::check_dependencies_exclusion(InstanceKlass* k, DumpTimeClassInfo* info) {
> 
> So this checks one level of InstanceKlass* k's dependencies, and the table above collects one level of k or k's subclass's dependencies and calls this for those.
> 
> class K : extends B implements I, J verifies C {} - the above hashtable has B, I and J and C.
> 
> This function takes B - so checks B class.
> 
> class B : extends A implements intf verifies C again, why not {}
> 
> So it'd check A and intf and C again.  But when do we check A's super?
> 
> It seems like the hashtable should be created for all levels of the hierarchy, so only one bit of code should walk the hierarchy, not two.

The ExclusionCheckCandidates table contains all (recursive) supertypes -- E.g., its `add_candidate(k)` method calls `add_candidate(k->java_super())`.

> src/hotspot/share/classfile/systemDictionaryShared.cpp line 542:
> 
>> 540: }
>> 541: 
>> 542: Klass* SystemDictionaryShared::find_verification_dependency_bottom_class(InstanceKlass* k, Symbol* dependency_class_name) {
> 
> Why don't you have this return InstanceKlasss so that the callers don't have to check the same thing. What you want is the bottom InstanceKlass so for [Lfoo; - you want foo.  Since class foo is the thing you care about.  Maybe the comment about why you return nullptr from check_verification_dependency_exclusion() could be here instead.
> 
> These names are a bit repetative, can you just call this find_bottom_instance_klass() and maybe it could be used for other things too.

The problem is if `k` is the ObjArrayKlass of `[[I`, then `k->bottom_klass()` is `[I`, which is not an InstanceKlass. If we change the return type to `InstanceKlass*`, then we would have to return `nullptr`. However, that would make it more difficult to explain the following case (I've updated the comments):


bool SystemDictionaryShared::check_verification_dependency_exclusion(InstanceKlass* k, Symbol* dependency_class_name) {
  Klass* dependency_bottom_class = find_verification_dependency_bottom_class(k, dependency_class_name);
  if (dependency_bottom_class == nullptr) {
    // Dependency_class_name has not yet been loaded. This happens when the new verifier was checking
    // if dependency_class_name is assignable to an interface, and found the answer without resolving
    // dependency_class_name.
    //
    // Since this class is not even loaded, it surely cannot be excluded.
    return false;
  }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26754#discussion_r2312711042
PR Review Comment: https://git.openjdk.org/jdk/pull/26754#discussion_r2312711119