RFR: 8166607: G1 needs klass_or_null_acquire

[Thomas Schatzl]

Hi,

On Tue, 2016-10-25 at 19:11 -0400, Kim Barrett wrote:
> > 
> > On Oct 21, 2016, at 9:54 PM, Kim Barrett <kim.barrett at oracle.com>
> > wrote:
> > 
> > > 
> > > On Oct 21, 2016, at 8:46 PM, Kim Barrett <kim.barrett at oracle.com>
> > > wrote:
> > > In the humongous case, if it bails because klass_or_null == NULL,
> > > we must re-enqueue
> > > the card …
> This update (webrev.02) reverts part of the previous change.
> 
> In the original RFR I said:
> 
>   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.
> 
> Ignoring such a stale card is incorrect at the point where it was
> being done.  At that point we've already cleaned the card, so we must
> either process the designated object(s) or, if we can't do the
> processing because of in-progress allocation (klass_or_null returned
> NULL), then re-queue the card for later reconsideration.
> 
> So the change to refine_card to eliminate that behavior, and the
> associated changes to oops_on_card_seq_iterate_careful, were a
> mistake, and are being reverted by this new version.  As a result,
> refine_card is no longer changed at all.

Thanks for catching this.

Maybe it would be cleaner to call a method in the barrier set instead
of inlining the dirtying + enqueuing in lines 685 to 691? Maybe as an
additional RFE.

> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8166607
> 
> Webrev:
> Full: http://cr.openjdk.java.net/~kbarrett/8166607/webrev.02/
> Incr: http://cr.openjdk.java.net/~kbarrett/8166607/webrev.02.inc/
> 
> Testing:
> Local specjbb2015 (non-perf).
> Local jtreg hotspot_all.
> Also tested as baseline of changes for JDK-8166811.
> 
> Additionally, in the original RFR I also said:
> 
>   Note that [...] At present the only source of stale cards in the
>   concurrent case seems to be HCC eviction.  [...]  Doing HCC cleanup
>   when freeing regions might remove the need for klass_or_null
>   checking in the humongous case for concurrent refinement, so might
>   be worth looking into later.
> 
> That was also incorrect; there are other sources of stale cards.

Can you elaborate on that?

>  That doesn't affect this change, but may effect how JDK-8166811
> should be fixed, and removes the rationale for JDK-8166995 (which has
> been resolved Won't Fix because of that).
> 
> See also the RFR for the followup JDK-8166811.

Thanks,
  Thomas