RFR: 8256265: G1: Improve parallelism in regions that failed evacuation [v5]

[Albert Mingkun Yang]

On Tue, 13 Sep 2022 16:00:05 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews on this change that makes the evacuation failure remove forwards phase split the work across threads within regions?
>> 
>> This work is a continuation of PR#7047 from @Hamlin-Li and latest improvements from @albertnetymk in that thread. This is reflected in the first commit, I added a fair amount of changes to fix bugs and streamline the code.  
>> 
>> Testing: tier1-8 with G1EvacuationFailureALot enabled, tier1-5 as is
>
> Thomas Schatzl has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - clear_bitmap_for_region() -> clear_mark_data_for_region()
>  - ayang review 3

> another that TAMS is intrinsically linked to the bitmap (and marking) so it should be managed (like TARS) by G1ConcurrentMark

I don't think tams and the bitmap are _intrinsically_ linked; they are two independent entities. Only in the marking context, are they linked. tams is intrinsically linked to marking but not the bitmap.

> That G1 messes with the bitmap for evacuation failure handling is from an abstraction POV not good, but overall acceptable (and adds these warts).

I don't view it as a leaky abstraction, since the bitmap is not really tied to marking in my mind. Recording evac-fail objs in the bitmap is completely isolated from the use of the bitmap during concurrent-marking.

> This omits that call in the second invocation of G1CollectedHeap::clear_bitmap_for_region() because TAMS must be bottom() at that point.

That's one of the reasons why I suggested moving tams assignment out.

> I do not think this difference is the heart of this review and should not be preventing progress :)

Agree; this is not a blocker.

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

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