On Sat, 27 Sep 2025 11:18:58 GMT, Jonas Norlinder <[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/hotspot/share/include/jmm.h line 85:

> 83:   JMM_GC_COUNT                       = 10,   /* Total number of 
> collections */
> 84:   JMM_GC_CPU_TIME                    = 11,   /* Total accumulated GC CPU 
> time */
> 85:   JMM_JVM_UPTIME_MS                  = 12,   /* The JVM uptime in 
> milliseconds */

It looks a bit odd to me to change the existing define of UPTIME here.  
OK it is not a public interface used between different versions, and people 
should not be mixing up jmm.h and management implementations...  But usually we 
would just add the new definition? (items below are not strictly grouped by 
name)  Maybe this is just a nit, and only makes cross-version comparisons 
easier.

Looks good overall.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27537#discussion_r2387698942

Reply via email to