Request for Review (s) - 8149343: assert(rp->num_q() == no_of_gc_workers) failed: sanity (original) (raw)

Thomas Schatzl thomas.schatzl at oracle.com
Wed Mar 16 10:13:30 UTC 2016

Hi Jon,

On Tue, 2016-03-15 at 15:19 -0700, Jon Masamitsu wrote:

Thomas,

Thanks for having a look. On 3/15/2016 5:12 AM, Thomas Schatzl wrote: > Hi Jon, > > On Thu, 2016-03-10 at 11:53 -0800, Jon Masamitsu wrote: > > https://bugs.openjdk.java.net/browse/JDK-8149343 > > > > The error here was that the number of active workers was > > not always set correctly for parallel Reference processing. > > The first fix was to set the number of active workers in the > > ReferenceProcessor in the task constructor. Once thatDiff > > was fixed a subsequent assertion failure occurred. > > > > # Internal Error (/export/jmasa/java/jdk9-rt > > -8149343/src/share/vm/gc/shared/referenceProcessor.cpp:884), > > pid=18444, tid=18500 > > # assert(id < maxnumq) failed: Id is out-of-bounds id 23 and_ _> > max > > id 23) > > > > This was fixed by the change > > > > - if (++nextid == numq) { > > + assert(!discoveryismt, "Round robin should only be used > > in > > serial discovery"); > > + if (++nextid >= numq) { > > > > See the CR for an example log which showed numq changing > > values between different phases of a collection and where the > > value of nextid was greater than numq. > I assume that the cause for the nextid being greater than numq > is that in a previous GC the number of threads has been larger than > in the current. Yes. The log in the CR shows the case where numq was 23, then 2 and then 23 again.

Then I understood it correctly :)

> While this change seems to work, I would kind of prefer that > nextid is simply reset in some initialization. The greater or > equal seems surprising here.

Early on I started to look for a place where I could add the initialization and decided not to. The nextid is part of the reference processor and reference processor created and reused so there didn't seem to be a dependable place to put the initialization. One place that might work is to set it in the setactivemtdegree() void setactivemtdegree(uint v) { numq = v; } Try it?

I think this would be an appropriate location.

As mentioned I would be okay with just a comment explaining the situation like Kim, so if this is urgent and you are really busy, don't.

Thanks, Thomas

More information about the hotspot-gc-dev mailing list