RFR: 8323807: Async UL: Add a stalling mode to async UL [v2]

David Holmes dholmes at openjdk.org
Wed Apr 3 01:42:14 UTC 2024


On Tue, 2 Apr 2024 14:54:30 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> From reading TBIVM, I think I'm using it incorrectly. It's currently this:
>> 
>> ```c++
>>       if (thread != nullptr && thread->is_Java_thread()) {
>>         ThreadBlockInVM tbivm(JavaThread::cast(thread));
>>         while (not_enough_memory()) {
>>           _producer_lock.wait(0);
>>         }
>>       }
>> 
>> 
>> But maybe it should be this:
>> 
>> ```c++
>>       if (thread != nullptr && thread->is_Java_thread()) {
>>         while (not_enough_memory()) {
>>         ThreadBlockInVM tbivm(JavaThread::cast(thread));
>>           _producer_lock.wait(0);
>>         }
>>       }
>> 
>> 
>> `Monitor` is also a bit confusing to me, it seems that only `JavaThread`s can use `Monitor` (see `Monitor::wait` for example), but we have no guarantee that UL is used from a `JavaThread`.
>> 
>> At the very least, this is revealing that I'm not that good at the JVM's thread and locking system :-).
>
> Right, I talked to ErikÖ, and the first version (the current one in the source tree) is correct. The second one isn't wrong either, but it's unnecessary.
> 
> I'm not creative enough to see any other safepoint issues, however.

Yeah the in-or-out-of the loop placement of TBIVM only impacts performance/overhead.

`Monitor::wait` includes safepoint checks and so is only for JavaThreads. For NonJavaTHreads and JavaThread's that don't want to perform safepoint checks you use `Monitor::wait_without_safepoint_check`. This used to be handled by an argument passed to `wait` but the API was reworked a few releases back. The caller has to check what kind of thread they have. So you will need to restructure the wait code you have.

Other safepoint issues would relate to potential deadlocks if the UL code is called whilst other locks are held and a safepoint is active. For non-stalling locking the short critical sections make that a non-issue, but with stalling it could well be an issue.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17757#discussion_r1548810050


More information about the hotspot-runtime-dev mailing list