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

[Stefan Johansson]

On Thu, 30 Nov 2023 21:43:36 GMT, Man Cao <manc at openjdk.org> wrote:

>> @simonis was the original suggester of this counter, so I will defer to his expertise. I do agree that dropping the counter would simplify things, but it also might not hurt to just leave it in. I'm okay with either option!
>
> Right, see @simonis 's comments at https://github.com/openjdk/jdk/pull/15082#pullrequestreview-1613868256, https://github.com/openjdk/jdk/pull/15082#discussion_r1321703912.
> 
> I initially had similar thought that `gc_total` isn't necessary and provides redundant data. Now I agree with @simonis that the `gc_total` is valuable to users. It saves users from extra work of aggregating different sets of counters for different garbage collectors, and potential mistakes of missing some counters. It is also future-proof that if GC implementation changes that add additional threads, users wouldn't need to change their code to add the counter for additional threads.
> 
> I think the maintenance overhead is quite small for `gc_total` since it is mostly in this class. The benefit to users is worth it.

I agree that the counter is valuable if always up-to-date, but if it is out of sync compared to the "concurrent counters" I think it will confuse some users. So if we want to keep it I think we should try to keep it in sync. 

I suppose adding a lock for updating `gc_total` should be ok. In the safepoint case we should never contend on that lock and for the concurrent updates it should not be a big deal. Basically what I think would be to change `update_counter(...)` to do something like:

if (CPUTimeGroups::is_gc_counter(name)) {
  <lock gc_total_lock>
  instance->get_counter(CPUTimeGroups::CPUTimeType::gc_total)->inc(net_cpu_time);
}


This way we would also be able to remove the publish part above, right? Any other problems with this approach?

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

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