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

[Xin Liu]

On Fri, 7 May 2021 06:56:23 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> 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).

Hi, @tstuefe , 
I agree with you that uintx is not a good type. uint32_t is big enough. 

I found it's  very tricky to move those counters to LogFileOutput objects. after than, I still need HashTable as a set to traverse all LogFileObject objects. 

Alternatively, there's a member variable `_outputs` in LogConfiguration, which refers to all LogOutput objects. I have to obtain  `ConfigurationLock` first before reading it because jcmd could modify outputs at any moment.  Grabbing `ConfigurationLock`  in `LogAsyncFlusher::flush` may lead to a dead lock with `_lock`.  This part of code is really subtle. I tried different approaches. no one is  more straight-forward than a hashmap.

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

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