<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">OK, I'll remove it.<br>
<br>
Thanks,<br>
Andreas<br>
<br>
On 2014-05-22 10:02, Bengt Rutisson wrote:<br>
</div>
<blockquote cite="mid:537DAEFC.2000508@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
<br>
Hi Andreas,<br>
<br>
On 5/21/14 2:05 PM, Andreas Eriksson wrote:<br>
</div>
<blockquote cite="mid:537C9695.5010902@oracle.com" type="cite">A
new webrev is up: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eaeriksso/8038925/webrev.02/">http://cr.openjdk.java.net/~aeriksso/8038925/webrev.02/</a>
<br>
<br>
The test now verifies that no full GC has been done after doing
the heap dump. <br>
I also modified the test to not run if it for some reason is not
using G1. <br>
</blockquote>
<br>
Thanks for the update, Andreas.<br>
<br>
The test looks good except for the @run tag.<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
@run main/othervm -Xms512m -Xmx1024m -XX:+UseG1GC
-XX:+ExplicitGCInvokesConcurrent TestG1ConcurrentGCHeapDump<br>
<br>
The problem is that more GC flags will be added when the test is
run in nightly testing. Some GC flags will conflict with UseG1GC.
On the other hand at some point UseG1GC will be one of the flags
that is added.<br>
<br>
So, I think what you need to do is just remove "-XX:+UseG1GC" from
the @run tag. Then your test will report log "skipped" when it is
run in the nightly testing except for the nightly testing done
with G1 where it will actually do something.<br>
<br>
Bengt<br>
<br>
<blockquote cite="mid:537C9695.5010902@oracle.com" type="cite"> <br>
Regards, <br>
Andreas <br>
<br>
On 2014-05-21 12:31, Bengt Rutisson wrote: <br>
<blockquote type="cite">On 5/21/14 12:12 PM, Andreas Eriksson
wrote: <br>
<blockquote type="cite">Hi Bengt, thanks for looking at this.
<br>
<br>
I agree that verifying that no full GC has happened would be
good. <br>
One thing I see as problematic though, is what to do if a
full GC has happened. <br>
Should I make the test fail? Or is there some way to signal
that the test was inconclusive / couldn't finish? <br>
</blockquote>
<br>
Personally I would prefer to make the test fail. JTreg only
has two states, PASS or FAIL, and I think that if we allow it
to pass we might not notice if there is some change that makes
the test always get a full GC and then never actually testing
anything. <br>
<br>
I don't think it will fail intermittently by getting full GCs.
I think the test is pretty stable. But I think it would be
good to have a way of noticing if it stops testing what it is
supposed to test. <br>
<br>
(By the way, I would really like a "SKIPPED" state in JTreg
but I haven't had any luck pushing for that. I think it could
be useful for other things as well. There is for example no
good reason for your test to be run 4 times each night with
the exact same binary. But that is what happens since we can't
say that we should skip this test if we run with any other GC
than G1.) <br>
<br>
Thanks, <br>
Bengt <br>
<br>
<blockquote type="cite"> <br>
Regards, <br>
Andreas <br>
<br>
On 2014-05-21 11:55, Bengt Rutisson wrote: <br>
<blockquote type="cite"> <br>
Hi Andreas, <br>
<br>
The fix looks good. <br>
<br>
One comment about the test. It does not verify that no
full GC happens. The way the test is set up I guess that
should not happen and I am not sure it is worth the effort
to add a check for it. Just wanted to mention it if you
want to make test more resilient to future changes in the
JVM that for some reason can trigger a full GC for this
test. <br>
<br>
I'm fine with leaving the test as it is. <br>
<br>
Thanks, <br>
Bengt <br>
<br>
<br>
On 5/20/14 4:26 PM, Andreas Eriksson wrote: <br>
<blockquote type="cite">Hi, <br>
<br>
Could I have a review for this G1 jdk7 only fix please?
<br>
(There is a related problem in jdk8 and jdk9, where an
assert can fail because of this problem. I have logged
another bug for this.) <br>
<br>
Description: <br>
Due to the marking cleanup reclaiming empty regions, and
having stale references a crash can occur when doing a
heap dump. <br>
The code tries to do an is_klass check on the object,
which crashes the VM. <br>
The fix is to add an is_perm check before doing the
check, since is_perm will do a bounds check on the oop
and if it's in the perm gen we know it's safe to look at
it since G1 only ever does full compactions of the perm
gen. <br>
<br>
For more information, and a more in-depth analysis,
please see the jira bug. <br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eaeriksso/8038925/webrev.01/">http://cr.openjdk.java.net/~aeriksso/8038925/webrev.01/</a>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8038925">https://bugs.openjdk.java.net/browse/JDK-8038925</a>
<br>
<br>
Regards, <br>
Andreas <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>