On 07/03/2014 04:44 PM, Erik Gahlin wrote:
Hi Jaroslav,
Here is an updated webrev:
http://cr.openjdk.java.net/~egahlin/8028474_7/
Thanks. No more comments.
-JB-
See comments inline.
Jaroslav Bachorik skrev 2014-07-03 16:03:
Hi Erik,
nice cleanup!
L160 "break" statement after this line would save unnecessary
iterating when a process has already been found
I'm not a big fan of break statements in nested loops, so refactored out
a separate method that returns when the process has been found. Also
updated releaseStarted
L172 You could use "return true". -
'MonitoredVmUtil.mainArgs(target).contains(args)' has been asserted to
be true on L171
Ops, not sure how it got there.
Thanks
Erik
-JB-
On 07/03/2014 01:06 PM, Erik Gahlin wrote:
Hi Roger,
The test has been updated. It now uses System.nanoTime.
http://cr.openjdk.java.net/~egahlin/8028474_6/
Thanks
Erik
roger riggs skrev 2014-07-01 14:35:
Hi Erik,
Consider switching to System.nanoTime; it is not sensitive to clock
changes
and avoids leaving a land mine that may cause a spurious
non-repeatable test failure.
'Deducing it from the log' means there is a failure and creates
probably an hour or two of work
for some quality engineer and burns a couple of hours re-running the
test run.
Roger
On 7/1/2014 3:37 AM, Erik Gahlin wrote:
JavaProcess.waitForRemoval: How about using timestamps
(currentTimeMillis()) before the loop and for each ite
ration to determine if the timeout has expired (instead of
"time+=100”)?
The code now uses currentTimeMillis(). Premature timeouts due to
clock changes can be deduced from the log.
