RFR: 8365407: Race condition in MethodTrainingData::verify() [v2]

[Igor Veresov]

On Thu, 21 Aug 2025 02:02:56 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Igor Veresov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix minimal build
>
> src/hotspot/share/compiler/compilationPolicy.cpp line 141:
> 
>> 139: }
>> 140: 
>> 141:  void CompilationPolicy::flush_replay_training_at_init(TRAPS) {
> 
> This method seems to be waiting for something to finish, not "flushing" anything itself.

It has a semantic effect of flushing the queue... What would you like it to be renamed to?

> src/hotspot/share/oops/trainingData.hpp line 678:
> 
>> 676:   void dec_init_deps_left(KlassTrainingData* ktd);
>> 677:   int init_deps_left() const {
>> 678:     return Atomic::load_acquire(&_init_deps_left);
> 
> Where is the `release_store` (or other ordered atomic op) that this pairs with?
> 
> Also there is a convention to make acquire/release semantics clear in the API method names i.e. in this case `init_deps_left_acquire()`.

There is an `Atomic::sub()` in `dec_init_deps_left()`.

> src/hotspot/share/runtime/java.cpp line 522:
> 
>> 520:   if (AOTVerifyTrainingData) {
>> 521:     EXCEPTION_MARK;
>> 522:     CompilationPolicy::flush_replay_training_at_init(THREAD);
> 
> Looks odd to have an `at_init` method executing during VM shutdown.

`init` here means `class initialization`. We can rename it if you want, but that's the current convention everybody expects in the leyden project.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26866#discussion_r2289712736
PR Review Comment: https://git.openjdk.org/jdk/pull/26866#discussion_r2289708246
PR Review Comment: https://git.openjdk.org/jdk/pull/26866#discussion_r2289710061