<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Igor,<br>
<br>
Thanks a lot for the review!<br>
<br>
<div class="moz-cite-prefix">On 20.06.2016 23:35, Igor Ignatyev
wrote:<br>
</div>
<blockquote
cite="mid:AA449D79-35B4-433F-BC5B-24A9F8714EC3@oracle.com"
type="cite">
<pre wrap="">Dima,
the fix looks good to me, thanks for implementing that. I have a question about -Xbootclasspath/a:bootClasses, is it required? I thought classes from requires.extraPropDefns.bootlibs will be added to BCP by jtreg.</pre>
</blockquote>
<br>
Yes, it's required. We might decided for example to use
-Xbootclasspath/<b>p</b>:
instead.<br>
<pre wrap="">bootClasses - is the name of the local folder (under scratch dir) where
classes from the <span class="changed">'requires.extraPropDefns.bootlibs' will be compiled to.
And I haven't mentioned, that I introduced a field, which is currently unused:
private static final WhiteBox WB = WhiteBox.getWhiteBox();
This is for the simplification of use in the future.
Thanks,
Dima
</span></pre>
<blockquote
cite="mid:AA449D79-35B4-433F-BC5B-24A9F8714EC3@oracle.com"
type="cite">
<pre wrap="">
Thanks,
— Igor
</pre>
<blockquote type="cite">
<pre wrap="">On Jun 20, 2016, at 6:12 PM, Dmitry Fazunenko <a class="moz-txt-link-rfc2396E" href="mailto:dmitry.fazunenko@oracle.com"><dmitry.fazunenko@oracle.com></a> wrote:
Hello,
During the review period of this CR a couple of changes adding new requires properties were integrated:
8157831: vm.simpleArch
8158412: vm.flightRecorder
Merged variant:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dfazunen/8151283/webrev.02/">http://cr.openjdk.java.net/~dfazunen/8151283/webrev.02/</a>
Thanks,
Dima
On 16.06.2016 16:34, Dmitry Fazunenko wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Michail,
This is TEST.ROOT file, which defines itself what is the root for libraries:
48 # Path to libraries in the topmost test directory. This is needed so @library
49 # does not need ../../ notation to reach them
50 external.lib.roots = ../../
So, for the stuff like VMProps we need to use "../.." in paths
Thanks,
Dima
On 16.06.2016 16:20, Michail Chernov wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Dima,
I have a question about paths:
requires.extraPropDefns = ../../test/jtreg-ext/requires/VMProps.java
-requires.properties=sun.arch.data.model
+requires.extraPropDefns.bootlibs = ../../test/lib/sun
Do we need to use relative paths instead of full? We use full path in tests:
TestSmallHeap.java: * @library /testlibrary /test/lib /test/lib/share/classes
Thanks,
Michail
On 06/14/2016 07:14 PM, Dmitry Fazunenko wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Stefan,
The idea of 'vm.gc.X.acceptable' was an ability to introduce other GC related properties like 'supported', 'default', 'setByErgonomics', 'deprecated', etc.
But setting 'vm.gc.X' won't prevent us from adding 'vm.gc.X.Y' later when needed.
I fully agree: @requires vm.gc.G1 looks much nicer and causes less questions.
New edition:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dfazunen/8151283/webrev.01/">http://cr.openjdk.java.net/~dfazunen/8151283/webrev.01/</a>
Thanks for suggestion that,
Dima
On 13.06.2016 10:46, Stefan Johansson wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Dima,
I don't know the code and the way this is invoked good enough to do a proper review, but I have a style comment. Wouldn't it be nice to only call the value "vm.gc.G1", and have:
@requires vm.gc.G1
I feel that "acceptable" provide very little information and what we want to @require is that the test is only run with G1 (when supported) and I think that is clear from such syntax. Comments?
Thanks,
Stefan
On 2016-06-10 18:04, Dmitry Fazunenko wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hello,
I'm looking for a couple of Reviewers for a simple change, which allows to skip tests depending on non supported garbage collectors.
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dfazunen/8151283/webrev.00/">http://cr.openjdk.java.net/~dfazunen/8151283/webrev.00/</a>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8151283">https://bugs.openjdk.java.net/browse/JDK-8151283</a>
The fix introduces four new requires boolean properties:
vm.gc.G1.acceptable
vm.gc.Serial.acceptable
vm.gc.Parallel.acceptable
vm.gc.ConcMarkSweep.acceptable
Which are set by the jtreg plugin prior the test execution starts.
After that fix we will be able to replace in tests:
@requires vm.gc == null | vm.gc == "G1"
with
@requires vm.gc.G1.acceptable
'vm.gc.G1.acceptable' will be evaluated to 'false' not only if other GC is set externally, but also if G1 is not supported.
In other words the fix allows not to run GC specific tests in not applicable configurations.
Thanks,
Dima
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>