RFR: 8254167 G1: Record regions where evacuation failed to provide targeted iteration [v6]

[Thomas Schatzl]

On Fri, 3 Sep 2021 03:19:57 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> This is another try to optimize evcuation failure for regions.
>> I record evacuation failed regions, and iterate these regions directly rather than iterate the whole cset.
>> The implementation reuses and refactors some of the existing data in g1CollectedHeap (_regions_failed_evacuation, _num_regions_failed_evacuation), and records these regions in an array, and iterate this array later in post evacuation phase.
>> 
>> I have 2 implementations:
>> 
>> - 1) CHT (Initial version)
>> - 2) bitmap (latest version, reuse _regions_failed_evacuation in g1CollectedHeap)
>> 
>> This implementation does not consider work distribution as mentioned in JDK-8254167 yet. But seems it already get better&stable performance gain than origin. We could improve it further later if work distribution is necessary.
>> 
>> I will attach the perf data in JBS.
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Refactor par iteration for regions of cset and evac failure.

Some minor issues remain. Sorry for not noticing some of them earlier.

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2318:

> 2316: 
> 2317: void G1CollectedHeap::par_iterate_regions_array_part_from(HeapRegionClosure* cl,
> 2318:                                         HeapRegionClaimer* hr_claimer,

Indentation.

src/hotspot/share/gc/g1/g1CollectedHeap.hpp line 1210:

> 1208:   }
> 1209:   void collection_set_iterate_increment_from(HeapRegionClosure *blk, HeapRegionClaimer* hr_claimer, uint worker_id);
> 1210:   // Iterate the part of a regions array given by the offset and length applying the given

Iterate part of an array of region indexes given by offset and length, applying the given HeapRegionClosure on each region. The worker_id....

src/hotspot/share/gc/g1/g1CollectedHeap.hpp line 1214:

> 1212:   // to allow for more efficient parallel iteration.
> 1213:   void par_iterate_regions_array_part_from(HeapRegionClosure* cl,
> 1214:                                    HeapRegionClaimer* hr_claimer,

Indentation - parameters should line up vertically.

src/hotspot/share/gc/g1/g1EvacFailure.cpp line 275:

> 273:   RemoveSelfForwardPtrHRClosure rsfp_cl(_rdcqs, worker_id, &_num_failed_regions);
> 274: 
> 275:   // Iterates through only the regions recorded as evacuation failure.

I would write something like:

// Iterate through all regions that failed evacuation during the entire collection.

But the code is kind of obvious to me now; the previous long-ish comment has only been there because to not over-optimize like has been done before. Your call.

src/hotspot/share/gc/g1/g1EvacFailureRegions.cpp line 53:

> 51:                                        uint worker_id) {
> 52:   G1CollectedHeap::heap()->par_iterate_regions_array_part_from(closure,
> 53:                                                         _hrclaimer,

Indentation.

src/hotspot/share/gc/g1/g1EvacFailureRegions.hpp line 28:

> 26: #define SHARE_GC_G1_G1EVACFAILUREREGIONS_HPP
> 27: 
> 28: #include "utilities/bitMap.inline.hpp"

need "runtime/atomic.hpp" too (or so) for the `Atomic` operations uses.

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

PR: https://git.openjdk.java.net/jdk/pull/5272