<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<font face="Times New Roman, Times, serif">The change to
re-initialize _next_id passed testing so is now<br>
the proposed fix.<br>
<br>
Delta from original fix:<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/8149343/webrev_delta.00_01/">http://cr.openjdk.java.net/~jmasa/8149343/webrev_delta.00_01/</a><br>
<br>
Full fix version 01<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/8149343/webrev.01/">http://cr.openjdk.java.net/~jmasa/8149343/webrev.01/</a><br>
<br>
Thanks.<br>
<br>
Jon<br>
</font><br>
<div class="moz-cite-prefix">On 03/16/2016 08:51 AM, Jon Masamitsu
wrote:<br>
</div>
<blockquote cite="mid:56E9810F.4060701@oracle.com" type="cite">
<br>
<br>
On 03/16/2016 03:13 AM, Thomas Schatzl wrote:
<br>
<blockquote type="cite">Hi Jon,
<br>
<br>
On Tue, 2016-03-15 at 15:19 -0700, Jon Masamitsu wrote:
<br>
<blockquote type="cite">Thomas,
<br>
<br>
Thanks for having a look.
<br>
<br>
On 3/15/2016 5:12 AM, Thomas Schatzl wrote:
<br>
<blockquote type="cite">Hi Jon,
<br>
<br>
On Thu, 2016-03-10 at 11:53 -0800, Jon Masamitsu wrote:
<br>
<blockquote type="cite"><a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8149343">https://bugs.openjdk.java.net/browse/JDK-8149343</a>
<br>
<br>
The error here was that the number of active workers was
<br>
not always set correctly for parallel Reference
processing.
<br>
The first fix was to set the number of active workers in
the
<br>
ReferenceProcessor in the task constructor. Once thatDiff
<br>
was fixed a subsequent assertion failure occurred.
<br>
<br>
# Internal Error (/export/jmasa/java/jdk9-rt
<br>
-8149343/src/share/vm/gc/shared/referenceProcessor.cpp:884),
<br>
pid=18444, tid=18500
<br>
# assert(id < _max_num_q) failed: Id is out-of-bounds
id 23 and
<br>
max
<br>
id 23)
<br>
<br>
This was fixed by the change
<br>
<br>
- if (++_next_id == _num_q) {
<br>
+ assert(!_discovery_is_mt, "Round robin should only be
used
<br>
in
<br>
serial discovery");
<br>
+ if (++_next_id >= _num_q) {
<br>
<br>
See the CR for an example log which showed _num_q changing
<br>
values between different phases of a collection and where
the
<br>
value of _next_id was greater than _num_q.
<br>
</blockquote>
I assume that the cause for the _next_id being greater than
_num_q
<br>
is that in a previous GC the number of threads has been
larger than
<br>
in the current.
<br>
</blockquote>
Yes. The log in the CR shows the case where _num_q was 23,
then 2
<br>
and then 23 again.
<br>
</blockquote>
Then I understood it correctly :)
<br>
<br>
<blockquote type="cite">
<blockquote type="cite">While this change seems to work, I
would kind of prefer that
<br>
_next_id is simply reset in some initialization. The greater
or
<br>
equal seems surprising here.
<br>
</blockquote>
Early on I started to look for a place where I could add the
<br>
initialization and decided not to. The _next_id is part of
the
<br>
reference processor and reference processor created and reused
so
<br>
there didn't seem to be a dependable place to put the
initialization.
<br>
One place that might work is to set it in the
<br>
set_active_mt_degree()
<br>
<br>
void set_active_mt_degree(uint v) { _num_q = v; }
<br>
<br>
Try it?
<br>
</blockquote>
I think this would be an appropriate location.
<br>
</blockquote>
<br>
As I said to Kim, I've implemented it and am testing it now.
First round
<br>
of testing on linux and G1 passsed but I want to test with the
other
<br>
GC's and on solaris sparcv9.
<br>
<br>
Thanks.
<br>
<br>
Jon
<br>
<br>
<blockquote type="cite">
<br>
As mentioned I would be okay with just a comment explaining the
<br>
situation like Kim, so if this is urgent and you are really
busy,
<br>
don't.
<br>
<br>
Thanks,
<br>
Thomas
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>