RFR: 8166607: G1 needs klass_or_null_acquire

[Kim Barrett]

> On Oct 10, 2016, at 9:54 AM, Erik Helin <erik.helin at oracle.com> wrote:
> 
> On 2016-09-29, Kim Barrett wrote:
>> Please review this change to G1 to use klass_or_null_acquire where
>> needed.
>> 
>> […]
>> 
>> Changed oops_on_card_seq_iterate_careful to use different code paths
>> when the region is humongous or not.
>> 
>> The humongous case is simplified because there can only be the one
>> (potential) object in the region.  Use klass_or_null_acquire to
>> determine whether the object has been "published" or allocation is
>> still in-progress.  The acquire barrier ensures processing the
>> humongous object is safe.  When the klass is NULL the card marking
>> must be stale and can be ignored.
>> 
>> In the non-humongous case, we know we're dealing with an old-gen
>> region.  Because of that, we know we don't need to deal with
>> in-progress allocations here.  That allows many simplifications.
>> 
>> As a result of the changes in oops_on_card_seq_iterate_careful, we now
>> almost never fail to process the card.  The only place where that can
>> occur is a stale card in a humongous region with an in-progress
>> allocation, where we can just ignore it.  So the only caller,
>> refine_card, no longer needs to examine the result of the call and
>> enqueue the card for later reconsideration.
> 
> I'm not sure this assumption is correct. Due to JDK-8166711 [0], can't
> we have the following scenario:
> 
> […]
> What do you think? This is very tricky code to analyze, but I think this
> patch won't work in the above scenario.
> 
> When JDK-8166711 [0] is solved however, then I do think that this patch
> will work. The important part for getting this patch to work is to ensure
> that the two first checks, mr.is_empty() and is_young(), aren't
> reordered.

I think the existing code can also fail in such a JDK-8166711 scenario.

I don’t think my proposed changes make that situation any worse, and will
enable changes I think can be made to fix 8166711 that will both simplify things
further and might not require any additional barriers.  Note that 8166711 is the
next thing I plan to work on.  I could add a loadload barrier as a temporary
stopgap; that didn’t seem necessary since the failures are pretty low probability
(not sure we’ve ever seen one) and I’m going to work on what I think will be
a better fix for that problem next.