RFR: 8229517: Support for optional asynchronous/buffered logging [v16]

[Xin Liu]

On Wed, 19 May 2021 16:54:48 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Implement the new discard policy: drop the incoming message.
>>   
>>   This patch also fix a bug. meta messages append to the temp logs instead
>>   of directly calling output->write_blocking(). This guranatees logsites are non-blocking.
>
> src/hotspot/share/logging/logAsyncWriter.cpp line 180:
> 
>> 178:       // over all logging I/O.
>> 179:       for (LogTagSet* ts = LogTagSet::first(); ts != NULL; ts = ts->next()) {
>> 180:         ts->wait_until_no_readers();
> 
> This loop is the only thing I'm not understanding in this whole PR but that doesn't mean it is wrong (it's probably just caused by my limited understanding of UL).
> So why do we have to wait here until "there are no more readers" and what does that actually means? Is this necessary to prevent mixing log messages which have been printed before async logging was started?

I would like to consolidate "exclusively". Once async logging takes over, we guarantee that no synchronous logsite is writing files in concurrency.  If we guarantee that, we can avoid FileLocker() because only AsyncLog Thread writes files. 

Fence isn't enough here. fence can only guarantee that logsites 'after' the fence use async logging.  It's possible that there are on-going synchronous logging. 

This is a RCU trick of Unified logging.  RCU is kind of synchronization biased to multiple readers. `LogOutputList::Iterator` actually embeds an atomic counter.
ts->wait_until_no_readers() waits until all readers finish. It means that all synchronous write have done. 

here is how synchronous log writes. Iterator increases to zero when the loop is done. 
 

void LogTagSet::log(LogLevelType level, const char* msg) {
  LogDecorations decorations(level, *this, _decorators);
  for (LogOutputList::Iterator it = _output_list.iterator(level); it != _output_list.end(); it++) {
    (*it)->write(decorations, msg);
  }
}```

> src/hotspot/share/logging/logAsyncWriter.hpp line 132:
> 
>> 130:   // and a variable-length c-string message.
>> 131:   // A normal logging  message is smaller than vwrite_buffer_size, which is defined in logtagset.cpp
>> 132:   const size_t _buffer_max_size = {AsyncLogBufferSize / (sizeof(AsyncLogMessage) + sizeof(LogDecorations) + vwrite_buffer_size)};
> 
> Isn't the `LogDecorations` object part of the `AsyncLogMessage` object? It is stored there as a "value object" not as a reference so its size should already be included in `sizeof(AsyncLogMessage)`.

yes, I changed logDecoration from pointer to value. I will update the comment.

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

PR: https://git.openjdk.java.net/jdk/pull/3135