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

[Jonathan Joo]

On Thu, 9 Nov 2023 10:29:29 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Jonathan Joo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add missing cpuTimeCounters files
>
> src/hotspot/share/gc/g1/g1ConcurrentRefineThread.cpp line 189:
> 
>> 187: void G1PrimaryConcurrentRefineThread::maybe_update_threads_cpu_time() {
>> 188:   if (UsePerfData && os::is_thread_cpu_time_supported()) {
>> 189:     cr()->update_concurrent_refine_threads_cpu_time();
> 
> I think we should pull the tracking closure in here and that way leave the concurrent refine class untouched. 
> 
> Suggestion:
> 
>     // The primary thread is responsible for updating the CPU time for all workers.
>     CPUTimeCounters* counters = G1CollectedHeap::heap()->cpu_time_counters();
>     ThreadTotalCPUTimeClosure tttc(counters, CPUTimeGroups::gc_conc_refine);
>     cr()->threads_do(&tttc);
> 
> 
> This is more or less a copy from `G1ConcurrentRefineThreadControl::update_threads_cpu_time()` which if we go with this solution can be removed. The above needs some new includes though.
> 
> I change the comment a because I could not fully understand it, the primary thread is the one always checking and starting more threads so it is not stopped first. Also not sure when a terminated thread could be read. Even the stopped threads are still present so should be fine. If I'm missing something feel free to add back the comment.

Thank you for the review! Do you think you could take a look at the newest update and see if that aligns with what you were thinking? I wanted to make sure I understood your comments correctly.

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

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