RFR: 8274051: Remove supports_vtime()/elapsedVTime()

[Kim Barrett]

On Thu, 26 Jun 2025 12:53:37 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   please review this change that removes the `*vtime()` methods from G1, replacing them with the appropriate `os::thread_cpu_time()` calls. They completely supersede the former ones.
> 
> Further I made support for `os::thread_cpu_time()` mandatory for G1, it will fail if not available. All platforms implement them, only non-OSX *BSD do not in mainline sources. However in the [official fork](https://github.com/freebsd/openjdk/blob/jdk24u-freebsd/src/hotspot/os/bsd/os_bsd.cpp#L2609) it already does.
> 
> There are upstreaming efforts underway too, so I do not see this as blocker for this behavior. 
> 
> One other change is that the various getters directly use the time returned by the OS function as cpu time, only counting the main work (marking steps, actual refinement) as used cpu time. I do not think this is honest to discount random cpu time spent in other work these threads must do (like getting to and leaving the main work described above).
> 
> There has been some effort to make naming a bit more consistent: cpu times were all named with `_cpu_time` with all non-standard units that unit appended (`_s`for seconds and so on).
> 
> Testing: tier1-3
> 
> Thanks,
>   Thomas

It makes me happy to see the vtime stuff removed.

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 2090:

> 2088: 
> 2089:     void do_thread(Thread* t) {
> 2090:       _total_cpu_time = os::thread_cpu_time(t);

Shouldn't this be `+=` rather than just `+`?

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 2272:

> 2270:   }
> 2271: 
> 2272:   jlong curr_time_ns = os::current_thread_cpu_time();

pre-existing: I wonder why this is here, rather than a few lines later, near where it's used.

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 2283:

> 2281:   // (5) We check whether we've reached our time quota. If we have,
> 2282:   // then we abort.
> 2283:   double elapsed_time_ms = (double)(curr_time_ns - _start_cpu_time_ns) / NANOSECS_PER_MILLISEC;

Maybe there should be a helper for this calculation, since there are several occurrences of it?

src/hotspot/share/gc/g1/g1RemSetSummary.cpp line 47:

> 45:     virtual void do_thread(Thread* t) {
> 46:       G1ConcurrentRefineThread* crt = static_cast<G1ConcurrentRefineThread*>(t);
> 47:       _summary->set_rs_thread_vtime(_counter, crt->cpu_time_s());

There's still a bunch of "vtime" nomenclature in this file. Is the plan to clean that up later?

src/hotspot/share/gc/g1/g1ServiceThread.cpp line 157:

> 155: 
> 156: void G1ServiceThread::update_perf_counter_cpu_time() {
> 157:   if (UsePerfData ) {

Stray space before close-paren.

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

PR Review: https://git.openjdk.org/jdk/pull/26001#pullrequestreview-2963322877
PR Review Comment: https://git.openjdk.org/jdk/pull/26001#discussion_r2169814422
PR Review Comment: https://git.openjdk.org/jdk/pull/26001#discussion_r2169819983
PR Review Comment: https://git.openjdk.org/jdk/pull/26001#discussion_r2169827699
PR Review Comment: https://git.openjdk.org/jdk/pull/26001#discussion_r2169865330
PR Review Comment: https://git.openjdk.org/jdk/pull/26001#discussion_r2169869495