RFR: 8315149: Add hsperf counters for CPU time of internal GC threads [v53]
Man Cao
manc at openjdk.org
Thu Nov 30 22:03:28 UTC 2023
On Thu, 30 Nov 2023 09:41:55 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:
>> Jonathan Joo has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add missing include
>
> src/hotspot/share/runtime/cpuTimeCounters.hpp line 59:
>
>> 57: NONCOPYABLE(CPUTimeCounters);
>> 58:
>> 59: static CPUTimeCounters* _instance;
>
> I would prefer if we made the whole class static and got rid of the instance and just made the `_cpu_time_counters` array static. The only drawback I/we (discussed this with Albert as well) can see is that the memory for the array would not be accounted in NMT, but this array will always be very small so should not be a big problem.
>
> Do you see any other concerns?
I thought it is typically preferred to initialize a singleton object on the heap, rather than using several static variables. It is easier to control the initialization order and timing of an on-heap singleton object than statics.
Moreover, for this class, `initialize()` could also check `if (UsePerfData)`, and only create the singleton object under `UsePerfData`. This could save some memory when `UsePerfData` is false.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1411317828
More information about the hotspot-gc-dev
mailing list