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