Integrated: 8274516: [REDO] JDK-8271880: Tighten condition for excluding regions from collecting cards with cross-references

Thomas Schatzl tschatzl at openjdk.java.net
Wed Oct 13 08:14:55 UTC 2021


On Fri, 1 Oct 2021 08:58:35 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   can I have reviews for this redo of (JDK-8271880: Tighten condition for excluding regions from collecting cards with cross-references)[https://bugs.openjdk.java.net/browse/JDK-8271880]?
> 
> The JDK-8271180 change has been backed out because it crashed fairly easily on (kitchensink)[https://bugs.openjdk.java.net/browse/JDK-8274340] (and apparently also on some (ArrayJuggle tests)[https://bugs.openjdk.java.net/browse/JDK-8274452]).
> 
> The reason is that the original change missed application of the barriers during removal of an element from the discovered list. This change fixes that, properly applying it (calling the `enqueue` methods).
> 
> This change consist of two commits at this time: first the revert of the backout for JDK-8271180, and a second one with the actual fixes. For people who already looked at the former I recommend only looking at that the latter.
> 
> There are three interesting changes:
> * change of the `EnqueueDiscoveredFieldClosure::enqueue` fields' first parameter to directly take the address to patch. This simplifies some code.
> * introduction of the barrier in the `DiscoveredListIterator::remove` method - this should be the only relevant place where the barrier application has been missing. There is some special handling when removing from the start of the discovered list - the address points into the C-heap at that time and we should not apply a barrier here.
> * the `EnqueueDiscoveredFieldClosure::enqueue` method filters out writes to non-live reference objects (as per the `_is_alive` closure) - we may get called during removal of elements, and the next element is also not live. There is no need to apply the barrier to those. Actually, leaving this out could trigger an assertion in `G1ParScanThreadState::write_ref_field_post` where it checks that the object we refer to that is in the collection set must be self-forwarded (i.e. an object where evacuation failed).
>  
> Testing: tier1-5, kitchensink (30min) 720 times with no crash, the failing ArrayJuggle28 test 2000 times
> 
> Thanks,
>   Thomas

This pull request has now been integrated.

Changeset: c3b75c6c
Author:    Thomas Schatzl <tschatzl at openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/c3b75c6cdf03ffa3c887bf3db29e17668b228f79
Stats:     188 lines in 16 files changed: 80 ins; 75 del; 33 mod

8274516: [REDO] JDK-8271880: Tighten condition for excluding regions from collecting cards with cross-references

Reviewed-by: sjohanss, ayang

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

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



More information about the hotspot-gc-dev mailing list