On Tue, 1 Jul 2025 23:20:59 GMT, Chen Liang <li...@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 incrementally with one additional
> commit since the last revision:
>
> Reviewed the diff on github
Changes requested by iklam (Reviewer).
src/hotspot/share/cds/aotClassInitializer.cpp line 41:
> 39:
> 40: // Tell if ik is marked as AOT initialization safe via
> @jdk.internal.vm.annotation.AOTSafeClassInitializer.
> 41: bool AOTClassInitializer::allows_aot_initialization(InstanceKlass* ik) {
Instead of doing the asserts here, I think the checks should be done in
`ClassFileParser`: if a class is marked `@AOTSafeClassInitializer` annotation,
then each of its supertypes must either be marked `@AOTSafeClassInitializer`,
or have no `<clinit>`. Otherwise an exception should be thrown
(ClassFileFormatError??).
This will be a good reminder that if you mark a class as
`@AOTSafeClassInitializer`, you should check the `<clinit>` of all of its
supertypes.
Then, this function can be removed, and the caller can call
`ik->has_aot_safe_initializer()` directly.
src/java.base/share/classes/java/lang/invoke/ClassSpecializer.java line 68:
> 66: * @param <S> species data type.
> 67: */
> 68: @AOTSafeClassInitializer
This should be placed below the `/*non-public*/` comment.
src/java.base/share/classes/jdk/internal/vm/annotation/AOT.md line 47:
> 45: because the result of loading an AOT cache must look like a valid,
> 46: standard-conformant execution of the JVM. But a heap object with an
> 47: uninitialized class is an impossibility within the JVMS.
We should resolve the differences between this file and
AOTSafeClassInitializer.java. The comments in the latter describes the current
implementation, whereas this file describes what we want in the future.
See
https://github.com/openjdk/jdk/blob/7d7e60c8aebc4b4c1e7121be702393e5bc46e9ce/src/hotspot/share/cds/heapShared.cpp#L317-L331
In the current implementation, only the objects in the "special object graph"
have their classes marked as "aot-initialized". The "special object graph" is
described here:
https://github.com/openjdk/jdk/blob/7d7e60c8aebc4b4c1e7121be702393e5bc46e9ce/src/hotspot/share/cds/heapShared.hpp#L274-L279
It essentially corresponds to the "objects directly or indirectly referenced by
resolved constant pool entries" by the comments in
AOTSafeClassInitializer.java.
src/java.base/share/classes/jdk/internal/vm/annotation/AOTSafeClassInitializer.java
line 53:
> 51: /// entries when it's safe and beneficial to do so.
> 52: ///
> 53: /// An AOT-resolved constant pool entry for an invokedynamic or
> invokehandle bytecodes can
bytecodes -> bytecode
-------------
PR Review: https://git.openjdk.org/jdk/pull/25922#pullrequestreview-2977642139
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2179033016
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2179038052
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2179049071
PR Review Comment: https://git.openjdk.org/jdk/pull/25922#discussion_r2179050047
