RFR: 8364638: Refactor and make accumulated GC CPU time code generic [v10]
Albert Mingkun Yang
ayang at openjdk.org
Tue Aug 19 11:35:39 UTC 2025
On Tue, 19 Aug 2025 08:42:15 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:
>
> Remove unused
src/hotspot/share/gc/shared/collectedHeap.cpp line 625:
> 623: ClassLoaderDataGraph::print_on(&ls_trace);
> 624: }
> 625: }
I don't think this code movement should be done -- this calls back to `Universe` and CLDG printing is not necessarily tired to heap.
src/hotspot/share/services/cpuTimeUsage.cpp line 43:
> 41: public:
> 42: virtual void do_thread(Thread* thread) {
> 43: jlong cpu_time = os::thread_cpu_time(thread);
I guess a wrapper for `thread_cpu_time` can be created to group error-handling logic together, for all uses in this file.
jlong thread_cpu_time_or_zero(thread) {
jlong cpu_time = os::..
if (cpu_time == -1) {
// mark-error
return 0;
}
return cpu_time;
}
src/hotspot/share/services/cpuTimeUsage.hpp line 47:
> 45: static jlong gc_threads();
> 46: static jlong vm_thread();
> 47: static jlong stringdedup();
I feel the API surface contains some redundancy.
For example, the GC-part API design exposes two ways to query and they are essentially the same -- for the sake of simplicity, I'd suggest keeping only one.
The calculation of `total` should be done by users of this API, I believe, when/if total is desirable.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26621#discussion_r2284933253
PR Review Comment: https://git.openjdk.org/jdk/pull/26621#discussion_r2284962585
PR Review Comment: https://git.openjdk.org/jdk/pull/26621#discussion_r2284952784
More information about the hotspot-dev
mailing list