<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">[RFR back from the dead...]<br>
<br>
I think last webrev for this was updated to follow the consensus
opinion, but it never received thumbs up/down from reviewers.<br>
<br>
I've created a new webrev for critical copyright notice updates.<br>
<br>
Summary:<br>
1) Updated the "needs_g1gc" list in TEST.groups to prevent G1
tests running in embedded.<br>
2) Added appropriate "@requires vm.gc" annotations to a few GC
tests that use the process builder and pass in a collector
explicitly. The new @requires annotations prevent unexpected
GC-specific tests running when a conflicting collector argument is
specified by the outer test harness.<br>
<br>
CR:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8078673">https://bugs.openjdk.java.net/browse/JDK-8078673</a>
<br>
<br>
Webrev: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/8078673/webrev.04/">http://cr.openjdk.java.net/~drwhite/8078673/webrev.04/</a>
<br>
<br>
Testing: <br>
JPRT <br>
<br>
Thanks!<br>
- Derek<br>
<br>
On 7/13/15 5:37 PM, Derek White wrote:<br>
</div>
<blockquote cite="mid:55A42FB0.5050909@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 7/12/15 10:38 PM, David Holmes
wrote:<br>
</div>
<blockquote cite="mid:55A324B5.106@oracle.com" type="cite">Hi
Derek, <br>
<br>
On 11/07/2015 1:28 AM, Derek White wrote: <br>
<blockquote type="cite">Please review this partial fix for GC
tests that require certain <br>
collectors (e.g. shouldn't run in embedded). <br>
<br>
This is an updated webrev to account for Leonid's fix for
"8079134: <br>
[TESTBUG] Remove applicable_*gc and needs_*gc groups from
TEST.groups", <br>
which removed a pile of TEST.groups lists including needs_gc,
<br>
needs_serialgc, needs_parallelgc, and needs_cmsgc. <br>
<br>
Now the fix simply updates the needs_g1gc list in TEST.groups
and adds <br>
appropriate "@requires vm.gc" annotations to a few GC tests. <br>
<br>
Note that the "@requires vm.gc" changes are /almost/ purely
documentary. <br>
These are ProcessBuilder-based tests, so any "-XX:+UsexxxGC"
flags <br>
passed in by jtreg are ignored. It's very confusing, as well
as <br>
unnecessary, for a jtreg run specifying -XX:+UseG1GC to be
running a <br>
test that then replaces the flag with "-XX:+UseParallelGC"
(etc). <br>
</blockquote>
<br>
So even though we would never pass through the jtreg specific GC
option we will skip these tests if that option doesn't match
with the GC's the test will be testing. As long as that doesn't
lead to these tests being untested that seems okay. <br>
</blockquote>
In all of the tests I modified, it always runs the test if jtreg
did not specify any GC. Most of the tests will also run if jtreg
specifies the same GC as the test. So we should have test
coverage.<br>
<blockquote cite="mid:55A324B5.106@oracle.com" type="cite"> <br>
But really this highlights a basic problem we have with our
approach to testing with different VM options. Unless the GC
option is the only option that changes across the runs you would
want the other options to be passed through to the actual tests
in many cases. :( <br>
</blockquote>
<br>
Some test do pass along the outer (jtreg) arguments, and some
don't. So the fix with "@requires vm.gc" removes the possibility
of conflicting GC's being specified in that case. There could
still be conflicts between other arguments specified by jtreg vs.
the ProcessBuilder. If the ProcessBuilder arguments were constant,
they could also be specified by
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
<code>"@requires vm.opt." annotations, but this leaves out many
cases.<br>
<br>
I'm also not satisfied by the current state of affairs, but I
think this fix gets us slightly closer to what we want.<br>
<br>
- Derek<br>
<br>
</code>
<blockquote cite="mid:55A324B5.106@oracle.com" type="cite"><br>
<blockquote type="cite">*CR:* <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8078673">https://bugs.openjdk.java.net/browse/JDK-8078673</a>
<br>
<br>
*Webrev:* <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/8078673/webrev.03/">http://cr.openjdk.java.net/~drwhite/8078673/webrev.03/</a>
<br>
<br>
*Testing:* <br>
JPRT <br>
<br>
*Open review comments:* <br>
David H: Your last comments on this subject requested changes
to the <br>
"needs_gc" list, which has been removed by 8079134. <br>
<br>
Kim and Jesper: I agree with your comments about wanting some
better for <br>
both testing multiple GCs and dealing with SE Embedded
properly in the <br>
testing infrastructure. This webrev is simply a good fix
within the <br>
existing infrastructure. <br>
<br>
Thanks, <br>
- Derek <br>
<br>
On 4/29/15 5:03 PM, Kim Barrett wrote: <br>
<blockquote type="cite">On Apr 29, 2015, at 4:38 PM, Jesper
Wilhelmsson<a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:jesper.wilhelmsson@oracle.com"><jesper.wilhelmsson@oracle.com></a>
wrote: <br>
<blockquote type="cite">Hi, <br>
<br>
There are tests like
hotspot/test/gc/g1/TestShrinkAuxiliaryData**.java where
there is a base class that provides the test and a bunch
of test classes that only starts the base test with
different arguments. This case would be similar but
slightly more ugly since the actual code would be the same
and trigger the same base test, but with different
@requires in the comment above the test. <br>
<br>
I'm not sure it would help though. What we really would
need here is a @requires that could check the host name or
the hardware platform or OS. <br>
</blockquote>
@requires has os.{name, family, arch, simpleArch} properties
that can be tested. But I’m not sure any of those are
really right for testing for “embedded system” (whatever
that actually means). <br>
<br>
<blockquote type="cite">Kim Barrett skrev den 29/4/15 20:35:
<br>
<blockquote type="cite">On Apr 29, 2015, at 2:06 PM, Derek
White<a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:derek.white@oracle.com"><derek.white@oracle.com></a>
wrote: <br>
<blockquote type="cite">So most of these tests use
ProcessBuilder to specify a command line, and
explicitly mention a GC to use. A single test might
actually run each collector (gc/logging/TestGCId, for
example). Does @requires matter in this case? <br>
<br>
Oh, maybe they should all have @requires
vm.gc=="null", becuase the actual test is ignoring GC
passed in by the test harness GC and running as a
separate process anyway. It's misleading if the
harness said UseG1GC and the test was actually running
UseParallelGC, for example. <br>
</blockquote>
That’s one solution. A different solution would be to
clone into multiple tests, one for each relevant
collector, where the vm.gc can be “null” or the
corresponding collector. That cloning is kind of ugly
though; it’s too bad there can only be one @requires
constraint per test, rather than per @run line or the
like. But we didn’t get any traction with such a
suggestion:<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/CODETOOLS-7901090">https://bugs.openjdk.java.net/browse/CODETOOLS-7901090</a>.
<br>
<br>
<blockquote type="cite">FYI, it sounds like my original
problem does require the exclude lists to keep the
embedded JVM from running GC tests that it shouldn't.
<br>
</blockquote>
I’m not sure how to address this problem. For example,
we don’t want to exclude TestSmallHeap.java on embedded
JVMs, we just want to exclude its sub-cases that would
attempt to use a gc that isn’t supported. <br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>