RFR: 8315149: Add hsperf counters for CPU time of internal GC threads [v53]
Stefan Johansson
sjohanss at openjdk.org
Fri Dec 1 10:01:27 UTC 2023
On Thu, 30 Nov 2023 21:59:41 GMT, Man Cao <manc at openjdk.org> wrote:
>> 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.
I would say it depends on the use-case and here when switching to use static functions to use the instance it felt more like an all-static class. I agree that it would be nice to avoid the additional memory usage if `UsePerfData` is `false` so I'm ok with keeping the instance if we add that.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1411872139
More information about the hotspot-gc-dev
mailing list