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

[Thomas Schatzl]

> 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

Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:

  ayang review2 - optimize remove()

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5786/files
  - new: https://git.openjdk.java.net/jdk/pull/5786/files/3cebd394..23b14f15

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5786&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5786&range=02-03

  Stats: 14 lines in 1 file changed: 4 ins; 5 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5786.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5786/head:pull/5786

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