<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ecaspole/JDK-8077265/02/webrev/">http://cr.openjdk.java.net/~ecaspole/JDK-8077265/02/webrev/</a><br>
<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>
</body>
</html>