On 6/20/19 7:59 PM, serguei.spit...@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?

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.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/

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





Reply via email to