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

[Man Cao]

On Mon, 11 Sep 2023 15:10:23 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 461:
>> 
>>> 459: 
>>> 460:     _g1_concurrent_mark_threads_cpu_time =
>>> 461:         PerfDataManager::create_counter(SUN_THREADS, "g1_conc_mark_thread.cpu_time",
>> 
>> I find a bit odd to have "." in the name. "." should be the separator for namespace, but not within the counter name.
>> 
>> I think @simonis 's suggestion about  `sun.gc.collector.<n>.cpu_time` or `sun.gc.cpu_time` is to have a single, aggregated counter named `cpu_time`. If we don't do such aggregation, the names should probably be `g1_conc_mark_cpu_time`, `parallel_gc_workers_cpu_time`, etc.
>
> I suggested to have a single, aggregated counter for all GC threads **in addition** to the specific counters. This aggregated counter should be the same for all GCs which implement CPU times. It could easily be used by tools without knowing which GC is enabled and even more important, it would be immune to implementation changes (e.g. if a GC would establish a new subset of GC worker threads). From what I understand this is also your primary use case for your adaptable heap sizing feature.
> 
> The aggregated counter should be in a generic place for *all* GCs (e.g. `sun.gc.cpu_time` or `sun.threads.gc_cpu_time`). For the JIT we could then add the corresponding `sun.ci.cpu_time` or `sun.threads.jit_cpu_time` (in addition to the more specific counters for C1, C2, etc.).
> 
> For the specific GC counters: instead of putting the `.cpu_time` in the counter name, you can create a sub-namespace instead (e.g. `sun.gc.cpu_time.*` or `sun.threads.cpu_time.*`) and add all the other counters under that namespace (e.g. `sun.gc.cpu_time.total`, `sun.gc.cpu_time.conc_mark`, `sun.gc.cpu_time.parallel_gc_workers`, etc... What I want to avoid in any case is to have the name of the GC in every single counter name because that's redundant information.

Thanks for the clarification. Yes, adding a new counter for aggregation sounds good.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1322182754