RFR: 8359110: Log accumulated GC and process CPU time upon VM exit [v14]

[Albert Mingkun Yang]

On Wed, 2 Jul 2025 16:32:31 GMT, Jonas Norlinder <duke at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/collectedHeap.cpp line 630:
>> 
>>> 628:     double process_cpu_time = os::elapsed_process_cpu_time();
>>> 629:     double gc_cpu_time = elapsed_gc_cpu_time();
>>> 630:     double string_dedup_cpu_time = UseStringDeduplication ? (double)os::thread_cpu_time((Thread*)StringDedup::_processor->_thread) / NANOSECS_PER_SEC : 0;
>> 
>> If we consider stringdedup as gc-time, should this line be inlined to `elapsed_gc_cpu_time`?
>
> I'd prefer not to, as it is strictly not GC-time and we may want to use these methods in other places in the future.

That would result into inconsistency, because we surely treat them as gc-time here. It'd be super confusing if the def of gc-time is not coherent inside JVM.

>> src/hotspot/share/gc/shared/collectedHeap.hpp line 135:
>> 
>>> 133:   NOT_PRODUCT(volatile size_t _promotion_failure_alot_gc_number;)
>>> 134: 
>>> 135:   static jlong _vmthread_cpu_time;
>> 
>> I wonder if it's more natural to make it non-static.
>
> Could you please elaborate? Other variables like `static size_t _stack_chunk_max_size` is also static.

This class contains both static and non-static fields. Since `_vmthread_cpu_time` is accessed in `elapsed_gc_cpu_time` (non-static), I tend to think it should be non-static. (`add_vmthread_cpu_time` is static, but its sole caller actually use the instance, i.e. dropping the static for it still compiles.)

>> src/hotspot/share/gc/shared/collectedHeap.hpp line 212:
>> 
>>> 210: 
>>> 211:   // Stop any onging concurrent work and prepare for exit.
>>> 212:   virtual void stop() = 0;
>> 
>> Why `= 0` instead of `{}` as before?
>
> It was suggested by @stefank in above discussions. https://github.com/openjdk/jdk/pull/25779#discussion_r2162341225 Is this something you think we should revert?

I understand the part of splitting it into two methods, but why making it pure-virtual? Wouldn't `{}` as before work?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25779#discussion_r2180979113
PR Review Comment: https://git.openjdk.org/jdk/pull/25779#discussion_r2180969511
PR Review Comment: https://git.openjdk.org/jdk/pull/25779#discussion_r2180975563