RFR: 8315149: Add hsperf counters for CPU time of internal GC threads [v44]

[Man Cao]

On Wed, 15 Nov 2023 23:50:02 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:
> 
>   Fix whitespace

Overall looks good, a few details could be improved.

src/hotspot/share/gc/g1/g1CollectedHeap.hpp line 59:

> 57: #include "memory/iterator.hpp"
> 58: #include "memory/memRegion.hpp"
> 59: #include "runtime/cpuTimeCounters.hpp"

Probably move this include to the .cpp file?

src/hotspot/share/gc/g1/g1ServiceThread.cpp line 161:

> 159:   if (UsePerfData && os::is_thread_cpu_time_supported()) {
> 160:     ThreadTotalCPUTimeClosure tttc(CPUTimeCounters::get_instance(), CPUTimeGroups::gc_service);
> 161:     tttc.do_thread(task->_service_thread);

It could just use `do_thread(this)`, then it can remove the `task` parameter.

src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.cpp line 72:

> 70:   _processor = new Processor();
> 71:   if (UsePerfData && os::is_thread_cpu_time_supported()) {
> 72:     EXCEPTION_MARK;

This whole `if` block could be updated to `CPUTimeCounters::get_instance()->create_counter(CPUTimeGroups::conc_dedup)`. We can also remove the `_concurrent_dedup_thread_cpu_time` field and the `ThreadTotalCPUTimeClosure(PerfCounter* counter)` constructor.

In `StringDedup::Processor::run()`, it can call

if (UsePerfData && os::is_thread_cpu_time_supported()) {
  ThreadTotalCPUTimeClosure tttc(CPUTimeCounters::get_instance(), CPUTimeGroups::conc_dedup);
  tttc.do_thread(thread);
}


Similar, this can be applied to vmThread.

src/hotspot/share/runtime/init.cpp line 124:

> 122:   codeCache_init();
> 123:   VM_Version_init();              // depends on codeCache_init for emitting code
> 124:   // Initialize CPUTimeCounters object, which must be done before creation of the heap.

Would it be possible to move this inside `universe_init()` in universe.cpp, somewhere above `Universe::initialize_heap()`? There's a similar `MetaspaceCounters::initialize_performance_counters()` in `universe_init()`.

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

Changes requested by manc (Committer).

PR Review: https://git.openjdk.org/jdk/pull/15082#pullrequestreview-1731128372
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1395001799
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1395007511
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1395009733
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1395000735