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

[Thomas Stuefe]

On Fri, 7 May 2021 06:13:19 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> This patch provides a buffer to store asynchrounous messages and flush them to
>> underlying files periodically.
>
> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Change option AsyncLogBufferEntries to AsyncLogBufferSize.
>   
>   AsyncLogBufferSize is the memory budget in bytes for Asynchronous Logging.
>   User can specify it in shorthand form. eg. -XX:AsyncLogBufferSize=10M.

src/hotspot/share/logging/logAsyncFlusher.cpp line 189:

> 187:     LogAsyncFlusher* self = _instance;
> 188: 
> 189:     Atomic::release_store<LogAsyncFlusher*, LogAsyncFlusher*>(&_instance, nullptr);

IIUC the mechanism you signal "async"-ness to the logging callsites with is via this pointer being NULL or not, right?

I am not sure this is safe wrt to lost log output on termination. This is not a big deal, but it would be nice if we lost no output from concurrent logging when terminating and switching over to non-async mode.

Proposal: 

in `LogFileOutput::write`, instead of querying this pointer for NULL, just call `LogAsyncFlusher::enqueue()` unconditionally and let enqueue() return a state whether write succeeded or not. In that function, return false if flusher is terminated or not active at all - in which case you do the normal synchronized writing.

Thinking this through, one could even scratch the LogAsyncBuffer dynamic allocation and make it either a static global object or even an AllStatic, with all static methods. As a bonus, you can replace the instance variable, as well as the NULL tests and possibly the Thread::current(), with access to either static LogAsyncBuffer functions or to a global static LogAsyncBuffer singleton.

src/hotspot/share/logging/logAsyncFlusher.cpp line 206:

> 204: 
> 205: LogAsyncFlusher* LogAsyncFlusher::instance() {
> 206:   if (Thread::current_or_null() != nullptr) {

I don't understand why this depends on the existence of a current thread? Ideally we should be able to log just fine in corner cases, e.g. when thread is detached or if we have an error (during signal handling e..g.)

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

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