RFR 9: 8064932: test java/lang/ProcessBuilder/Basic.java: waitFor didn't take long enough

[Martin Buchholz]

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.riggs at 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.holmes at 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.
>
>