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

[Jonathan Joo]

On Wed, 29 Nov 2023 15:24:52 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> src/hotspot/share/runtime/cpuTimeCounters.cpp line 91:
>> 
>>> 89:   } while (old_value != fetched_value);
>>> 90:   get_counter(CPUTimeGroups::CPUTimeType::gc_total)->inc(fetched_value);
>>> 91: }
>> 
>> Why do we have to do this publish dance? Couldn't the closure that update the diff instead just update the counter? From what I can tell we never have multiple closures active at the same time so should be no race there?
>
> This two-step update does seem unnecessary, IMO.

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?

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

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