[9] Review request : JDK-8059070: [TESTBUG] java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java failed - timeout
Konstantin Shefov
konstantin.shefov at oracle.com
Thu Oct 16 10:03:44 UTC 2014
Paul,
Thanks for reviewing
In the jtreg scripts of the three existing LFCaching tests timeout is
set explicitly to 300 seconds. The file currently being changed is not a
test itself, it is parent class of tests.
In fact we can unset this explicit timeout and use the current fix
together with default JTREG timeout and jtreg timeout factor option.
These test cases are randomly generated, so the more iterations, the
better, but in fact we need at least one iteration.
As for "if (avgIterTime > 2 * remainTime)", I think 2 is enough.
-Konstantin
On 16.10.2014 13:30, Paul Sandoz wrote:
> On Oct 16, 2014, at 10:43 AM, Konstantin Shefov <konstantin.shefov at oracle.com> wrote:
>
>> Gently reminder
>>
>> On 14.10.2014 16:58, Konstantin Shefov wrote:
>>> Hello,
>>>
>>> Please review the test bug fix https://bugs.openjdk.java.net/browse/JDK-8059070
>>> Webrev is http://cr.openjdk.java.net/~kshefov/8059070/webrev.00/
>>>
> 45 private static final long TIMEOUT = 300000;
>
> Does jtreg define a system property for this value? and is 300s the same as what jtreg defines as the default?
>
> The online documentation (jtreg -onlineHelp) says the default is 2 minutes, but that might be out of date.
>
> Might be more readable to use:
>
> TimeUnit.SECONDS.toMillis(300);
>
>
> 143 long passedTime = new Date().getTime() - startTime;
>
> Why don't you use System.currentTimeMillis() instead of "new Date().getTime()" ?
>
>
> 145 double timeoutFactor = new Double(System.getProperty("test.timeout.factor", "1.0"));
>
> You can that pull out into a static and perhaps merge with TIMEOUT.
>
>
> 147 if (avgIterTime > 2 * remainTime) {
>
> That seems sufficient but it will be interesting to see if intermittent failures still occur due to high variance.
>
> Paul.
More information about the core-libs-dev
mailing list