On Mon, 21 Sep 2026 01:40:48 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:
> 
>   Address Ashutosh's comments

I took a look at the compiler, C1 and C1 bits. I have a few minor comments and 
suggestions. My main comment is concerning the initialization of compiler 
threads with the knowledge whether they are AOT compilation threads so we do 
not have to pass a bool to many functions.

src/hotspot/share/code/aotCodeCache.cpp line 715:

> 713:   return state;
> 714: }
> 715: #endif

Suggestion:

#ifdef COMPILER2
static uint get_c2_ea_state() {
  uint state = 0;
  bool eliminate_alloc = (DoEscapeAnalysis && EliminateAllocations)
                          || EliminateAutoBox
                          || EnableVectorAggressiveReboxing;
  bool eliminate_lock  = (DoEscapeAnalysis || EliminateNestedLocks)
                          && EliminateLocks;
  state += DoEscapeAnalysis ? 1 : 0;
  state += eliminate_alloc  ? 2 : 0;
  state += eliminate_lock   ? 4 : 0;
  return state;
}
#endif // COMPILER2

src/hotspot/share/code/aotCodeCache.cpp line 741:

> 739:   _reduceInitialCardMarks = ReduceInitialCardMarks;
> 740:   _c2_ea_state = get_c2_ea_state();
> 741: #endif

Suggestion:

#ifdef COMPILER2
  _reduceInitialCardMarks = ReduceInitialCardMarks;
  _c2_ea_state = get_c2_ea_state();
#endif // COMPILER2

src/hotspot/share/code/aotCodeCache.cpp line 1261:

> 1259: 
> 1260: static void copy_bytes(const char* from, address to, uint size) {
> 1261:   precond((int)size > 0);

A `uint` is always non-negative, but casting it to `int`you are effectively 
checking `size > 0 && size <= max_jint`. Is that intended?

src/hotspot/share/code/aotCodeCache.cpp line 1636:

> 1634:   found = (i < count);
> 1635:   assert(found, "entry should exist");
> 1636: #endif

Suggestion:

#endif // ASSERT

src/hotspot/share/compiler/compileBroker.cpp line 860:

> 858:     // Count AOT compiler thread
> 859:     bool is_aot_thread = (type == compiler_t) && (queue != nullptr) &&
> 860:                          (queue == _ac1_compile_queue || queue == 
> _ac2_compile_queue);

This should be stored into a field in the `CompilerThread`. This would save us 
from passing `is_aot_thread` to methods down the line that have access to the 
compiler thread anyway. Further, this would simplify adding more methods that 
need this information in the future.

src/hotspot/share/compiler/compileBroker.cpp line 1481:

> 1479:     bool is_blocking = ReplayCompiles                                   
>           ||
> 1480:                        !directive->BackgroundCompilationOption          
>           ||
> 1481:                        (AOTPreloadBlocking && (compile_reason == 
> CompileTask::Reason_AOTPreload));

This should probably go into a small helper to keep it consistent with lines 
1342-1345.

src/hotspot/share/compiler/compiler_globals.hpp line 396:

> 394:   product(uint, DisableAOTCode, 0, DIAGNOSTIC,                           
>    \
> 395:           "Disable AOT code on some compilation levels "                 
>    \
> 396:           "(T1=1; T2=10; T4=1000; T5/preload=10000)")                    
>    \

Is tier 3 deliberately left out here?

src/hotspot/share/compiler/compiler_globals.hpp line 437:

> 435:                                                                          
>    \
> 436:   product(bool, UseAOTCodeCounters, true, DIAGNOSTIC,                    
>    \
> 437:           "Use AOT code counter to trigger JIT compilation")             
>    \

Why are these not moved to `c2_globals.hpp` then? Do we expect to implement 
this functionality in C1?

src/hotspot/share/opto/doCall.cpp line 621:

> 619:     clinit_barrier(holder_klass, method());
> 620:     if (stopped()) {
> 621:       return; // MUST uncommon-trap?

Could you explain this comment?

-------------

Changes requested by mhaessig (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/30778#pullrequestreview-5263936605
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4060416526
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4060414321
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4060505911
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4060671636
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4061712229
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4061791053
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4060165099
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4060195639
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4062031873

Reply via email to