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

[Stefan Johansson]

On Thu, 30 Nov 2023 06:45:02 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> I agree that in the case that multiple closures are not active at the same time, we wouldn't have to implement it in this way. However, isn't it possible to have multiple closures active simultaneously, e.g.  vm thread, concurrent mark thread, concurrent refine thread? I think to account for the races there, we can only update the `_gc_total_cpu_time_diff` member variable atomically during these closure destructions, and then publish the actual updated `gc_total` counter at manually specified times via `publish_gc_total_cpu_time()`. If we were to call `publish_gc_total_cpu_time` as part of the thread closure, I believe it would be difficult to prevent races when accessing the underlying counter from the various gc-related threads.
>> 
>> Or maybe there is another strategy that I'm not seeing?
>
> Both `publish_gc_total_cpu_time` and `~ThreadTotalCPUTimeClosure` are called by the vm-thread inside a safepoint, so there shouldn't be any other threads running simultaneously, I believe.

Me and Albert just spoke and we do see the problem that two concurrent threads could be executing the closure at the same time. So if having a total counter we need to sync the updates. But when talking we started to question how useful it is to have the gc_total counter. It is just an aggregate of the other gc-counters, but it is out of sync between safepoints. So you will always get a more accurate value by looking at the individual gc-counters.

We came to the conclusion that it would probably be easier to drop `gc_total` right now, rather than trying to keep it in sync for all updates to the individual counters. Because having them out of sync doesn't feel like a great option. 

Are we missing anything or do you agree?

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

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