Hi Dan,


On 6/21/19 06:44, Daniel D. Daugherty wrote:
On 6/21/19 1:24 AM, serguei.spit...@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


Reply via email to