RFR: 8297186: G1 triggers unnecessary full GCs when heap utilization is low [v3]

[Thomas Schatzl]

On Wed, 30 Nov 2022 17:35:48 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Improve test avoiding heap fragmentation so that it succeeds
>
> src/hotspot/share/gc/g1/g1Policy.cpp line 316:
> 
>> 314:     log_trace(gc, ergo, heap)("Young target length: Desired young length smaller than allocated after GC. Allowing one extra eden region.");
>> 315:     desired_young_length = allocated_young_length + 1;
>> 316:   }
> 
> I don't understand this.  The log message doesn't match the test being performed (log says "smaller" but
> the test is for `<=`).  And the action taken seems kind of mysterious.  I think it's to force us into the `else`
> clause of the following `if` statement.  It might be more obvious if instead of this the `if` condition was
> changed to `(!after_gc && allocated_young_length >= desired_young_length)`, though then there is some
> additional stuff to do in the `else` branch too.  A comment instead would help.

The change forces the remaining code in `calculate_young_target_length` to add an extra region to the number of desired regions if the number of currently allocated regions is equal or exceeds them.
This override should only be used if the caller requested it, i.e. after a young gc to avoid an immediate full gc right away because the allocation that caused the gc could not be satisfied with zero eden regions.

E.g.
desired regions = 1 (because of very small heap size)
allocation regions = 1 (because of survivor regions from previous gc)

Now this leaves zero regions for eden for the next allocation, causing the described full gcs.
This code pretends to the actual target calculation (if after a gc) that we actually want one eden region.

As for the location of this `if`, I moved it to `calculate_young_desired_length` with a comment, since this code actually modifies the desired length. It's imho still a bit awkward there because it is kind of a tacked on condition, but I do not know a better place for this code (I've been fairly undecided about that in the original code alreayd).

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

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