RFR: 8262950: Restructure compiler/intrinsics/TestRotate.java for easier compilation

Vladimir Kozlov kvn at openjdk.java.net
Wed Mar 3 19:36:45 UTC 2021


On Wed, 3 Mar 2021 14:57:43 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> It seems that after [JDK-8256438](https://bugs.openjdk.java.net/browse/JDK-8256438) this test is very slow. See [JDK-8262465 ](https://bugs.openjdk.java.net/browse/JDK-8262465) for more evidence of this.
> I believe the underlying cause is that `main()` is way too busy with test cases, so this eventually happens:
> 
> $ javac test/hotspot/jtreg/compiler/intrinsics/TestRotate.java
> $ java -XX:-TieredCompilation -XX:CompileThreshold=1000 -Xbatch -XX:+PrintCompilation -cp test/hotspot/jtreg compiler.intrinsics.TestRotate
>  ...
>    6112 112 % !b compiler.intrinsics.TestRotate::main @ 1494 (3008 bytes) COMPILE SKIPPED: out of nodes during split (retry at different tier)
> 
> So, while the test is intended to test rotate intrinsics, I think it never gets there because compilation bails. And this also makes the test slow. This is tier1 test, and so its performance is somewhat important for developer experience.
> 
> This improvement restructures the test to be easier to JIT compile. This drops the test run time from ~4 minutes to ~20 seconds on my desktop. Plus, additional diagnostic flag verifies that compiler does not bail anywhere.
> 
> Attn @theRealELiu.
> 
> Additional testing:
>   - [x] Linux x86_64 fastdebug, affected test
>   - [x] Linux x86_64 release, affected test
>   - [x] Linux aarch64 fastdebug, affected test

I completely agree with optimizing these tests. But what you did is not enough.

`ITER = 500,000;` is too much. C2 compilation will be triggered at 10,000. If you want to be completely sure use 50,000 which is more than enough.

Second, `verify()` should be called only after compilation. Why to call it for Interpreter? You can.

Third, `ITER` and `*_VALUES` loops should be inverted - hot loop will be inner.

Fourth, `verify()` error message could be part of `new Error()` message instead of separate print line. It will be ease to see.

Something like this (indents could be wrong):
    public static void testRorOrInts() {
         for (int i = 0; i < INT_VALUES.length; i++) {
             int val = INT_VALUES[i];
             verify("testRorOrInt1(" + val + ")",  testRorOrInt1(val),  TEST_ROR_OR_INT_1_EXPECTED[i]);
             verify("testRorOrInt16(" + val + ")", testRorOrInt16(val), TEST_ROR_OR_INT_16_EXPECTED[i]);
             verify("testRorOrInt31(" + val + ")", testRorOrInt31(val), TEST_ROR_OR_INT_31_EXPECTED[i]);
             verify("testRorOrInt32(" + val + ")", testRorOrInt32(val), TEST_ROR_OR_INT_32_EXPECTED[i]);
             for (int count = 0; count < ITERS; count++) {
                  testRorOrInt1(val);
                  testRorOrInt16(val);
                  testRorOrInt31(val);
                  testRorOrInt32(val);
             }
             verify("testRorOrInt1(" + val + ")",  testRorOrInt1(val),  TEST_ROR_OR_INT_1_EXPECTED[i]);
             verify("testRorOrInt16(" + val + ")", testRorOrInt16(val), TEST_ROR_OR_INT_16_EXPECTED[i]);
             verify("testRorOrInt31(" + val + ")", testRorOrInt31(val), TEST_ROR_OR_INT_31_EXPECTED[i]);
             verify("testRorOrInt32(" + val + ")", testRorOrInt32(val), TEST_ROR_OR_INT_32_EXPECTED[i]);
         }
     }

-------------

PR: https://git.openjdk.java.net/jdk/pull/2811


More information about the hotspot-compiler-dev mailing list