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 not sure this functionality is really JVMTI worthy but if Jonas thinks 
> this is useful for GC profiling then I will take his word for it.

Yes. I asked Jonas about it when we started our conversation and I rely on his 
justification which is in the enhancement description.

> This seems to contradict the description that it could be relative to a value 
> in the future and hence negative - thus it can't be "unsigned". But then I 
> don't see how it can be as described - for this timer to be useful it must be 
> tracking nanoseconds of CPU time consumed by GC since CPU time tracking 
> commenced, sometime after VM startup. This has to be similar to how thread 
> CPU time is defined.

Good catch, thanks. Fixed now.

> Also the hex value represents JDK 23 not 25.

Good catch, thanks. Fixed now.

> If it isn't supported then you can't have the capability and so won't reach 
> here.

Good comment. In fact, I've already made a decision to move this to the 
capability check. But it seems, it also needs to be fixed this way for 
`GetCurrentThreadCpuTime()` and `GetThreadCpuTime()`.

>> +       Universe::heap()->is_shutting_down()) {
>> +       *nanos_ptr = -1;
> 
> This seems wrong to me and violates the timer-info spec of this timer not 
> jumping backwards. I think you have to cache the last returned value for this 
> function and if you cannot calculate an updated value because of VM shutdown, 
> then that previous value should be returned.

I agree. I've already noticed this issue and was thinking where and how to fix 
it. Thank you for the suggestion. It can be fixed this way but I feel it is 
better to fix on the GC side. Will discuss it with Jonas.

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

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

Reply via email to