On Wed, 9 Jul 2025 13:25:22 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 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 @liach Sorry for the delay. I looked through your changes and it's good to see that we are moving away from the hard-code list of classes, etc in the VM code for archiving/caching initialized states. The mechanism of marking class/methods/fields in the Java source is a good start for the more general support. A few suggestions (or comments) for future considerations not don't necessarily to be included in your current PR: - heapShared.cpp contains a list of hard coded classes & fields for archiving the initialized states in CDS archive. With the support for using annotation for pre-init being added to the mainline, the support can be applied to those classes as well. - There could be cases where AOTC is not enabled but CDS or heap archiving with pre-initialization is enabled, it's good to consider more general naming for the annotations rather than using AOT-only-context based annotations, like `AOTSafeClassInitializer` and `AOTRuntimeSetup`. - As discussed during Leyden/premain meetings, there are classes/fields that are not suitable for pre-init and caching, such as Thread, Random, System, etc. We want to annotate or mark them so AOTC or CDS archiving ensures those states are not cached. - It's important to enhance the heap archiving mechanism to safeguard the cached object graphs to make sure they can be materialized safely for runtime usages without any unexpected side-effects. Restrictions/assumptions such as the ones that you documented in AOTSafeClassInitializer.java should be built into the heap archiving and ensured at AOT assembling phase or CDS archive time. We can look into extracting some of the parts from https://github.com/jianglizhou/jdk11u-fast-startup-incubator for the heap archiving related enhancements. /// `@AOTSafeClassInitializer`. This should be done when: /// /// 1. It's possible for an artifact used in the linking java.lang.invoke primitives /// (usually a MethodHandle) to directly or indirectly remember the value of a static /// field in this class. /// 2. You have validated that the static initializer of this class doesn't depend on /// transient states (i.e., names of temporary directories) that cannot be carried over /// to a future production run. /// 3. All supertypes of this class must also have the `@AOTSafeClassInitializer` /// annotation. ------------- PR Review: https://git.openjdk.org/jdk/pull/25922#pullrequestreview-3036852633
