RFR(s): 8029162: G1: Shared SATB queue never enabled

[Mikael Gerdin]

On Friday 29 November 2013 10.18.00 Thomas Schatzl wrote:
> Hi Per,
> 
> On Fri, 2013-11-29 at 09:47 +0100, Per Liden wrote:
> > Hi Mikael,
> > 
> > On 2013-11-29 08:59, Mikael Gerdin wrote:
> > > Per,
> > > 
> > > On Thursday 28 November 2013 17.49.53 Per Liden wrote:
> > >> Summary: When activating/deactivating the G1 pre-barrier all SATB
> > >> queues
> > >> are looped through and have their active state set accordingly. There
> > >> is
> > >> one queue per Java thread and a shared queue for all non-Java threads.
> > >> The loop done to set the active state of all queues forgets to set the
> > >> active state on the shared queue. I suspect this was missed because the
> > >> logic in SATBMarkQueueSet::set_active_all_threads() is mixed with quite
> > >> a bit of verification code. This patch also moves this verification
> > >> code
> > >> out into a separate function.
> > >> 
> > >> Testing done: jprt, kitchensink (10 hours), gcbasher (10 hours)
> > >> 
> > >> http://cr.openjdk.java.net/~pliden/8029162/webrev.0/
> > > 
> > > The change looks good to me.
> > 
> > Thanks for reviewing!
> > 
> > > Since this change enables the bug in JDK-8029255 I assume that these
> > > changes must be applied together?
> > 
> > Not necessarily applied together, but yes, JDK-892255 needs to be
> > applied first. That patch is not dependent on this fix.
> 
> Please strongly consider to push them in the same changeset (you can set
> two CRs in the header). This avoids somebody ever checking out/using a
> bugged version.

I agree.

/Mikael

> 
> Thanks,
> Thomas