-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8368174: Proactive initialization of @AOTSafeClassInitializer classes #27402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
8368174: Proactive initialization of @AOTSafeClassInitializer classes #27402
Conversation
👋 Welcome back iklam! A progress list of the required criteria for merging this PR into |
@iklam This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 36 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
return supertype->has_aot_safe_initializer(); | ||
}); | ||
} else { | ||
// @AOTRuntimeSetup only meaningful in @AOTClassInitializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// @AOTRuntimeSetup only meaningful in @AOTClassInitializer | |
// @AOTRuntimeSetup only meaningful in @AOTSafeClassInitializer |
// constant pool entries that were resolved during the training run. | ||
FinalImageRecipes::apply_recipes(CHECK); | ||
|
||
// Because the AOT assembly phase does not run the exact code as in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Because the AOT assembly phase does not run the exact code as in the | |
// Because the AOT assembly phase does not run the exact same code as in the |
/// This is usually the result of performing AOT optimizations for the | ||
/// `java.lang.invoke` package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// This is usually the result of performing AOT optimizations for the | |
/// `java.lang.invoke` package. | |
/// At present this is usually the result of performing AOT optimizations for | |
/// the `java.lang.invoke` package but the set of classes which may be | |
/// pre-initialized via this annotation is not restricted to just that case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much simpler and clearer than the previous version making it easier for non-Leyden devs to understand what they are buying into when they use the annotation. Nice work @ashu-mehra and @iklam.
I made a few suggestions to clarify comments which you are free to adopt or drop as you see fit. Otherwise looks good to go.
check_aot_annotations(ik); | ||
|
||
if (!ik->is_initialized() && !ik->is_being_initialized()) { | ||
if (ik->has_aot_safe_initializer()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we are forcing initialization of classes annotated with AOTSafeClassInitializer
, is it still possible that a class is not initialized but has_aot_safe_initializer()
is true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible in some rare circumstances. I.e, in the assembly phase, we take a path that was not cover in the training run (e.g. some sort of advanced JLI linkage). We could load a class X without initializing it. This could happen if X is used only for instanceof checking, or if it was loaded during verification of other classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant change
This is an alternative to #27024. Thanks to @ashu-mehra for the suggestion.
Background:
The AOT Assembly Phase is in essence a small Java program that executes a limited set of Java bytecodes. This program bootstraps the module system, loads classes, and performs certain ahead-of-time optimizations such as resolving
invokedynamic
call sites.As a side effect of Java program execution, a small set of Java classes are initialized in the Assembly Phase.
Since JDK-8360163, if a class
X
is annotated with@AOTSafeClassInitializer
and is initialized in the Assembly Phase, thenX
will be stored in the AOT cache in the "initialized" state. When the AOT cache is used in the Production Run,X::<clinit>
will not be executed, and the static variables ofX
will be available upon JVM bootstrap.Problem:
The Assembly Phase doesn't touch many classes that may benefit from
@AOTSafeClassInitializer
. For example,jdk.internal.math.MathUtils::<clinit>
creates a few large tables. CachingMathUtils
in the "initialized" state will improve start-up time. However, since no bytecodes executed by the Assembly Phase useMathUtils
. it will not be initialized.Fix:
If a class
X
has the@AOTSafeClassInitializer
annotation and was initialized in the AOT Training Run, the JVM will proactively initializeX
in the Assembly Phase. This will ensure thatX
will be cached in the "initialized" state.As a proof of concept,
@AOTSafeClassInitializer
is added toMathUtils
.@AOTSafeClassInitializer
will be added to more classes in future RFEs.Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27402/head:pull/27402
$ git checkout pull/27402
Update a local copy of the PR:
$ git checkout pull/27402
$ git pull https://git.openjdk.org/jdk.git pull/27402/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27402
View PR using the GUI difftool:
$ git pr show -t 27402
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27402.diff
Using Webrev
Link to Webrev Comment