On Tue, 22 Apr 2025 23:05:13 GMT, Vladimir Kozlov <k...@openjdk.org> wrote:
>> [JEP 483](https://bugs.openjdk.org/browse/JDK-8315737) preserves class >> information in AOT cache which helps Java startup performance. >> >> We should also preserve adapters (i2c, c2i) to further improve performance >> of class linking where adapters are generated. >> >> Short running Java application can see several percents improvement. I got >> 6% improvement when ran `HelloWorld.java` on Linux-x64 Ice Lake CPU (2.5Ghz): >> >> >> (perf stat -r 100 java -XX:AOTCache=app.aotcache -cp hello.jar HelloWorld > >> /dev/null) 2>&1 | grep elapsed >> 0.0299401 +- 0.0000504 seconds time elapsed ( +- 0.17% ) >> >> (perf stat -r 100 java -XX:AOTCache=app.aotcache >> -XX:+UnlockDiagnosticVMOptions -XX:-AOTAdapterCaching -cp hello.jar >> HelloWorld > /dev/null) 2>&1 | grep elapsed >> 0.0318654 +- 0.0000535 seconds time elapsed ( +- 0.17% ) >> >> >> New diagnostic flags are introduced (use `-XX:+UnlockDiagnosticVMOptions` to >> unlock them): >> >> >> -XX:+AOTAdapterCaching - Enable or disable saving and restoring i2c2i >> adapters >> -XX:AOTCodeMaxSize=10*M - buffer size in bytes for AOT code caching >> -XX:+AbortVMOnAOTCodeFailure - Abort VM on the first occurrence of AOT code >> caching failure >> >> By default `AOTAdapterCaching` is `false` and enabled ergonomically when >> `-XX:AOTCache` is specified. >> This flag is ignored when `AOTCache` is not specified. >> >> To use AOT adapters follow process described in JEP 483: >> >> >> java -XX:AOTMode=record -XX:AOTConfiguration=app.aotconf -cp app.jar App >> java -XX:AOTMode=create -XX:AOTConfiguration=app.aotconf >> -XX:AOTCache=app.aot -cp app.jar >> java -XX:AOTCache=app.aot -cp app.jar App >> >> >> There are several new UL flag combinations to trace the AOT code caching >> process: >> >> >> -Xlog:aot+codecache+init -Xlog:aot+codecache+exit -Xlog:aot+codecache+stubs >> >> >> @ashu-mehra is main author of changes. He implemented adapters caching. >> I did main framework (`AOTCodeCache` class) for saving and loading AOT code. >> >> Tested tier1-6,10, which includes tests with `AOTClassLinking` enabled. Also >> Xcomp,stress and JCK. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Fix message Finished the first pass over the code. Overall, looks good. Some feedback follows. src/hotspot/share/cds/aotCacheAccess.hpp line 38: > 36: // AOT Cache API for AOT compiler > 37: > 38: class AOTCacheAccess : AllStatic { It looks related to `AOTCodeCache`? Maybe `AOTCodeCacheAccess` then? src/hotspot/share/cds/aotCacheAccess.hpp line 40: > 38: class AOTCacheAccess : AllStatic { > 39: public: > 40: static void* allocate_aot_code(size_t size) NOT_CDS_RETURN_(nullptr); "allocate_aot_code_region", "get_aot_code_region_size", and "map_aot_code_region" would be clearer. src/hotspot/share/code/aotCodeCache.cpp line 62: > 60: } > 61: > 62: static void exit_vm_on_store_failure() { It's a bit confusing to see `exit_vm_on_load_failure()` and `exit_vm_on_store_failure()` to silently proceed unless a flag is explicitly specified. Moreover, how reliable `AOTAdapterCaching = false` to fail-fast and avoid repreated load/store attempts? At least, I see that `AOTCodeCache` ctor cache `AOTAdapterCaching`, so it won't see the update. How does it affect adapter code generation during assembly phase? src/hotspot/share/code/aotCodeCache.cpp line 645: > 643: return false; > 644: } > 645: log_info(aot, codecache, stubs)("Writing blob '%s' to AOT Code Cache", > name); I'd revisit logging code in AOTCodeCache and downgrade info->debug and debug->trace where appropriate. It feels too low-level most of the time. src/hotspot/share/runtime/sharedRuntime.cpp line 2966: > 2964: adapter_blob = > AdapterHandlerLibrary::link_aot_adapter_handler(this); > 2965: if (adapter_blob == nullptr) { > 2966: log_warning(cds)("Failed to link AdapterHandlerEntry (fp=%s) to > its code in the AOT code cache", _fingerprint->as_basic_args_string()); Doesn't it add noise in the output for not yet seen adapter shapes? It's a warning. ------------- PR Review: https://git.openjdk.org/jdk/pull/24740#pullrequestreview-2789025188 PR Review Comment: https://git.openjdk.org/jdk/pull/24740#discussion_r2057119115 PR Review Comment: https://git.openjdk.org/jdk/pull/24740#discussion_r2057115219 PR Review Comment: https://git.openjdk.org/jdk/pull/24740#discussion_r2057207417 PR Review Comment: https://git.openjdk.org/jdk/pull/24740#discussion_r2057187511 PR Review Comment: https://git.openjdk.org/jdk/pull/24740#discussion_r2057189917