RFR: 8346568: G1: Other time can be negative [v3]

[Sangheon Kim]

On Fri, 18 Apr 2025 09:38:33 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Sangheon Kim has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>> 
>>  - Review from Thomas
>>  - Separate measurement for cleanup
>
> src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp line 425:
> 
>> 423:   // Concurrent tasks of ResetMarkingState and NoteStartOfMark are triggered during
>> 424:   // young collection. However, their execution time are not included in _gc_pause_time_ms.
>> 425:   if (pre_concurrent_start_ms > 0.0) {
> 
> Since `pre_concurrent_start_ms` is now actually gathered, maybe print an extra line for it, with the `ResetMarkingState` and `NoteStartOfMark` log lines indented?
> 
> I.e. something like:
> 
> 
>   if (_cur_prepare_concurrent_task_time_ms > 0.0) {
>     debug_time("Prepare Concurrent Start", _cur_prepare_concurrent_task_time_ms);
>     debug_phase(_gc_par_phases[ResetMarkingState], 1);
>     debug_phase(_gc_par_phases[NoteStartOfMark], 1);
>   }
> 
> ?
> 
> Then we can also drop the calculation of the local `pre_concurrent_start_ms`.

Okay, this looks better.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24454#discussion_r2051391042