On Tue, 30 Sep 2025 04:29:40 GMT, Bernd <[email protected]> wrote:
>> Hi all,
>>
>> This PR augments the CPU time sampling measurement capabilities that a user
>> can perform from Java code with the addition of
>> `MemoryMXBean.getGcCpuTime()`. With this patch it will be possible for a
>> user to measure process and GC CPU time during critical section or
>> iterations in benchmarks to name a few. This new method complements the
>> existing `OperatingSystemMXBean.getProcessCpuTime()` for a refined
>> understanding.
>>
>> `CollectedHeap::gc_threads_do` may operate on terminated GC threads during
>> shutdown, but thanks to JDK-8366865 by @walulyai we can piggyback on the new
>> `Universe::is_shutting_down`. I have implemented a stress-test
>> `test/jdk/java/lang/management/MemoryMXBean/GetGcCpuTime.java` that may
>> identify reading CPU time of terminated threads. Synchronizing on
>> `Universe::is_shutting_down` and `Heap_lock` resolves this problem.
>>
>> FWIW; To my understanding we don't want to add a
>> `Universe::is_shutting_down` check in gc_threads_do as this may introduce a
>> performance penalty that is unacceptable, therefore we must be careful about
>> the few places where external users call upon gc_threads_do and may race
>> with a terminating VM.
>>
>> Tested: test/jdk/java/lang/management/MemoryMXBean/GetGcCpuTime.java,
>> jdk/javax/management/mxbean hotspot/jtreg/vmTestbase/nsk/monitoring on Linux
>> x64, Linux aarch64, Windows x64, macOS x64 and macOS aarch64 with release
>> and fastdebug.
>
> src/java.management/share/classes/java/lang/management/MemoryMXBean.java line
> 282:
>
>> 280: *
>> 281: * @return the total CPU time for all garbage collection
>> 282: * threads in nanoseconds.
>
> Not sure did I miss the discussion, other methods like
> getTotalCompilationTime() return millis, is it ok or required to use new
> units here?
It is indeed unfortunate that methods mix units but we are complementing,
`OperatingSystemMXBean.getProcessCpuTime()` which returns nanoseconds.
> test/jdk/java/lang/management/MemoryMXBean/GetGcCpuTime.java line 73:
>
>> 71: while (true) {
>> 72: long gcCpuTimeFromThread =
>> mxMemoryBean.getGcCpuTime();
>> 73: if (gcCpuTimeFromThread < -1) {
>
> None of the tests actually test if it is ever != -1 or if it is monotonically
> increasing or under which conditions (go? Platform? Build flags?) it is
> unsupported?
The reason why there is no test for that is because `-1` is a valid return
value. If this test is run on a system that does not support CPU time from os
or if it queried during shutdown when we are terminating threads we opt to
return `-1` for safety reasons. We cannot assume that OpenJDK is never run on a
platform that does not support certain optional `os` implementations.
The goal with this test is to verify the shutdown protection and the API.
Additionally, since this API is piggybacking on other methods (effectively in
`os`) that are actually responsible for the CPU time functionality, I believe
they would be responsible implement tests for what you suggest.
FWIW; I had a quick look and the tests that you are expecting might already be
found in e.g.
`test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/ThreadMonitor.java`,
`vmTestbase/nsk/jvmti/scenarios/capability/CM02/cm02t001/TestDescription.java`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27537#discussion_r2390416360
PR Review Comment: https://git.openjdk.org/jdk/pull/27537#discussion_r2390406537
