RFR(S): 8146096 - [TEST BUG] compiler/loopopts/UseCountedLoopSafepoints.java Timeouts

[Vladimir Kozlov]

Yes, this looks good.

On 9/1/16 7:28 AM, Dmitrij Pochepko wrote:
> Thank you for attentive review.
>
>
>> Wow! You wrote "parser" to C2 Ideal graph.
>>
>> // now, find SafePoint->CountedLoopEnd edge
>>
>> Actually CountedLoopEnd  input edge should point to SafePoint. Not
>> reverse. You should search from LoopEnd up.
> fixed
>>
>>  115    SafePoint    ===  112  1  107  1  1  10  110  [[ 116 ]]
>> SafePoint !orig=76 !jvms: SimpleTest::testMethod @ bci:21
>>  116    CountedLoopEnd    ===  115  105  [[ 117  118 ]] [lt]
>> P=0.500000, C=6633.000000 !orig=97,[80] !jvms: SimpleTest::testMethod
>> @ bci:5
>>
>> The test is too simple. Without UseCountedLoopSafepoints the loop is
>> folded to accum += 100. You need a little more complex.
> I've changed increment to combination of different shifts
>>
>> Also test should be run only with C2 (Tiered should be off). I don't
>> think require vm.opt.TieredStopAtLevel is the same as
>> -XX:-TieredCompilation.
> Requires expression just help to ensure level 4 is available, because
> testMethod specifically compiled on level=4 using WhiteBox
> (CompilerWhiteBoxTest.COMP_LEVEL_FULL_OPTIMIZATION).
> This differs from original test idea to disable tiered compilation and
> trigger compilation by running cycle 2 billion times, which made it a
> long "stress" test. Now that method compiled via WhiteBox, this test
> became much more faster

Got it.

Thanks,
Vladimir

>
> Please take a look at v03:
> http://cr.openjdk.java.net/~dpochepk/8146096/webrev.03/
>
> Thanks,
> Dmitrij
>>
>> Thanks,
>> Vladimir
>>
>> On 8/31/16 12:03 PM, Dmitrij Pochepko wrote:
>>> Hi,
>>>
>>> Please take a look at v02.
>>>
>>> I've rewritten this test. Now it launch vm with
>>> -XX:+UseCountedLoopSafepoints (restricting compilation to tested method
>>> with single simple counted loop) and checks that output of
>>> -XX:+PrintIdeal have edge SafePoint -> CountedLoopEnd
>>>
>>> Then, launch the same with -XX:-UseCountedLoopSafepoints and checks that
>>> there is no such edge.
>>>
>>> I've tested fix via rbt on all platforms.
>>>
>>> webrev: http://cr.openjdk.java.net/~dpochepk/8146096/webrev.02/
>>>
>>>
>>> Thanks,
>>>
>>> Dmitrij
>>>
>>> On 29.08.2016 19:28, Vladimir Kozlov wrote:
>>>> I am not against marking test as stress but I think the test itself is
>>>> not good. It should be rewrote. I added comment to the JBS with
>>>> discussion during original 8146096 RFR.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 8/29/16 7:08 AM, Dmitrij Pochepko wrote:
>>>>> Hi,
>>>>>
>>>>> please review small fix for 8146096 - [TEST BUG]
>>>>> compiler/loopopts/UseCountedLoopSafepoints.java Timeouts
>>>>>
>>>>>
>>>>> Test timeouts on slow platforms, so, this fix adds execution control
>>>>> with respect to elapsed time. Also, test marked as stress.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~dpochepk/8146096/webrev.01/
>>>>>
>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8146096
>>>>>
>>>>>
>>>>> I've tested this fix on linux-amd64.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Dmitrij
>>>>>
>>>
>