<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 2015-04-14 17:30, Eric Caspole
wrote:<br>
</div>
<blockquote cite="mid:552D32A9.3050901@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
Hi, here's a new webrev with the new assert in psOldGen as a
function -<br>
I capture only the covered_region in a local as it directly
relates to the problem.<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eecaspole/JDK-8077265/02/webrev/">http://cr.openjdk.java.net/~ecaspole/JDK-8077265/02/webrev/</a><br>
</blockquote>
<br>
Looks good.<br>
<br>
StefanK<br>
<br>
<blockquote cite="mid:552D32A9.3050901@oracle.com" type="cite"> <br>
Passes JPRT.<br>
Thanks,<br>
Eric<br>
<br>
<br>
<div class="moz-cite-prefix">On 4/13/2015 4:39 PM, Stefan Karlsson
wrote:<br>
</div>
<blockquote cite="mid:552C297F.6050003@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 2015-04-13 17:52, Eric Caspole
wrote:<br>
</div>
<blockquote cite="mid:552BE63B.5010705@oracle.com" type="cite">Thinking
about it over the weekend, I really do want some new asserts
in psOldGen because it should catch the problem in a
different/interesting way. <br>
I am happy to back out the changes in psPromotionLAB.cpp and
hpp, since that part already catches the problem. <br>
<br>
Stefan, do you want to this change to use the macro in
psOldGen as I was doing before or refactor into a new
function? <br>
</blockquote>
<br>
I think I'd prefer a new function to get rid of the setup of the
local __variable_name__ variables. We would lose the line number
of the call site, if we replace the macro with a function, but
the stack trace in the hs_err file tell us where we came from,
so I think that's OK.<br>
<br>
Thanks,<br>
StefanK<br>
<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
<blockquote cite="mid:552BE63B.5010705@oracle.com" type="cite">Thanks,
<br>
Eric <br>
<br>
<br>
On 4/10/2015 9:19 AM, Stefan Karlsson wrote: <br>
<blockquote type="cite">On 2015-04-10 15:15, Eric Caspole
wrote: <br>
<blockquote type="cite">I don't want to make sweeping
changes to try to debug what seems like a race. <br>
If you dont like the macro in psOldGen I will remove it
and just go on with the original one. <br>
</blockquote>
<br>
Sounds good to me. <br>
<br>
StefanK <br>
<br>
<blockquote type="cite">Eric <br>
<br>
<br>
On 4/10/2015 4:43 AM, Stefan Karlsson wrote: <br>
<blockquote type="cite">On 2015-04-09 22:17, Eric Caspole
wrote: <br>
<blockquote type="cite">Hi everybody, <br>
I updated this so the psOldGen part use a macro as
Stefan suggested. <br>
The assert in psPromotionLAB.hpp is allocating out of
an already allocated PLAB, so I don't think that one
will ever be hit but I want it there just in case. <br>
And as Jesper suggested I made the message more
helpful in the original place. <br>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eecaspole/JDK-8077265/01/webrev/">http://cr.openjdk.java.net/~ecaspole/JDK-8077265/01/webrev/</a>
<br>
</blockquote>
<br>
I had hoped you would use the new define for all assert,
but you've only used it for two out of four. If you're
not going to use the same macro for all usages, it
doesn't seem worth having. <br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eecaspole/JDK-8077265/01/webrev/src/share/vm/gc_implementation/parallelScavenge/psOldGen.hpp.udiff.html">http://cr.openjdk.java.net/~ecaspole/JDK-8077265/01/webrev/src/share/vm/gc_implementation/parallelScavenge/psOldGen.hpp.udiff.html</a>
<br>
<br>
It seems to me that we could refactor allocate_noexpand
and cas_allocate_noexpand to use a common function to do
the assert and update the start array. That way you
could get rid of the macro you added. <br>
<br>
Thanks, <br>
StefanK <br>
<br>
<blockquote type="cite"> <br>
Passes JPRT. <br>
Thanks, <br>
Eric <br>
<br>
On 4/9/2015 10:01 AM, Stefan Karlsson wrote: <br>
<blockquote type="cite">Hi Eric, <br>
<br>
On 2015-04-09 15:19, Eric Caspole wrote: <br>
<blockquote type="cite">HI everybody, <br>
Here is a webrev to add more asserts related to
debugging JDK-8068448. Beyond capturing more info
in the original assert, after looking at another
core I added more asserts to make sure there is no
other place where old gen allocations would
overrun the start array. <br>
</blockquote>
<br>
Why didn't these two new asserts get the same, more
informative, error message as the first assert you
changed? Maybe you could extract the check out to a
helper macro that prints the relevant information? <br>
<br>
Another point that Bengt mentioned yesterday, is
that we don't really need to print the old_gen part
of the assert. It's already printed in the hs_err
file. <br>
<br>
Thanks, <br>
StefanK <br>
<br>
<blockquote type="cite"> <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eecaspole/JDK-8077265/00/webrev/">http://cr.openjdk.java.net/~ecaspole/JDK-8077265/00/webrev/</a>
<br>
<br>
Passes JPRT. <br>
Thanks, <br>
Eric <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>