<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Bengt,<br>
<br>
Please see comments inline<br>
<div class="moz-cite-prefix">On 12.11.2014 19:43, Bengt Rutisson
wrote:<br>
</div>
<blockquote cite="mid:54638012.3010202@oracle.com" type="cite">
<meta http-equiv="Context-Type" content="text/html; charset=utf-8">
<br>
<div class="moz-cite-prefix">On 2014-11-12 16:21, Evgeniya
Stepanova wrote:<br>
</div>
<blockquote cite="mid:54637AF5.9080600@oracle.com" type="cite"> Hi
everyone!<br>
<br>
Since the decision was made to change only tests which fail
because of conflict for now (skip "selfish" tests), I post new
webrev for jdk 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>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eavstepan/eistepan/8062536/webrev.03/">http://cr.openjdk.java.net/~avstepan/eistepan/8062536/webrev.03/</a><br>
</blockquote>
<br>
Thanks for updating the webrev!<br>
<br>
A couple of comments:<br>
<br>
MemoryTestAllGC.sh<br>
<br>
The test is run three times, once with no params, once with
ParallelGC and once with CMS. So, I think the @requires should
just be vm.gc == "null". Similarly to what was done for
PendingAllGC.sh.<br>
<br>
</blockquote>
The third run (with CMS) is commented. Without this run
UseParallelGC is valid option<br>
#runOne -XX:+UseConcMarkSweepGC MemoryTest 3<br>
(<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~avstepan/eistepan/8062536/webrev.03/test/java/lang/management/MemoryMXBean/MemoryTestAllGC.sh.frames.html">http://cr.openjdk.java.net/~avstepan/eistepan/8062536/webrev.03/test/java/lang/management/MemoryMXBean/MemoryTestAllGC.sh.frames.html</a>)<br>
<blockquote cite="mid:54638012.3010202@oracle.com" type="cite"> <br>
TestInputArgument.sh<br>
<br>
The changes here seem unrelated to @requires.<br>
<br>
</blockquote>
This test was changed after conversation with David Holmes (see
thread below)<br>
<blockquote cite="mid:54638012.3010202@oracle.com" type="cite"> <br>
EnqueuePollRace.java<br>
<br>
Can you explain why it is safe to remove -XX:+UseSerialGC for this
test?<br>
<br>
<br>
</blockquote>
This test was modified after conversation with Filipp Zhinkin and
Mandy Chung (<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8051723">https://bugs.openjdk.java.net/browse/JDK-8051723</a>)<br>
<blockquote cite="mid:54638012.3010202@oracle.com" type="cite">
JpsHelper.java<br>
<br>
Can you explain why it is safe to remvoe -XX:+UseParallelGC for
this test?<br>
<br>
</blockquote>
This test was changed after conversation with Katja Kantserova (see
thread below), GC flag is just an arbitrary chosen test flag
<blockquote cite="mid:54638012.3010202@oracle.com" type="cite"> <br>
When I use Aurora to check what tests that currently are
considered known because of JDK-8019361 I get a pretty long list:<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://aurora.ru.oracle.com/functional/faces/CRRules.xhtml?cr=JDK-8019361">http://aurora.ru.oracle.com/functional/faces/CRRules.xhtml?cr=JDK-8019361</a><br>
<br>
Are the tests in webrev.03 the only tests that still fail? Have
the others been fixed in other ways?<br>
</blockquote>
There would be 2 more changes in reviews in closed part :)<br>
<br>
Thanks,<br>
Evgeniya Stepanova<br>
<blockquote cite="mid:54638012.3010202@oracle.com" type="cite"> <br>
Thanks,<br>
Bengt<br>
<br>
<br>
<br>
<blockquote cite="mid:54637AF5.9080600@oracle.com" type="cite"> <br>
Thanks,<br>
Evgeniya Stepanova
<div class="moz-cite-prefix">On 07.11.2014 15:34, Evgeniya
Stepanova wrote:<br>
</div>
<blockquote cite="mid:545CAE30.9010708@oracle.com" type="cite">
David, Filipp, Katja<br>
<br>
Diff have been updated one more time:<br>
java/lang/management/RuntimeMXBean/TestInputArgument.sh and
test/java/lang/ref/EnqueuePollRace.java have been changed<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eeistepan/8062536/webrev.02/">http://cr.openjdk.java.net/~eistepan/8062536/webrev.02/</a><br>
<br>
Thanks<br>
<br>
<div class="moz-cite-prefix">On 07.11.2014 9:37, David Holmes
wrote:<br>
</div>
<blockquote cite="mid:545C5AB6.6030103@oracle.com" type="cite">On
7/11/2014 12:36 AM, Evgeniya Stepanova wrote: <br>
<blockquote type="cite">New webrev: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eeistepan/8062536/webrev.01/">http://cr.openjdk.java.net/~eistepan/8062536/webrev.01/</a>
<br>
</blockquote>
<br>
In: <br>
<br>
test/java/lang/management/RuntimeMXBean/TestInputArgument.sh
<br>
<br>
the use of the gc options seems incidental - it's just
picking two innocuous options to use - similar to the
JpsHelper case. You could replace +UseParallelGC with
something like +UseFastJNIAccessors (platform independent
and normally true). <br>
<br>
David <br>
----- <br>
<br>
<blockquote type="cite">Thanks, <br>
Evgeniya Stepanova <br>
On 06.11.2014 17:35, Yekaterina Kantserova wrote: <br>
<blockquote type="cite">Thanks a lot! <br>
<br>
On 11/06/2014 02:05 PM, Evgeniya Stepanova wrote: <br>
<blockquote type="cite">Hi Katja, <br>
<br>
Ok, this seems to be a perfect solution. Thank you.
I'll change the <br>
diff accordingly. <br>
<br>
<br>
Thanks, <br>
Evgeniya Stepanova <br>
On 06.11.2014 16:56, Yekaterina Kantserova wrote: <br>
<blockquote type="cite">Hi Dima, <br>
<br>
On 11/06/2014 11:22 AM, Dmitry Fazunenko wrote: <br>
<blockquote type="cite">Hi Katja, <br>
<br>
You are right, there will be no conflict, because
test ignores any <br>
external VM flags. <br>
So, adding @requires seems unnecessary here,
but... <br>
<br>
Ignoring external options is bad thing, such
"selfish" tests are <br>
not applicable for other areas, like GC, compiler,
RT. <br>
</blockquote>
<br>
This particular case is to test the defined flags
are shown up as <br>
expected. <br>
<br>
Evgeniya, <br>
<br>
would you mind to change JpsHelper.java instead? <br>
<br>
+++ b/test/sun/tools/jps/JpsHelper.java <br>
@@ -93,7 +93,7 @@ <br>
/** <br>
* VM arguments to start test application with
<br>
*/ <br>
- public static final String[] VM_ARGS =
{"-Xmx512m", <br>
"-XX:+UseParallelGC"}; <br>
+ public static final String[] VM_ARGS =
{"-Xmx512m", <br>
"-XX:+PrintGCDetails"}; <br>
/** <br>
* VM flag to start test application with <br>
*/ <br>
<br>
Best regards, <br>
Katja <br>
<br>
<br>
<br>
<blockquote type="cite"> <br>
@requires will allow to modify tests to include
external vm <br>
options without any risk of bumping into conflict
and extend area <br>
of test applicability. <br>
<br>
But if you still believe, that @requires is not
necessary - it's <br>
not a problem, tests could be kept as is. <br>
<br>
Thanks, <br>
Dima <br>
<br>
<br>
On 06.11.2014 16:27, Yekaterina Kantserova wrote:
<br>
<blockquote type="cite"> <br>
Hi Evgeniya, <br>
<br>
As David has pointed out these jps tests are not
testing gc. The <br>
-XX:+UseParallelGC is just an arbitrary chosen
test flag. There <br>
should not be any conflicts either since these
tests are running <br>
in driver mode: <br>
<br>
... <br>
* @run driver TestJpsJar <br>
... <br>
<br>
which means no flags from above are accepted. <br>
<br>
Thanks, <br>
Katja <br>
<br>
<br>
<br>
On 11/06/2014 11:05 AM, Evgeniya Stepanova
wrote: <br>
<blockquote type="cite">Hi David, <br>
<br>
tag added because tests contain string <br>
cmd.addAll(JpsHelper.getVmArgs()); <br>
<br>
and JpsHelper defines <br>
... <br>
public static final String[] VM_ARGS =
{"-Xmx512m", <br>
"-XX:+UseParallelGC"}; <br>
... <br>
public static List<String> getVmArgs()
throws IOException { <br>
if (testVmArgs == null) { <br>
testVmArgs = new
ArrayList<>(); <br>
testVmArgs.addAll(Arrays.asList(VM_ARGS)); <br>
testVmArgs.add("-XX:Flags=" + <br>
getVmFlagsFile().getAbsolutePath()); <br>
} <br>
return testVmArgs; <br>
} <br>
<br>
Tests itself wouldn't fail if we use another
GC, tag added for <br>
cleanup-if we use for example SerialGC we must
be sure that tests <br>
passed with this GC, not with another one. Now
you will assume <br>
that concrete test passed with Serial GC, but
it run only with <br>
Parallel GC. Plus there is no any sense to run
test twice in TC <br>
(with different GC, since it use only
Parallel) <br>
<br>
Thanks, <br>
Evgeniya Stepanova <br>
On 06.11.2014 6:20, David Holmes wrote: <br>
<blockquote type="cite">Hi Evgeniya, <br>
<br>
On 6/11/2014 1:33 AM, Evgeniya Stepanova
wrote: <br>
<blockquote type="cite">Hi, <br>
<br>
Please review changes for 8062536, the
OpenJDK/jdk part of the <br>
JDK-8019361 <br>
<br>
bug: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8062536">https://bugs.openjdk.java.net/browse/JDK-8062536</a>
<br>
fix: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eeistepan/8062536/webrev.00/">http://cr.openjdk.java.net/~eistepan/8062536/webrev.00/</a>
<br>
<br>
Problem: Some tests explicitly set GC and
fail when another GC <br>
is set <br>
outside <br>
</blockquote>
<br>
I don't see why you have done this for the <br>
<br>
test/sun/tools/jps/TestJps*.java <br>
<br>
tests. They don't set any GC flags. <br>
<br>
<blockquote type="cite">Solution: Such tests
marked with the jtreg tag "requires" to <br>
skip test <br>
if there is a conflict <br>
</blockquote>
<br>
Just wondering: Does a skipped test get a
.jtr file showing it <br>
was skipped; or does it only appear in the
higher-level jtreg log? <br>
<br>
Thanks, <br>
David <br>
<br>
<blockquote type="cite">Tested locally with
different GC flags (-XX:+UseG1GC, <br>
-XX:+UseParallelGC, -XX:+UseSerialGC,
-XX:+UseConcMarkSweep and <br>
without <br>
any GC flag). Tests are being excluded as
expected. No tests <br>
failed <br>
because of the conflict. <br>
<br>
Thanks, <br>
Evgeniya Stepanova <br>
<br>
// <br>
</blockquote>
</blockquote>
<br>
-- <br>
/Evgeniya Stepanova/ <br>
</blockquote>
<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
-- <br>
/Evgeniya Stepanova/ <br>
</blockquote>
<br>
</blockquote>
<br>
-- <br>
/Evgeniya Stepanova/ <br>
</blockquote>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<i>Evgeniya Stepanova</i></div>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<i>Evgeniya Stepanova</i></div>
</blockquote>
<br>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<i>Evgeniya Stepanova</i></div>
</body>
</html>