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

Man Cao manc at openjdk.org
Wed Oct 11 23:54:19 UTC 2023


On Thu, 5 Oct 2023 03:00:36 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 comment and change if defined to ifdef

Changes requested by manc (Committer).

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 900:

> 898:   // behavior, we should rethink if it is still safe.
> 899:   gc_threads_do(&tttc);
> 900: }

It should also call `CollectedHeap::publish_total_cpu_time()` here, right?

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

> 296:   NOT_PRODUCT(_promotion_failure_alot_gc_number = 0;)
> 297: 
> 298:   if (UsePerfData && os::is_thread_cpu_time_supported()) {

This condition should be a nested if inside `if (UsePerfData)`:


if (os::is_thread_cpu_time_supported()) {
  _total_cpu_time = ...;
  _perf_parallel_worker_threads_cpu_time = ...;
}

Otherwise `_perf_gc_cause` and `_perf_gc_lastcause` could be broken.

src/hotspot/share/gc/shared/collectedHeap.hpp line 211:

> 209:   };
> 210: 
> 211: 

Nit: unnecessary new line.

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

> 132:   _terminate_lock = new Monitor(Mutex::nosafepoint, "VMThreadTerminate_lock");
> 133: 
> 134:   if (UsePerfData && os::is_thread_cpu_time_supported()) {

Similarly, check for `os::is_thread_cpu_time_supported()` should be a inner nested if, to avoid breaking `_perf_accumulated_vm_operation_time`.

test/jdk/sun/tools/jcmd/TestGcCounters.java line 1:

> 1: import static jdk.test.lib.Asserts.*;

New file should have Copyright header comment.
You can copy the header from test/jdk/javax/imageio/plugins/jpeg/LargeAdobeMarkerSegmentTest.java.

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

PR Review: https://git.openjdk.org/jdk/pull/15082#pullrequestreview-1672683420
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1355881805
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1355874329
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1355880603
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1355891495
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1355839803


More information about the hotspot-dev mailing list