[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR (S): 8223736:jvmti/scenarios/contention/TC04/tc04t001/TestDescription.java fails
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2019-06-21 0:47:48
Message-ID: f7409ec4-931a-a53d-c07f-6cf56f2d7032 () oracle ! com
[Download RAW message or body]

On 6/20/19 17:36, Daniel D. Daugherty wrote:
> On 6/20/19 7:59 PM, serguei.spitsyn@oracle.com wrote:
>> Hi Alex,
>>
>> Thank you a lot for review!
>>
>>
>>
>> On 6/20/19 16:30, Alex Menkov wrote:
>>> Hi Serguei,
>>>
>>> Main idea looks good.
>>>
>>> I'd simplify threads[i].join/threads[i].done logic with CountDownLatch
>>> (note also exception message ("Thread-" + i + " was interrupted by 
>>> timeout!") is not correct):
>>>
>>> in tc04t001 class add
>>>    final static CountDownLatch threadsDoneSignal = new 
>>> CountDownLatch(THREADS_LIMIT);
>>>
>>> replace cycle of threads[i].join / threads[i].done check with
>>>    if (!threadsDoneSignal.await(timeout, TimeUnit.MILLISECONDS)) {
>>>        throw new RuntimeException("Threads timeout");
>>>    }
>>> at the end of tc04t001Thread.run
>>> replace
>>>    done = true;
>>> with
>>>    tc04t001.threadsDoneSignal.countDown();
>>
>> Okay, thanks!
>> I've made this change.
>>
>> The updated webrev is:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8223736-mon-events-test.2/ 
>>
>
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC04/tc04t001.java 
>
>       L165:                System.out.println("Thread-" + i + ": increment 
> event: " + (lastEnterEventsCount + 1));
>                nit - trailing space at the end of this line (jcheck)
>
>                Do you want "(lastEnterEventsCount + 1)" or do you
>                want "enterEventsCount()" here?

It has to be the same.
But I'll change it to enterEventsCount() to follow the suggested fix logic.

Thanks!
Serguei:

>
> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/contention/TC04/tc04t001/tc04t001.cpp 
>
>        No comments.
>
> Thumbs up. No need for a new webrev if you choose to change
> the message above.
>
> Dan
>
>>
>> It also includes the changes suggested by Dan.
>>
>> Thanks,
>> Serguei
>>
>>>
>>> --alex
>>>
>>> On 06/19/2019 18:59, serguei.spitsyn@oracle.com wrote:
>>>> Sorry, forgot the   bug title to add to the email subject.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>> On 6/19/19 6:09 PM, serguei.spitsyn@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/ 
>>>>>
>>>>>
>>>>> 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
>>>>>
>>>>
>>
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]