RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind

[Jaroslav Bachorik]

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