I think we're all 3 in agreement on the general direction, even if we disagree on the details.
Once we properly round to millis, it is no longer necessary to use Math.max - just wait(NANOSECONDS.toMillis(rem + 999_999L)); As in my proposed change, I would like every call to wait immediately followed by a check if (hasExited) return true; to avoid the extra call to nanoTime. On Tue, Nov 18, 2014 at 12:09 PM, roger riggs <roger.ri...@oracle.com> wrote: > Hi, > > Work on Object.wait is an issue to be taken up separately. > > I agree that the timeout values should not be truncated to milliseconds, > likely > causing an additional cycle through the while loop and waiting. > The Process.waitFor methods are expected to wait for the specified time to > elapse. > The webrev has been updated for both Windows and Unix to use a ceiling > function > on milliseconds and ensure that at least the requested time has elapsed. > > Please review: > http://cr.openjdk.java.net/~rriggs/webrev-basic-notenough-8064932/ > > Thanks, Roger > > > > > On 11/18/2014 11:08 AM, Martin Buchholz wrote: >> >> On Mon, Nov 17, 2014 at 8:54 PM, David Holmes <david.hol...@oracle.com> >> wrote: >>> >>> On 18/11/2014 2:43 PM, Martin Buchholz wrote: >>>> >>>> Proposed sibling change >>>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/UNIXProcess.waitFor/ >>>> - don't unconditionally call nanoTime when the wait ends >>>> - use the millis/nanos form of Object.wait in case sub-millisecond >>>> waits are ever supported. >>> >>> >>> I don't really see the point of adding the extra math, plus an extra call >>> (the two arg wait will call the one arg version) for no actual gain. >> >> The idea was to code to the Object.wait API, not its current >> implementation with only millisecond resolution. >> Even if we code to the current implementation, we should round UP not >> down, >> to avoid the problem of needing to call wait twice in case of early >> return. >> That would also be progress. >> >>> If you want to add a fast exit path that's fine but the rest seems >>> superfluous to me. > >
