RFR 8204947: Port ShenandoahTaskTerminator to mainline and make it default

[Thomas Schatzl]

On Wed, 2018-12-05 at 19:20 -0500, zgu at redhat.com wrote:
> > > 
> > > Ha, this indeed my bug, could you try following patch? or I can
> > > generate new webrev.
> > > 
> > > diff -r 26bd32dafb9b
> > > src/hotspot/share/gc/shared/owstTaskTerminator.cpp
> > > ---
> > > a/src/hotspot/share/gc/shared/owstTaskTerminator.cpp        Wed
> > > Dec
> > > 05 10:12:32 2018 -0500
> > > +++
> > > b/src/hotspot/share/gc/shared/owstTaskTerminator.cpp        Wed
> > > Dec
> > > 05 13:40:00 2018 -0500
> > > @@ -37,6 +37,7 @@
> > >  
> > >    // Single worker, done
> > >    if (_n_threads == 1) {
> > > +    _offered_termination = 1;
> > >      return true;
> > >    }
> > > 
> > 
> >   yes, that's an issue, but does not fix the real problem. It also
> > occurs with multiple threads
> 
> Yeah, I was wondering if it was the only problem, probably you would
> have noticed :-)
> 
> > .
> > 
> > Basically for that assert to work, all threads *must* offer
> > termination at some point. G1 code does not guarantee that. It can
> > completely pass over the offer_termination call by setting a bool
> > flag in many places.
> 
> But for the worker that offered termination, it has to exit the
> termination protocol to proceed. When it exits, it decreases
> _offered_termination count, so when workers rendezvous,
> _offered_termination should be zero, right? 

Yes, but some threads simply do not offer termination in all cases but
just exit :) That is the day-one bug in G1 we trip over here. I will
file a CR.

> > 
> > Other uses of the terminator just re-instantiate the terminator for
> > every use, automatically avoiding this issue.
> > 
> > To avoid rewriting too much of G1 code (I would prefer it would too
> > just re-instantiate everything needed for every marking cycle)
> > let's stay with your latest version with the assignment operator
> > being a move.
> > 
> > I will look at it tomorrow at the latest webrev in detail again,
> > but I think it is good.
> 
> Great! 
> 
> I incorporated all suggestions and the fixes.
> http://cr.openjdk.java.net/~zgu/JDK-8204947/webrev.05/
> 

- In psScavenge.cpp:172 the indentation is off

Looks good otherwise. I do not need a re-review for above change. I
will have this final change run through our testing one more time and
will report back (tomorrow).

Thanks,
  Thomas