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

[Xin Liu]

On Thu, 25 Mar 2021 20:55:41 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - 8229517: Support for optional asynchronous/buffered logging
>>    
>>    add a constraint for the option LogAsyncInterval.
>>  - 8229517: Support for optional asynchronous/buffered logging
>>    
>>    LogMessage supports async_mode.
>>    remove the option AsyncLogging
>>    renanme  the option GCLogBufferSize to AsyncLogBufferSize
>>    move drop_log() to LogAsyncFlusher.
>
> src/hotspot/share/logging/logAsyncFlusher.hpp line 116:
> 
>> 114:   bool equals(const AsyncLogMessage& o) const {
>> 115:     return (&_output == &o._output) && (_message == o._message || !strcmp(_message, o._message));
>> 116:   }
> 
> [`strcmp()` is not defined for `NULL`](https://en.cppreference.com/w/cpp/string/byte/strcmp) but you can have `_message == NULL` if you've transferred ownership in the copy constructor.

yes, this is subtle bug! thanks!
I thought that if _message is NULL, then o._message  must be NULL, then it will be true for _message == o._message.  I was wrong.

> src/hotspot/share/logging/logAsyncFlusher.hpp line 124:
> 
>> 122: 
>> 123: class LogAsyncFlusher : public PeriodicTask {
>> 124:  private:
> 
> As far as I know, `PeriodicTask` is designed for short running task. But `LogAsyncFlusher::task()` will now call `AsyncLogMessage::writeback()` which does blocking I/O and can block for quite some time (that's why we have this change in the first place :). How does this affect the other periodic tasks and the `WatcherThread`. What's the worst case scenario if the `WatcherThread` is blocked? Is this any better than before?

ack. other reviewers also raise this question. 
I propose a dedicated nonjavathread to flush log.

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

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