RFR: 8273492: Move evacuation failure handling into G1YoungCollector [v3]

Albert Mingkun Yang ayang at openjdk.java.net
Fri Sep 24 12:06:59 UTC 2021


On Wed, 22 Sep 2021 15:29:24 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews for this change that moves young gc evacuation failure handling (`G1EvacFailureRegions`) from `G1CollectedHeap` to `G1YoungCollector`.
>> 
>> Testing: tier1-3
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
> 
>  - Merge branch 'master' into 8273492-move-evac-failure-handling-to-younggc
>  - sjohanss review
>  - Merge branch 'master' into 8273492-move-evac-failure-handling-to-younggc
>  - Move evac failure handling to young gc

I think the original placement about the design, update stats iff `evac_fail == false`, is better; this is sth internal to `G1Policy`, not a caller's decision. IOW, sth like:


G1Policy::record_young_collection_end(bool concurrent_operation_is_full_mark, bool evac_fail) {
  // Evacuation failures skew the timing too much ...
  update_stats = !evac_fail;
}


The same apply to `G1Policy::record_pause`.

Not strictly part of this PR:

`num_regions_failed_evacuation` is just a getter, right? Why the name difference btw the method and the field?

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

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



More information about the hotspot-gc-dev mailing list