RFR: 8229517: Support for optional asynchronous/buffered logging

[Xin Liu]

On Thu, 25 Mar 2021 19:37:25 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> This patch provides a buffer to store asynchrounous messages and flush them to
>> underlying files periodically.
>
> src/hotspot/share/logging/logAsyncFlusher.hpp line 64:
> 
>> 62:     if (h != NULL) {
>> 63:       --_size;
>> 64:       log_drop(h->data());
> 
> I'm a little unhappy that some semantics of the Dequeues basic datatype become visible in the implementation of a basic Dequeue method. E.g. "*log dropping*" is not something common for a Dequeue but for the LogAsyncFlusher. I think it would be better to drop the call to `log_drop()` here and implement this functionality right in `LogAsyncFlusher::enqueue()`.

ACK. I would like to have a general purpose linked-listed deque here.  
I will move the log_drop() logic to LogAsyncFlusher.

> src/hotspot/share/logging/logAsyncFlusher.hpp line 95:
> 
>> 93:     : _output(output), _decorators(decorations.get_decorators()),
>> 94:     _level(decorations.get_level()), _tagset(decorations.get_logTagSet()) {
>> 95:       // allow to fail here, then _message is NULL
> 
> Why do you extract and store the `LogDecorators`, `_level` and `_tagset` set separately and re-create  the `LogDecorations` in `AsyncLogMessage::writeback()`? Is it to save memory (because  `LogDecorators` are much smaller than the `LogDecorations`) at the expense of time for recreating?

Saving memory is my intension. To keep thing simple, I used to copy objects directly. Then I found `LogDecorations` consists of a char array(256bytes) . This information can be compressed into an uint mask, which is `LogDecorators`.  

Because AsyncLogMessage is the payload, which hotspot stores thousands of them in the buffer, size matters.

> src/hotspot/share/logging/logFileOutput.cpp line 336:
> 
>> 334:   }
>> 335: 
>> 336:   assert(!_async_mode, "AsyncLogging is not supported yet");
> 
> Can you please explain in which circumstances `LogFileOutput::write(LogMessageBuffer::Iterator msg_iterator)` will be called and why it is not necessary to support `_async_mode` here?
> 
> Looks like this method is used e.g. when doing a class loading log (`-Xlog:class+load`) and will now result in a hard crash in debug builds which is certainly not appropriate:
> $ ./images/jdk/bin/java -Xlog:class+load:file=/tmp/class.log::async=true -version
> # To suppress the following error report, specify this argument
> # after -XX: or in .hotspotrc:  SuppressErrorAt=/logFileOutput.cpp:336
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error (/priv/simonisv/OpenJDK/Git/jdk/src/hotspot/share/logging/logFileOutput.cpp:336), pid=20491, tid=20492
> #  assert(!_async_mode) failed: AsyncLogging is not supported yet
> #
> # JRE version:  (17.0) (slowdebug build )
> # Java VM: OpenJDK 64-Bit Server VM (slowdebug 17-internal+0-adhoc.simonisv.jdk, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
> # Problematic frame:
> # V  [libjvm.so+0xde5ae4]  LogFileOutput::write(LogMessageBuffer::Iterator)+0x3c
> #
> # No core dump will be written. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
> #
> # An error report file with more information is saved as:
> # /priv/simonisv/output/jdk-dbg/hs_err_pid20491.log
> #
> #
> Aborted

I overlook it.  Sorry. I didn't notice the logging construct `LogMessage`. 
I've  fixed this issue.  I will update it soon.

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

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