Hi Daniil,
For suspend001, are you saying the following is expected to timeout
sometimes, so you need a shorter waittime to avoid making the whole test
time out?
456 eventSet = eventQueue.remove
(Utils.adjustTimeout(waitTime*1000));
And I just noticed the space right after "remove". Can you remove it?
thanks,
Chris
On 2/25/19 6:57 PM, Daniil Titov wrote:
Hi Chris,
The timeout issue mentioned in the bug is about jtreg aborting the tests since
they are running longer than the maximum allowed time. That happens since these
tests use extreme long internal delays, e.g. a sleep for 5 minutes or a wait
for 5 minutes for a case when no events ( and a notify()) are expected.
Reducing these internal delays makes the test passing within the default jtreg
timeout ( 2 minutes).
Best regards,
Daniil
On 2/25/19, 6:15 PM, "Chris Plummer" <[email protected]> wrote:
Ok. So how is the timeout issue mentioned in the bug addressed when
there is now a shorter wait time?
Chris
On 2/25/19 5:04 PM, Daniil Titov wrote:
> Hi Chris,
>
> Forgot to answer to your another question:
> > > For these 3 tests the event wait timeout was reduced and
adjusted for test.timeout.factor:
> > > -vmTestbase/nsk/jdi/Event/_itself_/event001.java
> > >
-vmTestbase/nsk/jdi/VirtualMachine/suspend/suspend001/TestDescription.java
> > >
-vmTestbase/nsk/jdi/ThreadReference/suspend/suspend001.java
> > So overall is this a shorter or longer waittime now?
>
> Overall this is a shorter waitime now. Instead of 300 seconds it is now
20 seconds for Mach5 jobs (they are run with test.timeout.factor set to 4.0) and 5
seconds for regular jtreg runs.
>
> Best regards,
> Daniil
>
>
> On 2/25/19, 4:38 PM, "Chris Plummer" <[email protected]> wrote:
>
> Hi Daniil,
>
> Yes, my point was that the max time you wait for a single event is
much
> smaller now. I can see a possibility that with a little bit of
network
> instability a packet gets lost and resend does not happen fast
enough.
>
> thanks,
>
> Chris
>
> On 2/25/19 4:32 PM, Daniil Titov wrote:
> > Hi Chris,
> >
> > The code still waits for the whole total wait time. There is a
while loop at lines 163-186 that keeps receiving new events (line 183) till elapsed
time is less than the waittime (line 178) or a timeout happens (so eventSet is null).
> >
> > 159 begin = System.currentTimeMillis();
> > 160 eventSet =
debugee.VM().eventQueue().remove(waitTime);
> > 161 delta = System.currentTimeMillis() -
begin;
> > 162 totalWaitTime -= delta;
> > 163 while (eventSet != null) {
> > 164 EventIterator eventIterator =
eventSet.eventIterator();
> >
> > 178 if (totalWaitTime <= 0 || exit) {
> > 179 break;
> > 180 }
> > 181 debugee.resume();
> > 182 begin = System.currentTimeMillis();
> > 183 eventSet =
debugee.VM().eventQueue().remove(waitTime);
> > 184 delta = System.currentTimeMillis() -
begin;
> > 185 totalWaitTime -= delta;
> > 186 }
> >
> >
> > However, as I see now in case if a timeout happens on line 160
(eventSet is null) the loop is not executed at all. I haven't observed it in test
runs but I think it makes sense to adjust this test to take this potential case into
account. I will send an updated version of the patch soon.
> >
> > Thanks!
> >
> > Best regards,
> > Daniil
> >
> > On 2/25/19, 12:21 PM, "Chris Plummer" <[email protected]>
wrote:
> >
> > Hi Daniil,
> >
> > On 2/23/19 1:02 PM, Daniil Titov wrote:
> > > Please review the change that fixes timeout issues for the
following 10 tests when running with jtreg and default timeout factor (1.0).
> > In Utils.java, I think wait() should be moved right after
> > waitForCondition() and maybe given a more descriptive name.
It seems to
> > basically the same as waitForCondition(), except you added a
"log"
> > parameter and slightly changed the behavior. Are these
behavior
> > differences necessary? Could you share code with the existing
> > waitForCondition()?
> > >
> > > For the following 2 tests the event wait timeout was
reduced and adjusted for test.timeout.factor. Method receiveEvents(long,pattern) was
fixed to ensure that it gracefully exits after the specified wait period elapsed:
> > >
-vmTestbase/nsk/jdi/ClassUnloadRequest/addClassExclusionFilter/exclfilter001.java
> > >
-vmTestbase/nsk/jdi/ClassUnloadRequest/addClassFilter/filter001.java
> > 183 eventSet =
debugee.VM().eventQueue().remove(waitTime);
> >
> > This code used to wait for the total remaining waittime. Now
it waits a
> > fixed amount based on:
> >
> > 153 long waitTime = Utils.adjustTimeout(1000);
> >
> > How did you come up with this wait amount, and is it long
enough to deal
> > with occasional hiccups?
> > > For these 3 tests the event wait timeout was reduced and
adjusted for test.timeout.factor:
> > > -vmTestbase/nsk/jdi/Event/_itself_/event001.java
> > >
-vmTestbase/nsk/jdi/VirtualMachine/suspend/suspend001/TestDescription.java
> > >
-vmTestbase/nsk/jdi/ThreadReference/suspend/suspend001.java
> > So overall is this a shorter or longer waittime now?
> > >
> > > For next 2 tests the event wait timeout and the thread
sleep time were reduced and adjusted for test.timeout.factor. Additional synchronization
between the debugger and the debuggee was added to ensure the debugee process continues
as soon as the test finishes the timeout related checks and advances to the next steps:
> > > -
vmTestbase/nsk/jdi/EventQueue/remove_l/remove_l004/TestDescription.java
> > > -
vmTestbase/nsk/jdi/EventQueue/remove/remove004/TestDescription.java
> > Ok.
> > >
> > > Instead of just sleeping for 5 minutes while waiting for
the debuggee test thread to complete the tests now check whether the debuggee thread is
alive in the loop. The total waiting timeout was adjusted for test.timeout.factor:
> > >
-vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose004/TestDescription.java
> > >
-vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose003/TestDescription.java
> > >
-vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose002/TestDescription.java
> > Ok.
> >
> > thanks,
> >
> > Chris
> > >
> > > Testing.
> > > The following VM options were used in Mach5 jobs to
verify these changes:
> > > 1. No VM args
> > > 2. -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI
-XX:+TieredCompilation -XX:+UseJVMCICompiler -Djvmci.Compiler=grail
> > > 3. -Xcomp
> > >
> > > Also tier1, tier2 and tier3 Mach5 jobs succeeded.
> > >
> > > To verify that tests succeed with test.timeout.factor set
to 1.0 the following patch was used before running Mach5 jobs.
> > >
> > > --- a/make/RunTests.gmk Thu Feb 21 15:17:42 2019 -0800
> > > +++ b/make/RunTests.gmk Thu Feb 21 15:42:15 2019 -0800
> > > @@ -826,6 +826,7 @@
> > > else
> > > JTREG_TIMEOUT_FACTOR ?= 4
> > > endif
> > > + JTREG_TIMEOUT_FACTOR = 1
> > > JTREG_VERBOSE ?= fail,error,summary
> > > JTREG_RETAIN ?= fail,error
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8207367
> > > Webrev:
http://cr.openjdk.java.net/~dtitov/8207367/webrev.01
> > >
> > > Thanks!
> > > --Daniil
> > >
> > >
> >
> >
> >
> >
> >
>
>
>
>
>
