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

[Vladimir Kozlov]

So you implemented Vladimir Ivanov's suggestion.
Okay but you should add a check that it did not finish the loop by 
checking _num value. Imaging very fast machine that will take less then 
2 sec to finish loop without safepoint.

Thanks,
Vladimir

On 2/16/16 6:24 AM, Andreas Eriksson wrote:
> Hi,
>
> I've changed the test so it will not timeout, by using the
> WhiteBox.forceSafepoint() in a separate thread.
> If there is a safepoint in the loop, then the test will exit after
> taking one safepoint.
> If instead all safepoints in the loop were removed, then the safepoint
> operation will timeout after two seconds (because of
> -XX:+SafepointTimeout) and it will be detected and the test will fail.
>
> Webrev: http://cr.openjdk.java.net/~aeriksso/8146096/webrev.00/
>
> Regards,
> Andreas
>
> On 2016-01-20 10:26, Andreas Eriksson wrote:
>> Vladimir Kozlov and Vladimir Ivanov,
>>
>> Ok, I'll look into using the whitebox api to fix the test.
>> Thanks for looking at this.
>>
>> - Andreas
>>
>> On 2016-01-19 21:46, Vladimir Kozlov wrote:
>>> Simple use timeout to check for generated safepoint is bad idea. It
>>> is very inaccurate. At least you need to check call stack to see if
>>> it stopped in compiled method.
>>> I would prefer to see WB new interface which would check that loop
>>> SafePointNode is generated during compilation of method. It will be
>>> precise.
>>>
>>> And we need such tests to make sure a feature is working - we can't
>>> remove them.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 1/19/16 4:50 AM, Vladimir Ivanov wrote:
>>>> As an idea to improve the test: spawn a thread which executes the
>>>> counted loop and then use WhiteBox.forceSafepoint() to
>>>> trigger a safepoint.
>>>>
>>>> If the test times out, it means there's no safepoint in the loop.
>>>>
>>>> Also, it also simplifies the implementation - no need to spawn a
>>>> child process, the check can be done in-process.
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> On 1/19/16 3:32 PM, Andreas Eriksson wrote:
>>>>> Hi,
>>>>>
>>>>> Can I please have a review for the removal of
>>>>> hotspot/test/compiler/loopopts/UseCountedLoopSafepoints.java.
>>>>>
>>>>> The test needs to do a loop that takes more than two seconds to
>>>>> execute
>>>>> fully without doing a safepointing call. For this expensive atomic
>>>>> operations were used. The problem is that on certain embedded
>>>>> platforms
>>>>> they are too expensive, and the test times out.
>>>>> The loop length could probably be reduced, and it should still work on
>>>>> faster machines. However, the test is not very useful, so I think it's
>>>>> better to just remove it to avoid future problems.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146096
>>>>> Test to be removed:
>>>>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/d84a55e7aaf8/test/compiler/loopopts/UseCountedLoopSafepoints.java
>>>>>
>>>>>
>>>>> (I can create a webrev if you think it necessary.)
>>>>>
>>>>> Thanks,
>>>>> Andreas
>>
>