<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-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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ecaspole/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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ecaspole/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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ecaspole/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>
</body>
</html>