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

[Vladimir Kozlov]

On Wed, 3 Mar 2021 19:54:11 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> 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]);
>>          }
>>      }
>
> By "not enough" you probably mean "while we are at it, we can do even better"? Because current version definitely helps JIT compilation, as evidenced by test times and the absence of compilation aborts.
> 
> Off the top of my head:
>  - Dropping `ITERS` one order of magnitude down to 50K does not improve test times, AFAICS.
>  - Verifying each on each step verifies OSR compilations as well, which must be good.
>  - Having a test call not in triplicate (how you suggest) but in singular (how it is in this PR) seems more readable.
> 
> I can take a look at these later, but honestly, I think the current patch is an acceptable thing to make `tier1` run fast again.

500,000 vs 50,000 ITER with your changes:
elapsed time (seconds): 22.664
vs
elapsed time (seconds): 6.683

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

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