RFR: JDK-8220813: update hotspot tier1_gc tests depending on GC to use @requires vm.gc.X

Ao Qi aoqi at loongson.cn
Thu Mar 28 10:43:00 UTC 2019


Hi,

I updated the fix, please see:
http://cr.openjdk.java.net/~aoqi/8220813/webrev.01

I divided the modified tests into four groups, G1, G2, G3 and G4.

G1: tests with minor fix of @requires field. G1 list [1]:
test/hotspot/jtreg/gc/TestNumWorkerOutput.java
test/hotspot/jtreg/gc/arguments/TestMinInitialErgonomics.java
test/hotspot/jtreg/gc/arguments/TestParallelHeapSizeFlags.java
test/hotspot/jtreg/gc/arguments/TestSelectDefaultGC.java
test/hotspot/jtreg/gc/class_unloading/TestCMSClassUnloadingEnabledHWM.java
test/hotspot/jtreg/gc/class_unloading/TestG1ClassUnloadingHWM.java
test/hotspot/jtreg/gc/cms/GuardShrinkWarning.java
test/hotspot/jtreg/gc/g1/TestShrinkDefragmentedHeap.java
test/hotspot/jtreg/gc/logging/TestPrintReferences.java
test/hotspot/jtreg/gc/metaspace/TestMetaspaceCMSCancel.java
test/hotspot/jtreg/gc/parallel/AdaptiveGCBoundary.java
test/hotspot/jtreg/gc/startup_warnings/TestCMS.java
test/hotspot/jtreg/gc/startup_warnings/TestG1.java
test/hotspot/jtreg/gc/startup_warnings/TestParallelGC.java
test/hotspot/jtreg/gc/startup_warnings/TestParallelScavengeSerialOld.java

test/hotspot/jtreg/gc/arguments/TestSelectDefaultGC.java is a little
special. It assumes G1 is default for server class machines, and I
added @requires vm.gc.G1.

G2: tests were splited in @run field according to parameters to cover
GCs separately. G2 list [2]:
test/hotspot/jtreg/gc/TestAgeOutput.java
test/hotspot/jtreg/gc/TestGenerationPerfCounter.java
test/hotspot/jtreg/gc/TestHumongousReferenceObject.java
test/hotspot/jtreg/gc/TestMemoryMXBeansAndPoolsPresence.java
test/hotspot/jtreg/gc/TestPolicyNamePerfCounter.java
test/hotspot/jtreg/gc/TestSystemGC.java
test/hotspot/jtreg/gc/arguments/TestAlignmentToUseLargePages.java
test/hotspot/jtreg/gc/arguments/TestMaxNewSize.java
test/hotspot/jtreg/gc/arguments/TestUseCompressedOopsErgo.java
test/hotspot/jtreg/gc/class_unloading/TestClassUnloadingDisabled.java
test/hotspot/jtreg/gc/metaspace/TestMetaspacePerfCounters.java

G3 and G4: tests used GC.XX.isSupported() to cover GCs separately.
Tests in G3 include minor change, and tests in G4 do not.
G3 list [3]:
test/hotspot/jtreg/gc/TestSmallHeap.java
test/hotspot/jtreg/gc/ergonomics/TestDynamicNumberOfGCThreads.java
test/hotspot/jtreg/gc/ergonomics/TestInitialGCThreadLogging.java
test/hotspot/jtreg/gc/logging/TestGCId.java

G4 list [4]:
test/hotspot/jtreg/gc/arguments/TestParallelGCThreads.java
test/hotspot/jtreg/gc/arguments/TestParallelRefProc.java
test/hotspot/jtreg/gc/logging/TestMetaSpaceLog.java

Test results:

I tested the tests modified by this patch, using:
  jtreg -jdk:${path}/jdk -dir:/home/aoqi/jdk/test/hotspot/jtreg/
-verbose:summary ${g1_list} ${g2_list} ${g3_list} ${g4_list}

before fix:
  javaoption                                    pass  fail  error
  defaut                                          49    0    0
  -XX:+UseG1GC                          14    0    0
  -XX:+UseSerialGC                      14    0     0
  -XX:+UseParallelGC                   14    0     0
  -XX:+UseConcMarkSweepGC    14    0     0

after fix:
  javaoption                                    pass  fail  error
  defaut                                          69    0    0
  -XX:+UseG1GC                          23    0    0
  -XX:+UseSerialGC                      16    0    0
  -XX:+UseParallelGC                   21    0    0
  -XX:+UseConcMarkSweepGC   21    0    0

The differences are mainly because:
1. Some tests are splited.
2. @requires vm.gc=="null" => @requires vm.gc.XX, when using
-XX:+UseXXGC, more test will be executed after fix.
3. Some tests used -XX:+UseXXGC in the test code, not in the @run
field. For example, gc/startup_warnings/TestG1.java will not be
executed when using -javaoption:-XX:+UseConcMarkSweepGC after the fix.
It was executed when using -javaoption:-XX:+UseConcMarkSweepGC before
the fix.

