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

[Xin Liu]

On Wed, 19 May 2021 12:16:27 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 64:
> 
>> 62:     }
>> 63:     // drop the enqueueing message.
>> 64:     return;
> 
> I don't understand this? If we drop the incoming message `msg`, why do we increment the drop counter for the `LogFileOutput` of the first (i.e. `front()`) enqueued message?
> Shouldn't this simply be:
> 
> if (_buffer.size() >= _buffer_max_size)  {
>   bool p_created;
>   uint32_t* counter = _stats.add_if_absent(msg->output(), 0, &p_created);
>   *counter = *counter + 1;
>   return;

you are right. I will change this. 
in previous implementation, I pop_front() and enqueue msg. 
I didn't clean up everything.

> src/hotspot/share/logging/logAsyncWriter.cpp line 125:
> 
>> 123: 
>> 124: void AsyncLogWriter::perform_IO() {
>> 125:   // use kind of copy-and-swap idiom here.
> 
> Can you please start new sentences with an uppercase letter.

Okay.

> src/hotspot/share/logging/logConfiguration.cpp line 606:
> 
>> 604:   out->print_cr(" -Xlog:async -Xlog:gc=debug:file=gc.log -Xlog:safepoint=trace");
>> 605:   out->print_cr("\t Write logs asynchronously. Enable messages tagged with 'safepoint' up to 'trace' level to stdout ");
>> 606:   out->print_cr("\t and messages tagged with 'gc' up to 'trace' level to file 'gc.log'.");
> 
> This should read "..up to 'debug' level.."

yes.

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

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