<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Hi John,<br>
<br>
This looks good to me.<br>
<br>
I think Vitaly has a point about the fact that we just "know" that
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
_parallel_workers == NULL is equivalent to
parallel_marking_threads() == 0.<br>
<br>
I'm not going to insist on this, but would it make sense to add an
assert to convey this information? I'm actually less worried about
the case "parallel_marking_threads() > 0 but _parallel_workers
== NULL" since that will result in a null de-reference that can be
fairly easy to debug.<br>
<br>
But maybe this assert could be added at the start of the method:<br>
<br>
assert(_parallel_workers == NULL || parallel_marking_threads()
> 0, "work gang not set up correctly"); <br>
<br>
I'm not sure we need that assert and I'm not convinced this is the
right place to have it. But my thought is that this will detect an
unexpected state that we would otherwise silently ignore.<br>
<br>
Bengt<br>
<br>
<br>
This is probably not such a big problem if we <br>
<br>
On 1/10/13 1:47 AM, Vitaly Davidovich wrote:<br>
</div>
<blockquote
cite="mid:CAHjP37Hn=68a-xR439TKyC4LwSeaQ3WYSjE8uj+0adZ5+_KmOw@mail.gmail.com"
type="cite">
<p dir="ltr">Hi John,</p>
<p dir="ltr">Thanks for the response. Yeah, I figured it's the
same thing since it's not null iff # of workers > 0.
However, if this relationship is ever broken or perhaps the gang
can be set to null at some point even if workers > 0, then
this code will segv again. Hence I thought a null guard is a
bit better, but it was just a side comment - code looks fine as
is.</p>
<p dir="ltr">Thanks</p>
<p dir="ltr">Sent from my phone</p>
<div class="gmail_quote">On Jan 9, 2013 7:41 PM, "John
Cuthbertson" <<a moz-do-not-send="true"
href="mailto:john.cuthbertson@oracle.com">john.cuthbertson@oracle.com</a>>
wrote:<br type="attribution">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"> Hi Vitaly,<br>
<br>
Thanks for looking over the changes. AFAICT checking if
_parallel_workers is not null is equivalent to checking that
the number of parallel marking threads is > 0. I went
with the latter check as other references to the parallel
workers work gang are guarded by it. I'm not sure why the
code was originally written that way but my guess is that,
when originally written, the marking threads (like the
concurrent refinement threads currently) were not in a work
gang.<br>
<br>
Thanks,<br>
<br>
JohnC<br>
<br>
<div>On 1/8/2013 8:37 PM, Vitaly Davidovich wrote:<br>
</div>
<blockquote type="cite">
<p dir="ltr">Hi John,</p>
<p dir="ltr">What's the advantage of checking parallel
marking thread count > 0 rather than checking if
parallel workers is not NULL? Is it clearer that way?
I'm thinking checking for NULL here (perhaps with a
comment on when NULL can happen) may be a bit more
robust in case it can be null for some other reason,
even if parallel marking thread count is > 0.</p>
<p dir="ltr">Looks good though.</p>
<p dir="ltr">Thanks</p>
<p dir="ltr">Sent from my phone</p>
<div class="gmail_quote">On Jan 8, 2013 5:14 PM, "John
Cuthbertson" <<a moz-do-not-send="true"
href="mailto:john.cuthbertson@oracle.com"
target="_blank">john.cuthbertson@oracle.com</a>>
wrote:<br type="attribution">
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"> Hi
Everyone,<br>
<br>
Can I please have a couple of volunteers look over the
fix for this CR - the webrev can be found at: <a
moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ejohnc/8005875/webrev.0/"
target="_blank">http://cr.openjdk.java.net/~johnc/8005875/webrev.0/</a><br>
<br>
Summary:<br>
One of the modules in the Kitchensink test generates a
VM_PrintThreads vm operation. The JVM crashes when it
tries to print out G1's concurrent marking worker
threads when ParallelGCThreads=0 because the work gang
has not been created. The fix is to add the same check
that's used elsewhere in G1's concurrent marking.<br>
<br>
Testing:<br>
Kitchensink with ParallelGCThreads=0<br>
<br>
Thanks,<br>
<br>
JohnC<br>
</blockquote>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
<br>
</body>
</html>