RFR: 8254164: G1 only removes self forwarding pointers for last collection set increment [v2]

[Kim Barrett]

On Fri, 9 Oct 2020 07:21:39 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews for this change that fixes a previously hard to reproduce crash that started showing up much more
>>   frequently in conjunction with changes to young gen sizing (JDK-8244603)?
>> 
>> The issues is that the code to process regions where evacuation failed only processes the last increment. This leaves
>> forwarded pointers in the mark word of some objects. Obviously other code does not like that, e.g. the crashes in
>> JDK-8248438 which I plan to close as duplicate.  Only checking the last collection set increment for regions that
>> failed evacuation is wrong in case there is an evacuation failure caused by reference processing in a region that has
>> been evacuated in earlier evacuation increments. Reference processing (e.g. finalizers) can make it necessary to
>> resurrect an otherwise unreachable object at the very end of the collection that can't be copied and is located in a
>> region evacuated in an earlier increment.  This optimization to only look at the last increment for removal of self
>> forwarding pointers has been introduced in JDK-8218668.  Until changes to young gen sizing in JDK-8244603 this crashes
>> has been a very rare occurrence, but with that it has been common in some tier8 tests (KitchenSink8/24h, DaCapo24h)
>> particularly with some additional targeted verification (enable verification only at the end of mixed gcs with optional
>> evacuation). Without this fix both tests fail within 10 minutes to 2 hours. With the patch everything completes fine.
>> Testing:
>> - tier1-5
>> - KitchenSink/Dacapo24h with verification code and JDK-8244603.
>> 
>> Note: since this change strictly does more work during evacuation failure handling I consider this amount of testing
>> sufficient, i.e. with JDK-8244603. It is very hard to reproduce without JDK-8244603.
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   sjohanss comment

Looks good.  Well, as good as it can, given finalizers are still with us.

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

Marked as reviewed by kbarrett (Reviewer).

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