RFR(S): 8221372: Test vmTestbase/nsk/jvmti/GetThreadState/thrstat001/TestDescription.java times out

[Alex Menkov]

+1

The only nit:

87     jvmtiError err = jvmti->SetEventNotificationMode(mode, 
event_type, event_thread);
   88     if (err != JVMTI_ERROR_NONE) {
   89         printf("Failed to disable % event: %s (%d)\n",
   90                event_name, TranslateError(err), err);
   91         result = STATUS_FAILED;
   92     }

This func is used to both enable/disable, but logging always report 
"Failed to disable".

--alex

On 11/25/2019 12:41, Chris Plummer wrote:
> Hi Serguei,
> 
> It looks like before your fix, runs were normally just a few seconds, 
> but there occasionally took 1 to 15 minutes, some of which result in a 
> timeout. I looked at some of your recent results and it looks like now 
> they are always just a few seconds, so that's a good sign that you 
> addressed the timeout issue.
> 
> Changes look good.
> 
> thanks,
> 
> Chris
> 
> On 11/25/19 1:38 AM, serguei.spitsyn at oracle.com wrote:
>> Please, review a fix for test bug:
>> https://bugs.openjdk.java.net/browse/JDK-8221372
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8221372-jvmti-thread-state.1/
>>
>>
>> Summary:
>>   The test timeouts always happen with the JFR recording and mostly on 
>> windows.
>>   I was not able to reproduce this with mach5 100 runs though.
>>   However, I think the issue is with the MethodEnter/MethodExit events 
>> that are set globally.
>>   It is not only ~20 times slower but also impacts all JFR methods 
>> working in background.
>>
>>   The fix includes the following changes:
>>    - the MethodEnter/MethodExit events are removed
>>    - the code is refactored to implement repeating fragments as functions
>>    - minimal tracing is added to help with analysis of timeouts if 
>> they remain
>>
>>
>> Testing:
>>   Tested the  vmTestbase/nsk/jvmti/GetThreadState/thrstat001 test with 
>> mach5 100 runs.
>>
>>
>> Thanks,
>> Serguei
>