RFR: 8364638: Refactor and make accumulated GC CPU time code generic [v5]

[Stefan Johansson]

On Tue, 12 Aug 2025 12:24:35 GMT, Jonas Norlinder <duke at openjdk.org> wrote:

>> Hi all,
>> 
>> This PR refactors the newly added GC CPU time code from [JDK-8359110](https://bugs.openjdk.org/browse/JDK-8359110).
>> 
>> As a stepping-stone to enable consolidation of CPU time tracking in e.g. hsperf counters and GCTraceCPUTime and to have a unified interface for tracking CPU time of various components in Hotspot this code can be refactored. This PR introduces a new interface to retrieve CPU time for various Hotspot components and it currently supports:
>> 
>> CPUTimeUsage::GC::total() // the sum of gc_threads(), vm_thread(), stringdedup()
>> 
>> CPUTimeUsage::GC::gc_threads()
>> CPUTimeUsage::GC::vm_thread()
>> CPUTimeUsage::GC::stringdedup()
>> 
>> CPUTimeUsage::Runtime::vm_thread()
>> 
>> 
>> I moved `CPUTimeUsage` to `src/hotspot/share/services` since it seemed fitting as it housed similar performance tracking code like `RuntimeService`, as this is no longer a class that is only specific to GC.
>> 
>> I also made a minor improvement in the CPU time logging during exit. Since `CPUTimeUsage` supports more components than just GC I changed the logging flag to from `gc,cpu` to `cpu` and created a detailed table:
>> 
>> 
>> [71.425s][info][cpu   ] === CPU time Statistics =============================================================
>> [71.425s][info][cpu   ]                                                                             CPUs
>> [71.425s][info][cpu   ]                                                                s       %  utilized
>> [71.425s][info][cpu   ]    Process
>> [71.425s][info][cpu   ]      Total                                             1616.3627  100.00      22.6
>> [71.425s][info][cpu   ]      VM Thread                                            5.2992    0.33       0.1
>> [71.425s][info][cpu   ]      Garbage Collection                                  83.7322    5.18       1.2
>> [71.425s][info][cpu   ]        GC Threads                                        82.7671    5.12       1.2
>> [71.425s][info][cpu   ]        VM Thread                                          0.9651    0.06       0.0
>> [71.425s][info][cpu   ] =====================================================================================
>> 
>> 
>> Additionally, if CPU time retrieval fails it should not be the caller's responsibility to log warnings as this would bloat the code unnecessarily. I've noticed that `os` does log a warning for some methods if they fail so I continued on this path.
>
> Jonas Norlinder has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Feedback from Albert

Thanks for looking at this and improving the CPU tracking and making it easily accessible. 

Overall I think the change looks good, just a few comments/questions.

src/hotspot/os/linux/os_linux.cpp line 4953:

> 4951:       // to detach itself from the VM - which should result in ESRCH.
> 4952:       assert_status(rc == ESRCH, rc, "pthread_getcpuclockid failed");
> 4953:       log_warning(os)("Could not sample thread CPU time (return code %d)", rc);

Do we really want add a warning here? This API is used from a lot a places and reading the comment above i sound like it might happen from time to time without it being a very big deal?

The risk I see is that we sometimes will get a bunch of these warning in test and they will fail because we can't handle the additional output.

The same goes for the other warnings below.

src/hotspot/share/gc/shared/collectedHeap.cpp line 655:

> 653:   print_tracing_info();
> 654: 
> 655:   log_cpu_time();

As mentioned offline, I would prefer if this was moved away from `CollectedHeap` since it is now not only GC/Heap related times that are tracked. 

We could add a `Universe::before_exit()` which firsts does the CPU logging before calling `heap()->before_exit()`. As long as we do the logging before, would there be any problem with this?

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

Changes requested by sjohanss (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26621#pullrequestreview-3119416367
PR Review Comment: https://git.openjdk.org/jdk/pull/26621#discussion_r2275777432
PR Review Comment: https://git.openjdk.org/jdk/pull/26621#discussion_r2275805357