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

[Dmitrij Pochepko]

Thank you!


On 01.09.2016 21:03, Vladimir Kozlov wrote:
> Nice!
>
> Thank you,
> Vladimir
>
> On 9/1/16 11:03 AM, Dmitrij Pochepko wrote:
>> Hi,
>>
>> I've added LoopUnrollLimit=0 to spawned vm options and tested on
>> linux-amd64.
>>
>> Please take a look at v04:
>> http://cr.openjdk.java.net/~dpochepk/8146096/webrev.04/
>>
>> Thanks,
>>
>> Dmitrij
>>
>> On 01.09.2016 20:47, Vladimir Kozlov wrote:
>>> I forgot an other useful C2 flag -XX:LoopUnrollLimit=0  to avoid loop
>>> iteration splitting. Otherwise you will get several loops (pre-, main-
>>> post-).
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 9/1/16 10:40 AM, Vladimir Kozlov wrote:
>>>> 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
>>>>>>>>>
>>>>>>>
>>>>>
>>