RFR: 8315149: Add hsperf counters for CPU time of internal GC threads [v2]
Man Cao
manc at openjdk.org
Wed Sep 6 22:20:42 UTC 2023
On Wed, 6 Sep 2023 17:54:23 GMT, Volker Simonis <simonis at openjdk.org> wrote:
>> Jonathan Joo has updated the pull request incrementally with one additional commit since the last revision:
>>
>> address dholmes@ comments
>
> First of all thanks for this PR. I think it is useful and once it's done I think we're interested in implementing the corresponding counters for Shenandoah and also for the JIT compiler threads.
>
> First some general comments:
> - Instead of `PerfVariable` I'd use `PerfCounter` because they represent a "[*data value that can (should) be modified in a monotonic manner*](https://github.com/openjdk/jdk/blob/bd477810b176696e0fd043f5594663ebcf9884cf/src/hotspot/share/runtime/perfData.hpp#L419-L427)" rather than "*being modified in an unrestricted manner*".
> - Please put the counters into the appropriate namespace (e.g. `sun.gc.collector.<n>.cpu_time`). This fits better with the existing counters (we currently don't have counters outside the `java.` or `sun.` namespaces) and makes it easier for follow up changes to implement the corresponding counters for additional GCs.
> - Can you please aggregate all the different CPU time counters from the different GC phases into one GC CPU time counter (e.g. `sun.gc.cpu_time`). This would be the same for different GCs and would simplify the monitoring of GC overhead independently of the used GC algorithm.
> - Please add a test for the new counters.
Responding to some comments from @simonis:
> Please put the counters into the appropriate namespace (e.g. sun.gc.collector.<n>.cpu_time).
I suggested using the `SUN_THREADS` namespace instead, because:
- Counters like for VM thread and String Dedup thread do not belong to any GC collector namespace.
- It is hard to classify some counters into either `sun.gc.collector.0` or `sun.gc.collector.1`. E.g. `_perf_parallel_gc_threads_cpu_time` is used by both incremental and full collectors for G1. It is possible to put them under `sun.gc` directly though.
> Can you please aggregate all the different CPU time counters from the different GC phases into one GC CPU time counter (e.g. sun.gc.cpu_time).
Our experience for monitoring metrics is that it is best to provide individual metrics for each subcomponent. Monitoring tools can perform aggregation for these fine-grained metrics if they need to. If the JVM provide an aggregate metric, it is impossible to un-aggregate this metric. It is also hard to tell which subcomponent is the culprit if there's regression.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15082#issuecomment-1709199392
More information about the hotspot-dev
mailing list