RFR: 8360163: Replace hard-coded checks with AOTRuntimeSetup and AOTSafeClassInitializer [v11]

[John R Rose]

On Wed, 9 Jul 2025 13:25:22 GMT, Chen Liang <liach at openjdk.org> wrote:

>> I have updated this patch to avoid a redundant `runtimeSetup` annotation - we have agreed that the requirement for setup is a side effect of initialization, and such methods in AOTCI classes must be automatically recognized. This latest revision implements that model.
>> 
>> I intentionally avoided handling Class and ClassLoader `resetArchivedStates` and `MethodType::assemblySetup` - we talked about a generic `assemblyCleanup` method, but I did not find out where is the best place to call such a method in the assembly phase. We cna handle this in a subsequent patch.
>> 
>> In particular, please review the new AOT.md design document - I split it from the AOTCI annotation to prevent jamming; we can put general AOT information there when we have more AOT-specific annotations.
>> 
>> ---
>> 
>> Old description:
>> Currently, the list of classes that have <clinit> interdependencies and those that need runtimeSetup are maintained in a hardcoded list in CDS. This makes it risky for core library developers as they might introduce new interdependencies and observe CDS to fail. By moving the mechanism of these lists to core library annotations as a first step, we can gradually expose the AOT contracts as program semantics described by internal annotations, and also helps us to explore how we can expose these functionalities to the public later.
>
> Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 24 commits:
> 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into exp/cds-mh-anno
>  - Merge branch 'master' of https://github.com/openjdk/jdk into exp/cds-mh-anno
>  - Reviews
>  - Documentation
>  - Don't fail for patching tests
>  - Remove design document from code
>  - Some more from reviews
>  - Reviewed the diff on github
>  - Stage
>  - Missed comment updates
>  - ... and 14 more: https://git.openjdk.org/jdk/compare/a201be85...d2dd466b

Good fix.  I added some minor comments.  No logic changes requested!

I'm a little surprised to see the second annotation has survived; I'd prefer to see it totally nuked.  But I am not asking you to remove it; there is an argument that it is helpful.  I requested better documdentation and checking that the two annotations are properly interdependent.  The "runtime setup" annotation should not be allowed except in classes marked as AOT safe.

In a few places you removed an unrelated "non-public" comment.  That is not a good cleanup to incorporate in passing, since the comment is actually functional.

If somebody is arguing that such comments are somehow detrimental, let them make a separate argument and a separate PR to purge them.

src/hotspot/share/classfile/classFileParser.cpp line 5179:

> 5177:       log_info(aot, init)("Found @AOTSafeClassInitializer class %s", ik->external_name());
> 5178:     }
> 5179:   }

This would be a good place to validate that the runtime-setup annotation is only placed on `@AOTSCI` classes.

src/java.base/share/classes/java/lang/invoke/BoundMethodHandle.java line 47:

> 45:  * All bound arguments are encapsulated in dedicated species.
> 46:  */
> 47: /*non-public*/

please keep this comment intact; it reminds maintainers that the non-public access of this class is not accidental

src/java.base/share/classes/java/lang/invoke/ClassSpecializer.java line 67:

> 65:  * @param <S> species data type.
> 66:  */
> 67: /*non-public*/

please keep this comment intact; it reminds maintainers that the non-public access of this class is not accidental

src/java.base/share/classes/java/lang/invoke/DelegatingMethodHandle.java line 39:

> 37:  * @author jrose
> 38:  */
> 39: /*non-public*/

please keep this comment intact; it reminds maintainers that the non-public access of this class is not accidental

src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 75:

> 73:  * @author jrose
> 74:  */
> 75: /*non-public*/

please keep this comment intact; it reminds maintainers that the non-public access of this class is not accidental

src/java.base/share/classes/jdk/internal/vm/annotation/AOTRuntimeSetup.java line 39:

> 37: ///
> 38: /// `classFileParser.cpp` performs checks on the annotated method - if the
> 39: /// annotated method's signature differs from that described above, a

add:

 /// `classFileParser.cpp` performs checks on the annotated method - if the
 /// annotated method's signature differs from that described above,
+/// or if the enclosing class is not annotated `@AOTSafeClassInitializer`,
 /// a [ClassFormatError] will be thrown.


Otherwise, there are just some late-firing asserts that catch the error.
But the dependency of one annotation on the other should be documented up front.

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

Changes requested by jrose (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25922#pullrequestreview-3050063235
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2227419561
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2227480716
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2227481353
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2227482049
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2227484671
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2227401171