On Thu, 16 Apr 2026 23:09:52 GMT, Vladimir Kozlov <[email protected]> wrote:
> Improve startup and warmup time by making optimized native code for an > application instantly available when the HotSpot Java Virtual Machine starts. > Achieve this by compiling application code to native code in a training run, > storing the native code in the [AOT > cache](https://openjdk.org/jeps/483#Description) for use in subsequent > production runs. > > More details in the [JEP](https://openjdk.org/jeps/544). > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). More things from code analyzers run... I can take a look at Shenandoah parts, if you want. We probably just missing a few C1 stubs for it? If you could merge from master, I can kick off another code analyzer run. src/hotspot/cpu/x86/macroAssembler_x86.cpp line 5737: > 5735: // The best case scenario is that there is no base or shift. Then > it is already > 5736: // a pointer that needs nothing but a register rename. > 5737: movptr(dst, src); Hm. So previous thing was zero-extending 32-bit value (e.g. narrow-oop). This one does 64-bit move, so it implicitly assumes higher 32-bits are zeroes. The existing comment is a bit misleading. Do we actually need this for AOT to work? src/hotspot/share/c1/c1_Compiler.cpp line 258: > 256: CompileTask* task = env->task(); > 257: if (install_code && task->is_aot_load()) { > 258: assert(!task->preload(), "Pre-loading AOT code is not implemeted for > C1 code"); Suggestion: assert(!task->preload(), "Pre-loading AOT code is not implemented for C1 code"); src/hotspot/share/cds/cdsConfig.cpp line 122: > 120: // > 121: // By default, when using AOTClassLinking, use the > CompressedOops::HeapBasedNarrowOop > 122: // mode so that AOT code can be always work regardless of runtime heap > range. Suggestion: // mode so that AOT code can always work regardless of runtime heap range. src/hotspot/share/cds/cds_globals.hpp line 159: > 157: > \ > 158: product(bool, AOTCodeCaching, false, DIAGNOSTIC, > \ > 159: "Enable saving and restoring JIT comiled code in AOT cache") > \ `compiled` src/hotspot/share/ci/ciEnv.cpp line 1124: > 1122: // No safepoints are allowed. Otherwise, class redefinition can > occur in between. > 1123: MutexLocker ml(Compile_lock); > 1124: NoSafepointVerifier nsv; OK, so here is an interesting lifecycle oddity. We are here arming `NSV` for class redefinition reasons. Looks fine until we go to `ciEnv::make_code_usable`, which builds `MCS` for preloaded methods. But `MCS` allocation is _Metaspace_ allocation, so it can trigger GC allocation failure and GC safepoint. So this `NSV` may fail. src/hotspot/share/code/aotCodeCache.cpp line 289: > 287: // It is called from AOTMetaspace::initialize_shared_spaces() > 288: // which is called from universe_init(). > 289: // At this point all AOT class linking seetings are finalized Suggestion: // At this point all AOT class linking settings are finalized src/hotspot/share/code/aotCodeCache.cpp line 737: > 735: > 736: size_t codeCacheSize = pointer_delta(CodeCache::high_bound(), > CodeCache::low_bound(), 1); > 737: if (codeCacheSize > _codeCacheSize) { // Only allow smaller or equal > CodeCache size in production run It is not very clear to me why do we have this limit. What breaks if we have a larger code cache in production run? Some branches become not easily reachable, or something else? Let's polish the error message too: we need to say "larger" or "smaller" explicitly, so users can figure out what to adjust. src/hotspot/share/code/aotCodeCache.cpp line 746: > 744: return false; > 745: } > 746: if ((_compressedKlassBase == nullptr || > CompressedKlassPointers::base() == nullptr) && (_compressedKlassBase != > CompressedKlassPointers::base())) { Wait, so this check only fires when _either_ of bases is `nullptr`? So if both bases are not null and disagree with each other, we just pass this check? Sounds like we want just the plain `==` here? src/hotspot/share/code/aotCodeCache.hpp line 97: > 95: // Next field is exposed to external profilers - keep it as boolean. > 96: bool _for_preload; // Code can be used for preload (before > classes initialized) > 97: uint8_t _has_clinit_barriers:1, // Generated code has class init checks > (only in for_preload code) Same as nmethod bitfield, this yields a bitfield data race, which is straight up UB in C++. It would have been "fine" if field updates were under the same lock. But they are not: `set_not_entrant()` sets it under `NMethodState_lock`, `set_loaded()` sets another bit under `Compile_lock`, at very least. So break it up in separate fields? src/hotspot/share/code/aotCodeCache.hpp line 494: > 492: // Here should be version and other verification fields > 493: enum { > 494: AOT_CODE_VERSION = 1 Since we are extending the AOT code cache layout, it stands to reason we need to bump the version to catch incompatibilities? Although I suspect VM version fingerprinting already protects us from most of the problems. src/hotspot/share/code/nmethod.cpp line 1900: > 1898: const char* nm_kind = compile_kind(); > 1899: if (nm_kind != nullptr) log->print(" compile_kind='%s'", nm_kind); > 1900: log->print(" compile_kind='%s'", nm_kind); There is already the printout for `compile_kind` one line above. src/hotspot/share/code/nmethod.cpp line 1903: > 1901: log->print(" compiler='%s'", compiler_name()); > 1902: if (TieredCompilation) { > 1903: log->print(" compile_level='%d'", comp_level()); Do we really want to change XML output like this? Probably yields compatibility issues. src/hotspot/share/code/nmethod.hpp line 285: > 283: _load_reported:1, // used by jvmti to track if an > event has been posted for this nmethod > 284: _preloaded:1, > 285: _has_clinit_barriers:1; Not sure if we want to solve this here, but I think we update these bitfields separately without consistent synchronization. So there is a possibility we can stomp some bits if we do updates without locks. (Sighs) We should really fix it in mainline: https://bugs.openjdk.org/browse/JDK-8383954 src/hotspot/share/compiler/compiler_globals.hpp line 395: > 393: product(uint, DisableAOTCode, 0, DIAGNOSTIC, > \ > 394: "Disable AOT code on some compilation levels " > \ > 395: "(T1=1; T2=2; T4=4; T5/preload=8") > \ Suggestion: "(T1=1; T2=2; T4=4; T5/preload=8)") \ src/hotspot/share/compiler/compiler_globals.hpp line 400: > 398: "Produce AOT preload code which could be called on first " > \ > 399: "method invocation, add class initialization barriers, " > \ > 400: "other checks and constrains if needed " > \ Suggestion: "other checks and constraints if needed " \ src/hotspot/share/compiler/compiler_globals.hpp line 429: > 427: product(bool, PreloadBlocking, false, DIAGNOSTIC, > \ > 428: "Preload code is processed with blocking. Startup would not " > \ > 429: "proceed until all code preloaded code is done loading.") > \ Suggestion: "proceed until all preloaded code is done loading.") \ ------------- PR Review: https://git.openjdk.org/jdk/pull/30778#pullrequestreview-4236293571 PR Comment: https://git.openjdk.org/jdk/pull/30778#issuecomment-4268257914 PR Comment: https://git.openjdk.org/jdk/pull/30778#issuecomment-4412872591 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195799060 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195755556 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3196014533 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195750818 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195937905 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195756843 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195888902 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195821826 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3196039499 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195740408 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195645990 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195959558 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195728696 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3195752938 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3196007202 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3196010743
