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

[Stefan Johansson]

On Fri, 1 Dec 2023 22:37:58 GMT, Jonathan Joo <jjoo at openjdk.org> wrote:

>> I think the ideal approach to simplify this is to support Atomic operation on a `PerfCounter`.
>> We could either introduce a `PerfAtomicCounter`/`PerfAtomicLongCounter` class, or perform `Atomic::add()` on the `PerfData::_valuep` pointer. There's already `PerfData::get_address()`, so we might be able to do:
>> 
>> 
>> Atomic::add((volatile jlong *)(instance->get_counter(CPUTimeGroups::CPUTimeType::gc_total)->get_address()), net_cpu_time);
>> 
>> 
>> However, a new class `PerfAtomicCounter`  is likely cleaner. E.g., we may also want to make `PerfAtomicCounter::sample()` use a CAS. It is probably better to introduce `PerfAtomicCounter` in a separate RFE later.
>> 
>> Would the `Atomic::add()` with `PerfData::get_address()` approach be OK for now, or would we rather introduce a lock, or leave the `gc_total` mechanism as-is and address the out-of-sync-ness in a follow-up RFE?
>> 
>> IMO the out-of-sync-ness problem is minor, because users are likely to either look at a single `gc_total` counter, or look at each individual GC  CPU counter and disregard `gc_total`.
>
> In the interest of the RDP1 deadline, should we leave improving the sync issues with gc_total to a separate RFE? (Especially given that a "correct" design may take some time to come up with, and that gc_total being slightly out of sync is not a major issue.)

Me and Albert discussed this again and we are ok with handling the `gc_total` sync issue as a follow up. Please create the RFE for that. If that would include needing a `PerfAtomicCounter`, that would a be its own RFE as well. For me I think a lock would be a good enough solution. 

>From our point of view having the counters out of sync for a long period of time (think a long concurrent mark cycle without any young collections updating the total) is not good since it shows that the counters are not incremented in sync. It would also be nice to avoid the two-step updating of the total time, so please try to find time to work on this.

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

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