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

Xin Liu xliu at openjdk.java.net
Fri May 21 22:13:09 UTC 2021


On Fri, 21 May 2021 05:45:12 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update according to reviewer's feedback.
>
> src/hotspot/share/logging/logAsyncWriter.hpp line 113:
> 
>> 111: typedef KVHashtable<LogFileOutput*, uint32_t, mtLogging> AsyncLogMap;
>> 112: 
>> 113: class AsyncLogWriter: public NonJavaThread {
> 
> Any reason this is not a NamedThread? How will this appear in thread-dumps and crash reports?
> 
> Also I would have expected two classes to simplify the APIs: AsyncLogWriter and AsyncLogThread

NamedThread() is not just NonJavaThread with a name.  It has some gc-specific fields. it seams NamedThread is for GC threads. Further, I don't need to support dynamic names. 

I just figure out `Threads::print_on()` needs  special care for dumpthread.  Nice catch!
With my patch, it finally shows up. 

"AsyncLog Thread" os_prio=0 cpu=239.26ms elapsed=52.13s tid=0x00007fb814c76ec0 nid=0x9023 runnable


Now there are only 4 APIs for clients. Splitting up 2 classes seems not to simplify code a lot.

> src/hotspot/share/logging/logAsyncWriter.hpp line 147:
> 
>> 145: 
>> 146:   // None of following functions are thread-safe.
>> 147:   static AsyncLogWriter* instance();
> 
> instance() isn't thread-safe ??

yes, the comment is outdated.  This is MT-safe. 
I will update it.

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

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


More information about the hotspot-dev mailing list