<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Aleksey,<div class=""><br class=""></div><div class="">I haven't review all changes. Just have a couple of small comments about tests. </div><div class="">1) It makes a sense to add @requires tag to all epsilon tests like it is done for other GC-specific tests. So jtreg filter them out if epsilon GC is not supported or any other GC is set in command-line explicitly.</div><div class="">Also you don't need to have any additional conditional checks and in tests themselves since you know that epsilon GC is supported inside the test.</div><div class=""><br class=""></div><div class="">See example for G1-specific test:</div><div class=""><a href="http://hg.openjdk.java.net/jdk/jdk/file/54eda3aad6dd/test/hotspot/jtreg/gc/g1/TestConcurrentSystemGC.java" class="">http://hg.openjdk.java.net/jdk/jdk/file/54eda3aad6dd/test/hotspot/jtreg/gc/g1/TestConcurrentSystemGC.java</a></div><div class=""><br class=""></div><div class="">The 'requires' support is here:</div><div class=""><a href="http://hg.openjdk.java.net/jdk/jdk/file/54eda3aad6dd/test/jtreg-ext/requires/VMProps.java" class="">http://hg.openjdk.java.net/jdk/jdk/file/54eda3aad6dd/test/jtreg-ext/requires/VMProps.java</a></div><div class=""><a href="http://hg.openjdk.java.net/jdk/jdk/file/54eda3aad6dd/test/lib/sun/hotspot/gc/GC.java" class="">http://hg.openjdk.java.net/jdk/jdk/file/54eda3aad6dd/test/lib/sun/hotspot/gc/GC.java</a></div><div class=""><br class=""></div><div class="">2) Also I don't see why these tests are not in tier1. </div><div class=""><br class=""></div><div class="">Group tier1_gc_2 includes all tests in gc/ until they are excluded explicitly. So you need to exclude gc/epsilon tests from it.</div><div class=""><br class=""></div><div class="">Leonid</div><div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On May 7, 2018, at 7:58 AM, Aleksey Shipilev <<a href="mailto:shade@redhat.com" class="">shade@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hi,<br class=""><br class="">This is round 2 code review for Epsilon GC changes.<br class=""><br class="">JEP, targeted to 11:<br class=""> <a href="http://openjdk.java.net/jeps/318" class="">http://openjdk.java.net/jeps/318</a><br class=""> (you can find links to binary builds and sandbox locations there)<br class=""><br class="">Webrev:<br class=""> <a href="http://cr.openjdk.java.net/~shade/epsilon/webrev.06/" class="">http://cr.openjdk.java.net/~shade/epsilon/webrev.06/</a><br class=""><br class="">Notes:<br class=""><br class=""> *) See how the whole thing is almost completely isolated to hotspot/share/gc, without having<br class="">arch-specific mess -- thanks to GC interface work done over last years. The only leftover is the<br class="">change in graphKit.cpp that should go away after C2 barriers modularization [1] is complete;<br class=""><br class=""> *) Recently added GC TLD facility and Elastic TLABs feature enables Epsilon to be more streamlined<br class="">in figuring out the TLAB sizes;<br class=""><br class=""> *) Conditional GC compilation is supported. ISVs may choose not to build Epsilon, if they do not<br class="">want to extend whatever small notion of support that comes with experimental VM option [2].<br class=""><br class=""> *) Half of the webrev are Epsilon-specific tests. They take ~30s in release and ~60s in fastdebug<br class="">on my desktop. They are not in tier1, so smoke testing time is not affected;<br class=""><br class=""><br class="">Builds:<br class=""> server X {x86_64, x86_32, aarch64, arm32, ppc64le, s390x}<br class=""> zero X {x86_64}<br class=""> minimal X {x86}<br class=""><br class="">Testing: gc/epsilon on x86_64<br class=""><br class="">Thanks,<br class="">-Aleksey<br class=""><br class="">[1] <a href="https://bugs.openjdk.java.net/browse/JDK-8202377" class="">https://bugs.openjdk.java.net/browse/JDK-8202377</a><br class="">[2] <a href="http://hg.openjdk.java.net/jdk/jdk/file/3661f31c6df4/src/hotspot/share/runtime/globals.hpp#l150" class="">http://hg.openjdk.java.net/jdk/jdk/file/3661f31c6df4/src/hotspot/share/runtime/globals.hpp#l150</a><br class=""><br class=""></div></div></blockquote></div><br class=""></div></body></html>