(13) RFR (S): 8224555: vmTestbase/nsk/jvmti/scenarios/contention/TC02/tc02t001/TestDescription.java failed

[serguei.spitsyn at oracle.com]

Thanks a lot!
Serguei

On 6/21/19 17:34, Alex Menkov wrote:
> Looks good to me too.
>
> --alex
>
> On 06/21/2019 11:38, serguei.spitsyn at oracle.com wrote:
>> Hi Dan,
>>
>>
>> On 6/21/19 06:44, Daniel D. Daugherty wrote:
>>> On 6/21/19 1:24 AM, serguei.spitsyn at oracle.com wrote:
>>>> Please, review the test bug fix for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8224555
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8224555-mon-events2-test.1/ 
>>>>
>>>
>>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC02/tc02t001.java 
>>>
>>>     L99:             for (int j = 0; j < 1000; j++) {
>>>         Why a literal '1000' this time? You could use "timeout / 20"
>>>         based on:
>>>
>>>         L120:         timeout = argHandler.getWaitTime() * 60 * 1000;
>>>
>>>         That seems to be what all the other timeout logic is based on.
>>
>> Good suggestion, thanks.
>> Taken.
>>
>>
>>>
>>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC02/tc02t001/tc02t001.cpp 
>>>
>>>     No comments.
>>>
>>> Thumbs up. I don't need a new webrev if you decide to make the
>>> minor change above.
>>
>> Thanks you for review, Dan!
>> Serguei
>>
>>
>>>
>>> Dan
>>>
>>>>
>>>>
>>>> Summary:
>>>>   The test sleeps for 1 sec in hope to get a contention on the 
>>>> monitor tc02t001Thread.M.
>>>>   It seems, this is not enough when the JFR is enabled.
>>>>   The fix uses a better approach to ensure events are really happen.
>>>>   This approach is similar to the on in the fix of 8223736 (just 
>>>> reviewed).
>>>>
>>>>   Also, the class line number sensitive tc02t001Thread is moved to 
>>>> the beginning of
>>>>   the file to make it independent from the rest of the file.
>>>>
>>>>
>>>> Testing:
>>>>   A mach5 submission is in progress.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>
>>