RFR: 8369449: Spec: introduce new JVMTI function GetTotalGCCpuTime [v3]

[Serguei Spitsyn]

On Sat, 18 Oct 2025 00:56:25 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> With [JDK-8359110](https://bugs.openjdk.org/browse/JDK-8359110) a framework to measure GC CPU time was introduced.
>> It will be exposed in JMX as `MemoryMXBean.getTotalGcCpuTime()`. There is also interest to get the same performance data from JVMTI.
>> The following API's are being added with this enhancement:
>> 
>> Introduce:
>>  - new capability: `can_get_gc_cpu_time`
>>  - new JVMTI functions:
>>     - `jvmtiError GetGCCpuTimerInfo(jvmtiEnv* env, jvmtiTimerInfo* info_ptr)`
>>     - `jvmtiError GetTotalGCCpuTime(jvmtiEnv* env, jlong* nanos_ptr)`
>> 
>> **CSR**: [8370159](https://bugs.openjdk.org/browse/JDK-8370159): Spec: introduce new JVMTI function GetTotalGCCpuTime
>> 
>> Testing:
>>  - TBD: Mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix a typo in GetGCCpuTimerInfo: long => jlong

> I'm wondering, would a user trying to call GetTotalGCCpuTime if can_get_gc_cpu_time is not successfully set to 1 be undefined behavior? The specs say "To possess a capability, the agent must add the capability (https://docs.oracle.com/en/java/javase/25/docs/specs/jvmti.html#capability). If yes maybe we can discard the extra call to os::is_thread_cpu_time_supported in JvmtiEnv::GetTotalGCCpuTime? That seems to align with the pattern to not have that check in the other methods as you pointed out.

Right thinking, thanks. In fact, I had a plan to move this check to the capability side after the week ends. :)

> Should this be called GetTotalGCCpuTimerInfo?

I was already thinking on this for some time. I wanted to generalize this timer a little bit for a case if we ever decide to add more GC related functions. I'm not sure that all timers have to be strictly bound to the `CpuTime` function` and can be potentially reused for some other cases.

> There may be two issues with the patch as is:
> Calling GetTotalGCCpuTime in Agent_OnLoad can cause crash (since CollectedHeap::gc_threads_do do not protect against races on VM startup/shutdown).
>
> If GetTotalGCCpuTime is invoked in a callback for GC start/end, this will cause a deadlock as the Heap_lock is already held. The MutexLocker hl(Heap_lock) pattern was introduced to avoid races that could happen from internal usage in G1 of CPUTimeUsage::GC::total() during shutdown. I could recall this wrong but I think the usage Heap_lock (which evidently has uses in other places) is an optimization to avoid having to create a new mutex shutdown variable. I could be wrong but it is maybe possible that this deadlock would be resolved by introducing a new mutex only used for syncing on the state of Universe::_is_shutting_down. I will ask @walulyai for his thoughts.

It is nice you have caught this early. It would be nice to sort out this issue, and it is better to fix it on the GC side. I'd suggest to file a bug to separate this issue.

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

PR Comment: https://git.openjdk.org/jdk/pull/27879#issuecomment-3423402965