RFR: 8315149: Add hsperf counters for CPU time of internal GC threads [v53]
Stefan Johansson
sjohanss at openjdk.org
Thu Nov 30 09:58:29 UTC 2023
On Thu, 30 Nov 2023 02:45:45 GMT, Jonathan Joo <jjoo at openjdk.org> wrote:
>> 8315149: Add hsperf counters for CPU time of internal GC threads
>
> Jonathan Joo has updated the pull request incrementally with one additional commit since the last revision:
>
> Add missing include
Few more comments after the latest changes.
src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 905:
> 903: gc_threads_do(&tttc);
> 904:
> 905: CPUTimeCounters::publish_gc_total_cpu_time();
As I suggested in the other comment, maybe we should not keep the total counter, but if we do we need to make sure the destructor of the closure is run before the call to `publish_gc_total_cpu_time()`. Otherwise we will publish a not yet updated value.
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?
-------------
Changes requested by sjohanss (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15082#pullrequestreview-1757031262
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1410415366
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1410410265
More information about the hotspot-gc-dev
mailing list