<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 05/29/2013 12:46 AM, John
Cuthbertson wrote:<br>
</div>
<blockquote cite="mid:51A533AD.1090706@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
Hi Stefan,<br>
<br>
Replies inline....<br>
<br>
<div class="moz-cite-prefix">On 5/27/2013 12:16 PM, Stefan
Karlsson wrote:<br>
</div>
<blockquote cite="mid:51A3B110.6050502@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 2013-05-25 00:19, John
Cuthbertson wrote:<br>
</div>
<blockquote cite="mid:519FE787.70408@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
Hi Everyone,<br>
<br>
Can I have a couple of reviewers look over these changes - the
webrev is: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejohnc/8015237/webrev.0/">http://cr.openjdk.java.net/~johnc/8015237/webrev.0/</a><br>
</blockquote>
<br>
Not a complete review, yet. But I have a couple of comments from
browsing through the patch.<br>
<br>
There's a lot of places where you have add an extra worker_id
parameter. It's only used for one out-commented print statement
and a very toothless assert. If you remove that, the patch will
shrink from ten files changed to three files changed and we'll
keep the process_strong_roots functions from getting more
parameters.<br>
</blockquote>
<br>
I added the extra parameter because I've added it 3 times to
different changes for PSR runs and my own tracing to verify the
distribution of work among the threads. If I have to add it again
for another change - it stays then OK?<br>
</blockquote>
<br>
I think you should maintain your own patch for this. No need to
litter the code base with this if its not really needed.<br>
<br>
<blockquote cite="mid:51A533AD.1090706@oracle.com" type="cite"> <br>
<blockquote cite="mid:51A3B110.6050502@oracle.com" type="cite"> <br>
Have you also consider to parallelize the StringTable::unlink
function? <br>
</blockquote>
<br>
I didn't. I was focused on the G1 data generated by my
instrumented runs from PSR - which only indicated that the
scanning was an issue. Parallelization of the unlink could be
achieved in a similar way but we would need to wrap the unlink
calls in suitable work gang tasks for the frame work collectors or
ParallelScavenge/ParallelCompact GC tasks for parallel GC. I think
that should be a separate CR.<br>
</blockquote>
<br>
OK. Yes, I agree that it should not be done as a part of this CR.<br>
<br>
thanks,<br>
StefanK<br>
<br>
<blockquote cite="mid:51A533AD.1090706@oracle.com" type="cite"> <br>
JohnC<br>
<br>
</blockquote>
<br>
</body>
</html>