<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Hi Evgeniya,<br>
<br>
On 10/30/14 3:05 PM, Evgeniya Stepanova wrote:<br>
</div>
<blockquote cite="mid:545245C5.4050504@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
Hi,<br>
<br>
Please review changes for 8062537, the OpenJDK/hotspot part of the
<a moz-do-not-send="true" id="key-val" rel="4684019"
href="https://bugs.openjdk.java.net/browse/JDK-8019361">JDK-8019361</a><br>
<br>
bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8062537">https://bugs.openjdk.java.net/browse/JDK-8062537</a><br>
fix: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eeistepan/8062537/webrev.00/">http://cr.openjdk.java.net/~eistepan/8062537/webrev.00/</a><br>
<br>
Problem: Some tests explicitly set GC and fail when jtreg set
another GC.<br>
Solution: Such tests marked with the jtreg tag "requires" to skip
test if there is a conflict<br>
</blockquote>
<br>
Thanks for fixing this! It is really great that we finally start
sorting this out.<br>
<br>
First a general comment. The @requires tag has been developed
without much cooperation with the GC team. We did have a lot of
feedback when it was first presented a year ago, but it does not
seem like this feedback was incorporated into the @requires that was
eventually built.<br>
<br>
I think this change that gets proposed now is a big step forward and
I won't object to it. But I am pretty convinced that we will soon
run in to the limitations of the current @requires implementation
and we will have to redo this work.<br>
<br>
Some of the points I don't really like about the @requires tag are:<br>
<br>
- the "vm.gc" abstraction is more limiting than helping. It would
have been better to just "require" any command line flag.<br>
- the requirement should be per @run tag. Right now we have to do
what you did in this change and use vm.gc=null even when some tests
could actually have been run when a GC was specified.<br>
- there are many tests that require more than just a specific GC.
Often there are other flags that can't be changed either for the
test to work properly.<br>
<br>
Maybe this is not the right place to discuss the current
implementation of the @requires tag. I just want to say that I'm not
too happy about how the @requires tag turned out. But assuming we
have to use it the way it is now I guess the proposed changeset
looks good.<br>
<br>
<blockquote cite="mid:545245C5.4050504@oracle.com" type="cite">
Tested locally with different GC flags (-XX:+UseG1GC,
-XX:+UseParallelGC, -XX:+UseSerialGC, -XX:+UseConcMarkSweep and
without any GC flag). Tests are being excluded as expected. No
tests failed because of the conflict.<br>
</blockquote>
Have you tested with -Xconcgc too? It's an alias for
-XX:+UseConcMarkSweepGC.<br>
<br>
I think some of the test, like
test/gc/startup_warnings/TestDefNewCMS.java, will fail if you run
with -XX:+UseParNewGC. Others, like
test/gc/startup_warnings/TestParNewCMS.java, will fail if you run
with -XX:-UseParNewGC. Could you test these two cases too?<br>
<br>
Similarly it looks to me like there are tests that will fail if you
run them with
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
-XX:-UseParallelOldGC or -XX:+UseParallelOldGC.<br>
<br>
Just a heads up. These two tests will soon be removed. I'm about to
push a changeset that removes them:<br>
<br>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
test/gc/startup_warnings/TestCMSIncrementalMode.java<br>
test/gc/startup_warnings/TestCMSNoIncrementalMode.java<br>
<br>
Is there some way of making sure that all tests are run at one time
or another. With this change there is a risk that some tests are
never run and always skipped. Will we somehow be tracking what gets
skipped and make sure that all tests are at least run once with the
correct GC so that it is not skipped all the time?<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:545245C5.4050504@oracle.com" type="cite"> <br>
Thanks,<br>
Evgeniya Stepanova
<div class="moz-signature"><br>
</div>
</blockquote>
<br>
</body>
</html>