RFR: 8278917: Use Prev Bitmap for recording evac failed objects [v2]

[Stefan Johansson]

On Fri, 17 Dec 2021 14:52:08 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Stefan Johansson has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Albert review
>
> src/hotspot/share/gc/g1/g1EvacFailure.cpp line 193:
> 
>> 191: 
>> 192:       hr->note_self_forwarding_removal_end(live_bytes);
>> 193:       _g1h->verifier()->check_bitmaps("Self-Forwarding Ptr Removal", hr);
> 
> Now that you are editing this method, why `if (_evac_failure_regions->contains(hr->hrm_index()))`? I thought that's the precondition.

Did not check the call-chain, but I agree this should be an assert. Probably a left-over from the past when all heap regions were iterated.

Will fix.

> src/hotspot/share/gc/g1/g1FullGCCompactTask.cpp line 95:
> 
>> 93:     // location, but also clears the bitmap for it. This is needed
>> 94:     // for bitmap verification as well as for being able to use the
>> 95:     // prev_bitmap for evacuation failures.
> 
> Instead of clearing the bit for each obj individually, I wonder if clearing the bitmap for the whole region will be faster. IOW, just call `collector()->mark_bitmap()->clear_region(hr);` unconditionally.

I did this first, just removing the `G1VerifyBitmaps` condition. But this will cause a notable regression for regions with very few marked objects. I'm not certain that regression is to bad, but I did not see anything suggesting that clearing the whole bitmap afterwards would be quicker in any other scenario either. So went with the bit-by-bit approach.

> src/hotspot/share/gc/g1/g1RemSet.cpp line 1274:
> 
>> 1272:       if (hr->is_old()) {
>> 1273:         _g1h->clear_region_bitmap(hr);
>> 1274:       }
> 
> Does it make sense to assert "Young regions should always have cleared bitmaps"?

Certainly! Good idea, will fix.

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

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