<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Bengt,<br>
<br>
Thanks for looking at the changes.<br>
<br>
On 3/12/2013 8:05 AM, Bengt Rutisson wrote:<br>
<blockquote cite="mid:513F4420.8050307@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi John,<br>
<br>
Thanks for fixing this so quickly!<br>
</div>
</blockquote>
<br>
The main change is based upon your patch. It just took a little time
to evaluate and disregard the alternatives (as well as fix the
underlying problems they discovered).<br>
<br>
<blockquote cite="mid:513F4420.8050307@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
I have only had a quick look at the change. I'll make sure to
look closer tomorrow. However, I have two questions. If you have
time to answer them it would be good. If you don't have time I
hope they become clear when I look more in detail at the change
tomorrow...<br>
<br>
First, in the constructor for
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
G1CMDrainMarkingStackClosure() we do:<br>
<br>
2285 _do_stealing = !_is_serial;<br>
2286 _do_termination = true;<br>
</div>
</blockquote>
<br>
Yes we can. I had thought of that but I thought people wouldn't like
it. <br>
<blockquote cite="mid:513F4420.8050307@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Just from looking at this it seems like we could get away with
only having one instance variable instead of three. Is that the
case or can any of _do_stealing, _is_serial or _do_termination
change during the life time of a G1CMDrainMarkingStackClosure
instance?<br>
<br>
Second, as you describe in the text below you are actually
fixing two issues with this patch. Would it make sense to split
the changes up into two changesets?<br>
</div>
</blockquote>
<br>
OK. That's probably a good idea as long as the second gets in fairly
soon - the Lucene tests will fail with an assertion failure when
parallel reference processing is enabled, if the overflows occur at
just the right points. I'll split them up into adding the serial
path to do_marking_step followed by the changes for the assertion
failure.<br>
<br>
New webrev soon.<br>
<br>
Thanks,<br>
<br>
JohnC<br>
<br>
</body>
</html>