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

[Thomas Stuefe]

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

>> src/hotspot/share/logging/logAsyncFlusher.hpp line 123:
>> 
>>> 121: 
>>> 122: typedef LinkedListDeque<AsyncLogMessage, mtLogging> AsyncLogBuffer;
>>> 123: typedef KVHashtable<LogFileOutput*, uintx, mtLogging> AsyncLogMap;
>> 
>> This is just to keep statistics of dropped messages per output, right? 
>> 
>> I think it would be a lot simpler just to add a reset-able counter to LogFileOutput directly. I don't even think it has to be uintx. 32bit should be sufficient (also, uintx makes no sense - do we somehow expect less message drops on 32bit platforms?)
>
> Yes, it is for the statistics of dropped messages.
> 
> AsyncLogMap looks a little cumbersome indeed, but the good side is that I isolate async logging things in logAsyncFlusher.hpp/cpp.  I manage not to pollute shared unified logging code. 
> 
> Another reason is that I can deliver the zero-cost promise when async logging is off.  If I move the counter to LogFileOutput, it will increase those object size a little bit.  In current implementation,  the singleton object LogAsyncFlusher is not created if -Xlog:async is absent.
> 
> Size wise, I think it's okay to to have 8 byte.  if a network-based device disappeared, who know how long asynclog thread  would block.  bigger counter is better.  Cost is almost same because the number of logFileOutput objects is usually less than 4.

I respect your "no costs promise" concern, but really this is no problem. We create exactly one LogFileOutput object per file sink; so usually we have one, maybe two of those if async=true. I think the costs of those counters would be bearable :)

I understand the concern of not polluting the UL code base. But here I think it does not apply, since you bring your patch upstream anyway, so async logging is now part of the UL code base.

wrt to counter size, okay lets use 64bit. But lets really use 64bit, also in 32bit VMs (so, use uint64_t instead of uintx).

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

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