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

[David Holmes]

On Tue, 2 Apr 2024 10:57:13 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> src/hotspot/share/logging/circularStringBuffer.hpp line 119:
>> 
>>> 117:   PlatformMonitor& _stats_lock;
>>> 118: 
>>> 119:   // Can't use a Monitor here as we need a low-level API that can be used without Thread::current().
>> 
>> I had forgotten that the async logging thread already used a `PlatformMutex` and so had to go back to the original PR to understand why - see some initial discussion here:
>> 
>> https://github.com/openjdk/jdk/pull/3135#discussion_r629739367
>> 
>> The problem you now face is that by using monitors with explicit `wait()`'s for the stalling is that these will delay safepoints because the PlatformMonitor is not safepoint aware. I think you will need to introduce `ThreadBlockInVM` around potentially blocking enqueues - though it may not be that simple as you still need to address the logging that happens after the current thread has been deleted.
>
> Thank you for finding the original comment thread. Wouldn't it be sufficient to add a `TBIVM` if and only if `Thread::current_or_null() != nullptr`?

That will mean a little duplicated code due to the TBIVM being block-scoped (which we do elsewhere). The TBIVM is  necessary but I'm not certain it is sufficient to deal with all safepoint issues.

>> src/hotspot/share/logging/circularStringBuffer.hpp line 124:
>> 
>>> 122:   Semaphore _flush_sem;
>>> 123: 
>>> 124:   struct ReadLocker : public StackObj {
>> 
>> The ReadLocker/WriteLocker terminology initially made me think you were using ReadWriteLocks, but IIUC these are simply two distinct locks for different mutating operations. I prefer the producer/consumer naming from the description in JBS.
>> 
>> The overall locking protocol is unclear - the JBS description only uses mutexes not monitors.
>
>>The overall locking protocol is unclear - the JBS description only uses mutexes not monitors.
> 
> The wait/notify capability is used for announcing that more memory is available or that a new message is available. This isn't necessary for the locking protocol in itself, we could also repeatedly take the respective locks and look at the data in order to determine the state of the system. Wait/notify simply allows us to yield the thread in the cases of a full buffer or no messages being available for printing.

The wait/notify becomes a little clearer with producer/consumer terminlogy.

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

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