Re: RFR (S): 8223736:jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails due to wrong number of MonitorContendedEntered events

[serguei.spit...@oracle.com]

Hi Dan, On 6/20/19 16:17, Daniel D. Daugherty wrote: On 6/19/19 9:59 PM, serguei.spit...@oracle.com wrote: Sorry, forgot the  bug title to add to the email subject. Thanks, Serguei On 6/19/19 6:09 PM, serguei.spit...@oracle.com wrote: Please review a fix for test bug:   https://bugs.openjdk.java.net/browse/JDK-8223736 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8223736-mon-events-test.1/ test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC04/tc04t001.java     L164:             if (enterEventsCount() == lastEnterEventsCount + 1) {         I wonder if '==' should be '>=' to more resilient against         multiple events. I tried to figure out the flow control         for the event posting, but I couldn't figure it out with         a quick look. The only one event can come at the time. It is because one worker thread is waiting for event to happen (does the check at line 164) and the other worker thread is expected to get the MonitorContendedEnter event. To make it a little bit simpler I can change it to:   if (enterEventsCount() > lastEnterEventsCount) {       L170:         if (enterEventsCount() != lastEnterEventsCount + 1) {         Similarly I wonder if this should be:                   if (enterEventsCount() == lastEnterEventsCount) {         to simply detect that the count hasn't changed. It is taken, thanks! I like it because it is a little bit simpler. :) Please, see the answer above. test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC04/tc04t001/tc04t001.cpp     No comments. Thumbs up. My concerns above may not be correct depending on how the test really works. Your call. Thanks a lot for review, Dan! Serguei Dan Summary:  It seems that waiting for 0.5 sec for a MonitorContendedEnter event in the  increment() method sometime is not enough (especially when the JFR is enabled).  The fix implement an approach to ensure the event has posted before the worker  thread goes to the next iteration.  Also, another check is added to diagnose if any of two worker threads  (tc04t001Thread) has been interrupted by timeout.  In fact, we have many other tests which miss this kind of check and diagnostics.  We may want to consider fixing other cases if we encounter this eventually happens. Testing:  A mach5 test submission is in progress. Thanks, Serguei