<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi</p>
<p>I looked at the tests changes only. It seems that a some tests
might start failing if JDK built without Shenandoah GC. Also some
test are not going to be selected as expected.<br>
</p>
<p>Unfortunately logic of '@requires' and @run in jtreg tests
doesn't always work well for specific cases of different GC
selection. The '@requires' tags are combined with '&' and
whole test is selected or not. Test always execute ALL @run
actions so it fails if option -XX:+UseShenandoahGC is not
supported (not valid). The only way to split 'run' actions is to
add more @test with same sources. They could be in the same file.<br>
</p>
<p> See detailed info about jtreg tags here:<a
moz-do-not-send="true"
href="http://openjdk.java.net/jtreg/tag-spec.html">
http://openjdk.java.net/jtreg/tag-spec.html</a></p>
<p>Could you please run your tests with with JDK which built without
Shenandoah GC . It helps to verify that Shenandoah-specific
tests/runs are not selected when this GC is not supported. Also
it would be nice to verify that there are no any valid tests which
became filtered out with your patch. The running of all test suite
with available but not selected Shenandoah GC and enabled Graal
also might help to verify @requires settings. (It doesn't help
when Shenandoah GCis not supported though.) </p>
<p>I haven't looked at the tests in directory gc/shenandoah in
details. But all of them should be guarded with @requires. Placing
them in separate directory is not enough. See G1 tests as example:</p>
<a moz-do-not-send="true"
href="http://hg.openjdk.java.net/jdk/jdk/file/10c6e9066819/test/hotspot/jtreg/gc/g1/TestStringDeduplicationFullGC.java#l29">http://hg.openjdk.java.net/jdk/jdk/file/10c6e9066819/test/hotspot/jtreg/gc/g1/TestEagerReclaimHumongousRegions.java</a><br>
<p><br>
</p>
<p>See more detailed comments about shared tests:</p>
<p><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/TEST.groups.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/TEST.groups.sdiff.html</a></p>
<p>219: tier2_gc_shenandoah = \ <br>
</p>
<p>Usually tierN doesn't include tests from tierN-1. TierN is
executed after TierN-1 completed so no need to re-run the same
tests. The typical groups might looks like:</p>
<tt>tier1_gc_shenandoah = \</tt><tt><br>
</tt><tt> gc/shenandoah/<tier1-tests> \</tt><tt><br>
</tt><tt> other-tests</tt><tt><br>
</tt><tt><br>
</tt><tt>tier2_gc_shenandoah = \</tt><br>
<tt> gc/</tt><tt><tt>shenandoah/<tier2-tests></tt><tt> \</tt></tt><br>
<tt><tt>
</tt></tt><tt> -:tier1_gc_shenandoah </tt><tt><br>
</tt><br>
<tt>tier3_gc_shenandoah = \</tt><br>
<tt> gc/shenandoah/ \ //all-other-tests</tt><br>
<tt> -:tier2_gc_shenandoah</tt><br>
<p><br>
</p>
<p><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/CriticalNativeArgs.java.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/CriticalNativeArgs.java.html</a></p>
<p><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/stress/CriticalNativeStress.java.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/stress/CriticalNativeStress.java.html</a><br>
</p>
<p>So your test will be skipped if any of -XX:+UseEpsilonGC or
-XX:+UseShenandoahGC is set. Also test might run only all run
actions or none of them. It would be better to split this test
into 2 tests. So epsilon tests might be executed if Shenandoah is
absent.</p>
<p>I think that (vm.bits == "64") is redundant in tag (you set x64
or arm64). <br>
</p>
<p>Please use 4-space indentation in java code. <br>
</p>
<p><br>
</p>
<p><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/TestFullGCCount.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/TestFullGCCount.java.sdiff.html</a></p>
<p>Even original requires seems confusing to me (but it works for
CMS/G1 pair)<br>
</p>
<p>28 * @requires !(vm.gc.ConcMarkSweep &
vm.opt.ExplicitGCInvokesConcurrent == true)<tt> </tt></p>
<div class="moz-cite-prefix">So currently test is executed if GC is
CMS or default GC and ExplicitGCInvokesConcurrent is not set to
true.</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">With your additional requirements
'vm.gc == "Shenandoah"' test is not selected if ANY GC is set.
Test doesn't set any GC itself so only default GC might be
tested. See [1].<br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/TestHumongousReferenceObject.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/TestHumongousReferenceObject.java.sdiff.html</a></div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/TestSystemGC.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/TestSystemGC.java.sdiff.html</a><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">Tests will always just fail if
-XX:+UseShenandoahGC is not supported. (invalid option) You might
want to split test is it done for CMS GC.<br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/arguments/TestAlignmentToUseLargePages.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/arguments/TestAlignmentToUseLargePages.java.sdiff.html</a></div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">I think <br>
</div>
<div class="moz-cite-prefix">56 * @requires vm.gc=="null" &
!vm.graal.enabled <br>
</div>
<div class="moz-cite-prefix">should be something like @requires
vm.gc.Shenandoah & !vm.graal.enabled <br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/arguments/TestUseCompressedOopsErgo.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/arguments/TestUseCompressedOopsErgo.java.sdiff.html</a></div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/class_unloading/TestClassUnloadingDisabled.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/class_unloading/TestClassUnloadingDisabled.java.sdiff.html</a><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">The same for 62 * @requires
vm.gc=="null" & !vm.graal.enabled</div>
<div class="moz-cite-prefix">and</div>
<div class="moz-cite-prefix">72 * @requires vm.gc=="null" &
!vm.graal.enabled</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/ergonomics/TestDynamicNumberOfGCThreads.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/ergonomics/TestDynamicNumberOfGCThreads.java.sdiff.html</a></div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/logging/TestGCId.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/logging/TestGCId.java.sdiff.html</a></div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/metaspace/TestMetaspacePerfCounters.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/metaspace/TestMetaspacePerfCounters.java.sdiff.html</a></div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java.sdiff.html</a><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java.sdiff.html</a></div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/metaspace/TestMetaspacePerfCounters.java.sdiff.html">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/test/hotspot/jtreg/gc/metaspace/TestMetaspacePerfCounters.java.sdiff.html</a><br>
</div>
<div class="moz-cite-prefix">Also these tests are going to be run
with all GC and fails if Shenandoah is not supported.</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">Leonid<br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">[1] <a moz-do-not-send="true"
href="http://openjdk.java.net/jtreg/tag-spec.html">http://openjdk.java.net/jtreg/tag-spec.html</a><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 11/26/18 1:39 PM, Roman Kennke
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:9ff4171e-9827-8710-554a-b84da309277a@redhat.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<p>Hi,<br>
<br>
This is the first round of changes for including Shenandoah GC
into mainline. <br>
I divided the review into parts that roughly correspond to the
mailing lists<br>
that would normally review it, and I divided it into 'shared'
code changes and<br>
'shenandoah' code changes (actually, mostly additions). The
intend is to eventually<br>
push them as single 'combined' changeset, once reviewed.<br>
<br>
JEP: <br>
<a moz-do-not-send="true"
href="https://openjdk.java.net/jeps/189">https://openjdk.java.net/jeps/189</a><br>
Bug entry:</p>
<p> <a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8214259">https://bugs.openjdk.java.net/browse/JDK-8214259</a><br>
</p>
<p>Webrevs:<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/">http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/</a><br>
<br>
For those who want to see the full change, have a look at the <a
moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-complete/">shenandoah-complete</a>
directory,<br>
it contains the full combined webrev. Alternatively, there is
the file <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-master.patch">shenandoah-master.patch</a>,<br>
which is what I intend to commit (and which should be equivalent
to the 'shenandoah-complete' webrev).<br>
<br>
Sections to review (at this point) are the following:<br>
*) <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-gc/">shenandoah-gc</a><br>
- Actual Shenandoah implementation, almost completely
residing in gc/shenandoah<br>
<br>
*) <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-gc/">shared-gc</a><br>
- This is mostly boilerplate that is common to any GC<br>
- referenceProcessor.cpp has a little change to make one
assert not fail (next to CMS and G1)<br>
- taskqueue.hpp has some small adjustments to enable
subclassing<br>
<br>
*) <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-serviceability/">shared-serviceability</a><br>
- The usual code to support another GC<br>
<br>
*) <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-runtime/">shared-runtime</a><br>
- A number of friends declarations to allow Shenandoah
iterators to hook up with,<br>
e.g. ClassLoaderData, CodeCache, etc<br>
- Warning and disabling JFR LeakProfiler<br>
- fieldDescriptor.hpp added is_stable() accessor, for use in
Shenandoah C2 optimizations<br>
- Locks initialization in mutexLocker.cpp as usual<br>
- VM operations defines for Shenandoah's VM ops<br>
- globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in
Shenandoah's logging<br>
- The usual macros in macro.hpp<br>
<br>
*) <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-build/">shared-build</a><br>
- Add shenandoah feature, enabled by default, as agreed with
Vladimir K. beforehand<br>
- Some flags for shenandoah-enabled compilation to get
SUPPORT_BARRIER_ON_PRIMITIVES<br>
and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for
Shenandoah's barriers<br>
- --param inline-unit-growth=1000 settings for 2 shenandoah
source files, which is<br>
useful to get the whole marking loop inlined (observed
significant regression if we<br>
don't)<br>
<br>
*) <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/">shared-tests</a><br>
- Test infrastructure to support Shenandoah<br>
- Shenandoah test groups<br>
- Exclude Shenandoah in various tests that can be run with
selected GC<br>
- Enable/add configure for Shenandoah for tests that make
sense to run with it<br>
<br>
*) <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-tests/">shenandoah-tests</a><br>
- Shenandoah specific tests, most reside in gc/shenandoah
subdirectory<br>
- A couple of tests configurations have been added, e.g.
TestGCBasherWithShenandoah.java<br>
<br>
I intentionally left out shared-compiler for now, because we
have some work left to do<br>
there, but if you click around you'll find the patch anyway, in
case you want to take<br>
a peek at it.<br>
<br>
We have regular builds on:<br>
- {Linux} x {x86_64, x86_32, armhf, aarch64, ppc64el, s390x}<br>
- {Windows} x {x86_64}, <br>
- {MacOS X} x {x86_64}<br>
<br>
This also routinely passes:<br>
- the new Shenandoah tests<br>
- jcstress with/without aggressive Shenandoah verification<br>
- specjvm2008 with/without aggressive Shenandoah verification</p>
<p><br>
I'd like to thank my collegues at Red Hat: Christine Flood, she
deserves the credit for being the original inventor of
Shenandoah, Aleksey Shiplëv, Roland Westrelin & Zhengyu Gu
for their countless contributions, everybody else in Red Hat's
OpenJDK team for testing, advice and support, my collegues in
Oracle's GC, runtime and compiler teams for tirelessly helping
with and reviewing all the GC interface and related changes, and
of course the many early adopters for reporting bugs and success
stories and feature requests: we wouldn't be here without any of
you!<br>
<br>
Best regards,<br>
Roman<br>
<br>
</p>
</blockquote>
</body>
</html>