RFR: 8358343: [leyden] Drop notify_all in CompilationPolicyUtils::Queue::pop [v2]
Aleksey Shipilev
shade at openjdk.org
Wed Sep 3 09:40:02 UTC 2025
On Fri, 6 Jun 2025 07:34:46 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> Found this when reading premain-vs-mainline webrev. Mainline does not have `notify_all` in this method:
>> https://github.com/openjdk/jdk/blob/c382da579884c28f2765b2c6ba68c0ad4fdcb2ce/src/hotspot/share/compiler/compilationPolicy.hpp#L85-L92
>>
>> But if you remove `notify_all()` in `premain`, then tests start to deadlock, see bug for a sample. The culprit is `CompilationPolicy::flush_replay_training_at_init`, which is only present in premain. I fixed it by using timed waits, which obviates the need for extra notifications. We only enter this method with `-XX:+AOTVerifyTrainingData`, so we don't care much about its performance. This is IMO better than doing a questionable `notify_all` followed by `wait` in load-bearing code.
>>
>> Additional testing:
>> - [x] Linux x86_64 server fastdebug, `runtime/cds` (5x, no timeouts yet; still running more iterations)
>
> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>
> - Merge branch 'premain' into JDK-8358343-leyden-training-notify-all
> - Fix
Dang, too late. I think this was fixed in `premain` with: https://github.com/openjdk/leyden/commit/ca21af49a1de66b623d5a7481877766e91543c7f
-------------
PR Comment: https://git.openjdk.org/leyden/pull/74#issuecomment-3248479147
More information about the leyden-dev
mailing list