RFR: 8290376: G1: Refactor G1MMUTracker::when_sec

[Thomas Schatzl]

On Fri, 15 Jul 2022 14:14:07 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Introducing a more intuitive implementation.
> 
> This PR consists of two commits: the first adds the new algorithm and verifies the result is same as before. The second commit removes the old implementation.
> 
> Test: tier1-6 for the first commit.

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/g1/g1MMUTracker.cpp line 129:

> 127: // over them from the newest to the oldest (right-to-left in the diagram) and
> 128: // try to locate the timestamp annotated with ^, so that the accumulated GC
> 129: // time inside [deficit, current_timestamp], is equal to gc_budget. Next,

`gc_budget` is not defined anywhere in this comment or the diagram. This is confusing.

src/hotspot/share/gc/g1/g1MMUTracker.cpp line 136:

> 134:   assert(pause_time > 0.0, "precondition");
> 135: 
> 136:   // Clamp it by max

Suggestion:

    // If the pause is over the maximum, just assume that it's the maximum.

I would prefer the original comment as it explicitly states that this is a conscious decision.

src/hotspot/share/gc/g1/g1MMUTracker.cpp line 142:

> 140: 
> 141:   double limit = current_timestamp + pause_time - _time_slice;
> 142:   // Iterate from newest to oldest

Suggestion:

  // Iterate from newest to oldest.

src/hotspot/share/gc/g1/g1MMUTracker.cpp line 146:

> 144:     int index = trim_index(_head_index + i);
> 145:     G1MMUTrackerElem *elem = &_array[index];
> 146:     // Outside the window

Suggestion:

    // Outside the window.

src/hotspot/share/gc/g1/g1MMUTracker.cpp line 152:

> 150: 
> 151:     double duration = (elem->end_time() - MAX2(elem->start_time(), limit));
> 152:     // Would exceed the budget; strictly greater than

Suggestion:

    // This duration would exceed the budget. Check for strictly greater than.

Not sure what the "[Check for ]strictly greater than" adds, but it's okay.

src/hotspot/share/gc/g1/g1MMUTracker.cpp line 154:

> 152:     // Would exceed the budget; strictly greater than
> 153:     if (duration > gc_budget) {
> 154:       // The timestamp where a budget deficit occurs.

Suggestion:

      // This GC event contains the timestamp where the budget deficit occurs.

src/hotspot/share/gc/g1/g1MMUTracker.cpp line 163:

> 161:   }
> 162: 
> 163:   // No enough gc time inside the window; a budget surplus

Suggestion:

  // Not enough gc time spent inside the window, we have a budget surplus.

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

PR: https://git.openjdk.org/jdk/pull/9517