On Sat, 18 Oct 2025 00:56:25 GMT, Serguei Spitsyn <[email protected]> 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
