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

[Man Cao]

On Tue, 21 Nov 2023 21:42:39 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 two additional commits since the last revision:
> 
>  - Update memory tracking type for CPUTimeCounters
>  - Fix assertion logic

Looks pretty clean. Only a few minor cleanups remain.

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 71:

> 69: #include "oops/oop.inline.hpp"
> 70: #include "runtime/atomic.hpp"
> 71: #include "runtime/cpuTimeCounters.hpp"

This include could be removed.

src/hotspot/share/gc/g1/g1ConcurrentRefine.cpp line 38:

> 36: #include "memory/allocation.inline.hpp"
> 37: #include "memory/iterator.hpp"
> 38: #include "runtime/cpuTimeCounters.hpp"

This could be removed too.

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

> 24: 
> 25: #include "precompiled.hpp"
> 26: #include "gc/g1/g1CollectedHeap.hpp"

Could this include be removed?

src/hotspot/share/gc/g1/g1ServiceThread.hpp line 30:

> 28: #include "gc/shared/concurrentGCThread.hpp"
> 29: #include "runtime/mutex.hpp"
> 30: #include "runtime/perfData.hpp"

This could be removed.

src/hotspot/share/gc/shared/collectedHeap.cpp line 52:

> 50: #include "oops/instanceMirrorKlass.hpp"
> 51: #include "oops/oop.inline.hpp"
> 52: #include "runtime/atomic.hpp"

This could be removed.

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

> 26: #include "classfile/javaClasses.inline.hpp"
> 27: #include "classfile/stringTable.hpp"
> 28: #include "gc/shared/collectedHeap.hpp"

This include as well as the include for perfData.hpp (line 45) could be removed.

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

> 68: void StringDedup::Processor::initialize() {
> 69:   _processor = new Processor();
> 70:   if (UsePerfData && os::is_thread_cpu_time_supported()) {

The if and EXCEPTION_MARK could be removed, because `create_counter()` does that internally.

src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.hpp line 30:

> 28: #include "gc/shared/stringdedup/stringDedup.hpp"
> 29: #include "memory/allocation.hpp"
> 30: #include "runtime/perfData.hpp"

This could be removed.

src/hotspot/share/runtime/cpuTimeCounters.hpp line 97:

> 95: class ThreadTotalCPUTimeClosure: public ThreadClosure {
> 96:  private:
> 97:   jlong _gc_total;

_total is a more appropriate name for this field.

src/hotspot/share/runtime/vmThread.cpp line 140:

> 138:                  PerfDataManager::create_counter(SUN_THREADS, "vmOperationTime",
> 139:                                                  PerfData::U_Ticks, CHECK);
> 140:     if (os::is_thread_cpu_time_supported()) {

The if could be remove, as `create_counter()` checks it internally.

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

Marked as reviewed by manc (Committer).

PR Review: https://git.openjdk.org/jdk/pull/15082#pullrequestreview-1743425918
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401413936
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401415033
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401416191
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401416622
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401416969
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401417321
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401418586
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401420582
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401410366
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1401419999