On Fri, 19 Sep 2025 02:05:46 GMT, Ioi Lam <[email protected]> wrote: >>> Note, the check of the above requirement has been moved to >>> AOTClassInitializer::check_aot_annotations(). The previous check in >>> ClassFileParser was not executed because the class is loaded in the AOT >>> training run, where CDSConfig::is_initing_classes_at_dump_time() returns >>> false (this function returns true only in the AOT assembly phase). >> >> So this is a bug already present in the code and effectively disables super >> type checks for AOTSafeClassInitializer annotation, is that right? >> There is a reference to ClassFileParser in the documentation for >> AOTSafeClassInitializer. I think it needs to be updated as well: >> https://github.com/openjdk/jdk/blob/18dc186a8f4820ed78c21173713dd127ef512e1f/src/java.base/share/classes/jdk/internal/vm/annotation/AOTSafeClassInitializer.java#L124-L129 > >> > Note, the check of the above requirement has been moved to >> > AOTClassInitializer::check_aot_annotations(). The previous check in >> > ClassFileParser was not executed because the class is loaded in the AOT >> > training run, where CDSConfig::is_initing_classes_at_dump_time() returns >> > false (this function returns true only in the AOT assembly phase). >> >> So this is a bug already present in the code and effectively disables super >> type checks for AOTSafeClassInitializer annotation, is that right? > > Yes, that's a bug in the current code in the mainline. > >> There is a reference to ClassFileParser in the documentation for >> AOTSafeClassInitializer. I think it needs to be updated as well: >> >> https://github.com/openjdk/jdk/blob/18dc186a8f4820ed78c21173713dd127ef512e1f/src/java.base/share/classes/jdk/internal/vm/annotation/AOTSafeClassInitializer.java#L124-L129 > > I updated the requirements about safety and supertypes for both > `AOTInitialize` and `AOTSafeClassInitializer` to use the exact same wording. > I think it's OK to describe the effect (the AOT assembly phase will fail) > without the details about where we do the checks: > > > /// Before adding this annotation to a class _X_, the author must determine > /// that it's safe to execute the static initializer of _X_ during the AOT > /// assembly phase. In addition, all supertypes of _X_ must also have this > /// annotation. If a supertype of _X_ is found to be missing this annotation, > /// the AOT assembly phase will fail.
> @iklam Reading up the description of the bug tracker, it seems we are trying > to find out more opportunities to AOT initialize classes than the current > state. Is that correct? I may be missing something here, why not "force > initialize" classes annotated with `AOTSafeClassInitializer` in > `AOTMetaspace::try_link_class`, instead of introducing new annotation? > Wouldn't that serve the same purpose? And I think we also need to track the > classes that are actually initialized in the training phase, and apply "force > initialization" only on that subset. Otherwise we can end up in situation > where a classes is loaded and not initialized in training phase, but still > gets AOT initialized because it is annotated with `AOTInitialize`. I think this is a better alternative. I will fix the implementation to do what you described (by tracking the set of initialized classes in finalImageRecipes..cpp). To avoid confusion, I will post a separate PR. ------------- PR Comment: https://git.openjdk.org/jdk/pull/27024#issuecomment-3314360461
