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

Axel Boldt-Christmas aboldtch at openjdk.org
Wed Jan 22 14:57:57 UTC 2025


On Wed, 22 Jan 2025 14:05:02 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> src/hotspot/share/logging/logAsyncWriter.cpp line 197:
>> 
>>> 195:       ConsumerLocker clocker;
>>> 196: 
>>> 197:       while (!_data_available && _stalled_message == nullptr) {
>> 
>> Is the only reason we get here with `_data_availiable == false && _stalled_message != nullptr` if we try to log larger than `AsyncLogBufferSize / 2` (the size of the buffer). AFAICT the only path where this could happen, is when we try to log something that does not fit in an empty buffer. 
>> 
>> Can `_data_availiable` be a signal for both the `_buffer` and the `_stalled_message`?
>
>>Can _data_availiable be a signal for both the _buffer and the _stalled_message?
> 
> We can have that in this check, but the `_stalled_message` is needed to communicate that a `_stalled_message`exists. It seems easier to reason about two signals, as we can take care of each one after another.

Alright. I think to me the two signals is more confusing. Mainly because we have mutual exclusion on producing data (this includes taking the consumer lock for producing), and if it stalls the producer lock is held until the consumer consumes. So if we ignore the case where the log message is larger than the buffer, if we stall we also know that the consumer must have been notified and `_data_availiable` been set to true. 

> Axel correctly identified that a notify call is missing. This turned out not to matter, perhaps because of spurious wake ups? Regardless, I fixed it.

The reason it used to work is that we probably never logged 50k+ bytes. If there is a single byte in the buffer when a message stalls we must have had another producer that have a notification in flight for the consumer.

We can then have, pseudo-C++.
Producer side:
```C++
  ProducerLocker pl();
  ConsumerLocker cl();
  produce_data(); // May produce a stall
  if (!_data_availiable) {
    _data_availiable = true;
    cl.notify();
  }
  while (_stalled_msg) {
    cl.wait();
  }
  // Free the stalled message here after the lock or on the consumer side


Consumer side:
```C++
  {
    ConsumerLocker cl();
    while (!_data_availiable) {
      cl.wait();
    }
    claim_data();
    _data_availiable = false;
  }
  process_data();
  if (_stalled_msg) {
    ConsumerLocker cl();
    _stalled_msg = nullptr;
    cl.notify();
  }


The nice thing with one signal, is that it can be kept one to one with the property "we have notified the consumer". 

Doing the current way may have some redundant notify calls, and have a bit more state to keep in your head while parsing the code. But I am fine with it as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22770#discussion_r1925458676


More information about the hotspot-runtime-dev mailing list