RFR: 8140326: G1: Consider putting regions where evacuation failed into next collection set [v4]

[Thomas Schatzl]

On Fri, 9 Jun 2023 13:12:29 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Thomas Schatzl has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - More debug option removal
>>  - Remove debug options for tests
>
> src/hotspot/share/gc/g1/g1CollectionSetCandidates.hpp line 138:
> 
>> 136: 
>> 137: // Iterator for G1CollectionSetCandidates. Multiplexes across the marking/retained
>> 138: // region lists based on gc efficiency.
> 
> Why does the iteration require ordering? Going through its use sites, I find they just need to perform SIMD like op on each element regardless of the order.

API consistency. All iterators return them sorted in efficiency order, and if not doing this for that as well will give unnecessary surprises.
I can change this if you insist, but I can already see problems with that in the future.

> src/hotspot/share/gc/g1/g1Policy.cpp line 655:
> 
>> 653: 
>> 654:   size_t threshold = G1MixedGCLiveThresholdPercent * HeapRegion::GrainBytes / 100;
>> 655:   return live_bytes < threshold;
> 
> If the region was Old at gc-start, it would have passed through the same criteria to be in cset; if the region was Young at gc-start, using a threshold meant only for Old-region is questionable, IMO.
> 
> Could retaining be performed unconditionally for now based on the assumption that evac-fail regions are sparse?

Testing showed that this can happen, i.e. fairly full young gen regions. As explained above, scheduling expensive regions within regular young gcs is painful which performance we want to keep unchanged as much as possible. This heuristic is fairly efficient to remove those (and also tests keeping regions that is required for the next change anyway.)

I understand that it is somewhat bad to use a metric that has old gen/mixed gc in it - I can add another option for that if you think it is better.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14220#discussion_r1227634772
PR Review Comment: https://git.openjdk.org/jdk/pull/14220#discussion_r1227633401