I also did some test using jdk which doesn't support a particular GC.
The method is using configure --with-jvm-features to disable a
particular GC.

before (configure using --with-jvm-features):
                             pass fail error
  default                  49    0    0
  -cmsgc                 28    21    0
  -epsilongc            49    0    0
  -g1gc,-jfr              25    24    0
  -parallelgc            27    22    0
  -shenandoahgc    43    0    0
  -zgc                      49    0    0

after (configure using --with-jvm-features):
                             pass fail error
  default                  69    0    0
  -cmsgc                 55    0    0
  -epsilongc            69    0    0
  -g1gc,-jfr              52    0    0
  -parallelgc            54    0    0
  -shenandoahgc    63    0    0
  -zgc                      69    0    0

Some more comments are inline.

On Wed, Mar 20, 2019 at 10:45 AM Leonid Mesnik <leonid.mesnik at oracle.com> wrote:
>
> Hi
>
>
> On Mar 19, 2019, at 6:36 PM, Ao Qi <aoqi at loongson.cn> wrote:
>
> Hi Leonid Mesnik,
>
> Thanks for you comments. Please see inline.
>
> See my answers inline.
>
>
> On Wed, Mar 20, 2019 at 4:02 AM Leonid Mesnik <leonid.mesnik at oracle.com> wrote:
>
>
> Hi
>
> Using 'requires vm.gc.XXX' is correct way to select supported GC. However I think your fix is incomplete. Your fix skips tests if any of tested GC is not supported. I think that it would be better to test supported configurations rather then skip whole test.
>
> Let see test
> http://cr.openjdk.java.net/~aoqi/8220813/webrev.00/test/hotspot/jtreg/gc/TestAgeOutput.java.udiff.html
>
> If G1 is not build then whole test is skipped. So it makes sense to split following test
>
>  26 /*
>  27  * @test TestAgeOutput
>  28  * @bug 8164936
>  29  * @summary Check that collectors using age table based aging print an age table even for the first garbage collection
>  30  * @key gc
>  31  * @requires vm.gc.Serial & vm.gc.G1
>  32  * @modules java.base/jdk.internal.misc
>  33  * @library /test/lib
>  34  * @build sun.hotspot.WhiteBox
>  35  * @run driver ClassFileInstaller sun.hotspot.WhiteBox
>  36  * @run main/othervm -XX:+UseSerialGC gc.TestAgeOutput UseSerialGC
>  37  * @run main/othervm -XX:+UseG1GC gc.TestAgeOutput UseG1GC
>  38  */
>  39
>
> into 2 tests to cover Serial/G1 GC separately.
>
>   /*
>    * @test TestAgeOutput
>    * @bug 8164936
>    * @summary Check that collectors using age table based aging print an age table even for the first garbage collection
>    * @key gc
>    * @requires vm.gc.Serial
>    ...
>    * @run main/othervm -XX:+UseSerialGC gc.TestAgeOutput UseSerialGC
>
>    ...
>
>   /*
>    * @test TestAgeOutput
>    * @bug 8164936
>    * @summary Check that collectors using age table based aging print an age table even for the first garbage collection
>    * @key gc
>    * @requires vm.gc.G1
> ...
>    * @run main/othervm -XX:+UseG1GC gc.TestAgeOutput UseG1GC
> ...
>
> Good suggest. I will try that.

Done that. It is in G2.

>
> The same for tests which execute VM with different arguments and don't use external arguments. Test
> http://cr.openjdk.java.net/~aoqi/8220813/webrev.00/test/hotspot/jtreg/gc/arguments/TestParallelRefProc.java.html
> is not going to be executed if any of GC is not supported.
>
> Indeed, after the fix these tests would be skipped, but before the fix
> these tests would fail. I am not sure which is better. I think the
> best way is also to split the test, but I cannot find a easy way like
> above. Could you give some hint?
>
> Tests might better reorganized like TestAgeOutput.java so VM options are given as test parameters in @run or as vm parameters.
> As another way you could check if GC is supported using sun.hotspot.gc.GC.isSupported/isSelected and throw  SkippedException if tests should not be executed. The example is checking Shenandoah in: ./ergonomics/TestDynamicNumberOfGCThreads.java.

Done that. It is in G3 and G4.

>
>
> The another question is that before fix it was enough to run tests with default GC and all supported GC to get full coverage without significant duplication. But now it seems that a lot of tests will be executed twice.
> Might be after complete fix it would be enough to run tests with selected GC only? Could you please provide some summary of test coverage/duplication after your fix.
>
>
> I didn't notice that (a lot of tests will be executed twice). Could
> you give me one example?
>
> The test http://cr.openjdk.java.net/~aoqi/8220813/webrev.00/test/hotspot/jtreg/gc/arguments/TestParallelRefProc.java.html is example.
> If you run all tests with default GC + with G1, CMS, Parallel & Serial GC this test will be executed 5 times with all GC. While previously it has been executed only once.
>

Sorry, I still don't understand very much. Does the problem still
exist in the new version of this patch?

> I think that intention would be to test only specified GC and some reasonable combination for default GC.
>
>
> I am not sure if this fix is worth doing, so comments are welcome:)
>
> It certainly worth if anyone is going to build and test JDK without Parallel/G1/CMS GC. The one example is minimal VM which supports SerialGC only. I don't know anything about other examples.
> Appropriate fix also might reduce required testing by better test selection and improve test maintenance.
> It might helps if you provide motivation for this fix as a part of review.
>

The initial motivation I fix this is because I am porting jdk to mips
and the default GC of this mips port is not G1 (G1 crashes) at
present. When I did jtreg test, some tests failed because G1 is not
supported, or even is not default. I have to find which failure is
because of G1 and which is not every time.

Cheers,
Ao Qi

[1] http://cr.openjdk.java.net/~aoqi/8220813/01/g1
[2] http://cr.openjdk.java.net/~aoqi/8220813/01/g2
[3] http://cr.openjdk.java.net/~aoqi/8220813/01/g3
[4] http://cr.openjdk.java.net/~aoqi/8220813/01/g4

> Leonid
>
>
> Thanks,
> Ao Qi
>
> Leonid
>
> On Mar 19, 2019, at 2:48 AM, Ao Qi <aoqi at loongson.cn> wrote:
>
> Hi,
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8220813
> Webrev: http://cr.openjdk.java.net/~aoqi/8220813/webrev.00/
>
> If a particular GC is not supported, some hotspot tier1_gc tests
> fails. The patch allows tests to be skipped on platforms where a GC is
> not supported.
>
> Tested:
>
> before (configure using --with-jvm-features):
> configure          TOTAL  PASS  FAIL ERROR
> default                  251   251     0     0
> -cmsgc                 243   222    21     0
> -epsilongc            229   229     0     0
> -g1gc,-jfr              181   157    24     0
> -parallelgc            241   219    22     0
> -shenandoahgc    206   206     0     0
> -zgc                      251   251     0     0
>
> after (configure using --with-jvm-features):
> configure          TOTAL  PASS  FAIL ERROR
> default                    251   251     0     0
> -cmsgc                   222   222     0     0
> -epsilongc              229   229     0     0
> -g1gc,-jfr                157   157     0     0
> -parallelgc              219   219     0     0
> -shenandoahgc      206   206     0     0
> -zgc                        251   251     0     0
>
> before:
> VM_OPTIONS                         TOTAL  PASS  FAIL ERROR
> default                                        251   251     0     0
> -XX:+UseG1GC                         129   129     0     0
> -XX:+UseSerialGC                      65    65     0     0
> -XX:+UseParallelGC                   69    69     0     0
> -XX:+UseConcMarkSweepGC    67    67     0     0
>
> after:
> VM_OPTIONS                         TOTAL  PASS  FAIL ERROR
> default                                        251   251     0     0
> -XX:+UseG1GC                         121   121     0     0
> -XX:+UseSerialGC                      51    51     0     0
> -XX:+UseParallelGC                   60    60     0     0
> -XX:+UseConcMarkSweepGC    66    66     0     0
>
> The difference in VM_OPTIONS tests mainly comes from two reasons:
> 1. This patch make this kind of change:
>
> @requires vm.gc=="null" & !vm.graal.enabled
> =>
> @requires vm.gc.ConcMarkSweep & !vm.graal.enabled
>
> Take gc/arguments/TestAlignmentToUseLargePages.java#id1 for example.
> Before the patch, even if VM_OPTIONS=-XX:+UseConcMarkSweepGC is set,
> gc/arguments/TestAlignmentToUseLargePages.java#id1 will not be
> executed (but the test is intended to test -XX:+UseConcMarkSweepGC).
> After the patch, the test will be executed if
> VM_OPTIONS=-XX:+UseConcMarkSweepGC is set.
>
> 2. If a test use a particular GC in the code (not in the jtreg tag
> fields) and another GC is passed by -XX:UseXGC, the test will be
> skipped.
> For example, gc/startup_warnings/TestG1.java uses -XX:+UseG1GC in the
> main method. This test will be skipped, when
> VM_OPTIONS=-XX:+UseConcMarkSweepGC is used. Before this patch, it will
> be executed.
>
> I am doing more tests and checking test results above. Could someone
> give some advice on this change?
>
> Thanks,
> Ao Qi
>
>
>


More information about the hotspot-gc-dev mailing list