On Thu, 17 Sep 2026 21:29:28 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). > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Update AOT code tests timeout value. Move two tests to tier2 Some more review comments. They all come under "clean-up" category, so feel free to leave the code as-is. and they can be addressed later. src/hotspot/share/cds/aotCacheAccess.cpp line 84: > 82: uintptr_t low_bound = p2u(MetaspaceObj::aot_metaspace_base()); > 83: uintptr_t high_bound = p2u(MetaspaceObj::aot_metaspace_top()); > 84: if (base > low_bound || low_bound > high_bound) { // paranoid check These checks can be done just once during AOTCache setup, not needed on every read. src/hotspot/share/cds/aotCacheAccess.cpp line 91: > 89: > 90: size_t offset = AOTCompressedPointers::get_byte_offset(narrowp); > 91: if (offset == 0 || offset > > AOTCompressedPointers::MaxMetadataOffsetBytes || IMO if we want these checks, they should be put in `AOTCompressedPointers::decode/decode_not_null` so that other code in AOTCache is also protected against spurious/corrupted narrowPtr. src/hotspot/share/cds/aotCacheAccess.cpp line 98: > 96: > 97: Metadata* meta = reinterpret_cast<Metadata*>(base + offset); > 98: assert(Metaspace::in_aot_cache(meta), "must be"); Isn't `Metaspace::in_aot_cache(meta)` sufficient to detect spurious/corrupted narrowPtr? src/hotspot/share/cds/aotCacheAccess.hpp line 76: > 74: * The encoded pointer is normally obtained by reading a value embedded > in some other AOT-ed entry, like an AOT compiled code. > 75: */ > 76: static Klass* narrow_ptr_to_klass(narrowPtr narrowp) { This method and `narrow_ptr_to_method` are now redundant. src/hotspot/share/cds/cdsConfig.cpp line 35: > 33: #include "classfile/classLoaderDataShared.hpp" > 34: #include "classfile/moduleEntry.hpp" > 35: #include "classfile/systemDictionaryShared.hpp" This include is not required. src/hotspot/share/cds/cdsConfig.cpp line 789: > 787: } else { > 788: AOTCodeCache::disable_caching(); > 789: // Use separate compilation queues and threads for AOT code loading This comment seems misplaced or redundant now. src/hotspot/share/ci/ciField.cpp line 34: > 32: #include "classfile/javaClasses.hpp" > 33: #include "classfile/vmClasses.hpp" > 34: #include "code/aotCodeCache.hpp" This include is not needed. src/hotspot/share/ci/ciField.cpp line 373: > 371: return ciConstant(); > 372: } > 373: if (CURRENT_ENV->is_aot_compile()) { // Restrict only when we generate > AOT code This check can be moved to the top of the method to avoid executing other code when this condition is true. src/hotspot/share/ci/ciInstanceKlass.cpp line 65: > 63: if (!ik->is_loaded()) { > 64: ResourceMark rm; > 65: ik->print_on(tty); Is this required? Next assert statement already prints the name. src/hotspot/share/ci/ciMethod.cpp line 25: > 23: */ > 24: > 25: #include "cds/aotLinkedClassBulkLoader.hpp" This include is not needed. ------------- PR Review: https://git.openjdk.org/jdk/pull/30778#pullrequestreview-5259506525 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056135113 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056135334 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056135459 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056099057 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056098729 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056135887 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056136631 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056136777 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056138759 PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4056152707
