RFR: 8260555: Change the default TIMEOUT_FACTOR from 4 to 1 [v5]

Fri Aug 22 16:10:58 UTC 2025

On Wed, 20 Aug 2025 17:05:59 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:

>> This changes the timeout factor from 4 to 1. Most of the changes add timeouts to individual test cases so that I am able to run them with a timeout factor of 0.7 (some margin to the checked in factor of one)
>> 
>> In addition to changing the timeout factor, I am also using a library call to parse the timeout factor from the Java properties (I cannot use the library function everywhere as jtreg does not allow me to add @library notations to non test case files).
>> 
>> My approach has been to run all tests, and afterwards updating those that fail due to the timeout factor. The amount of updated test cases is huge, and my strategy has been to quadruple the timeout if I could not directly see that less was needed. In a few places, I have added a bit more timeout so that it will work with the 0.7 timeout factor.
>> 
>> These fixes have been created when I have ploughed through test cases:
>> JDK-8352719: Add an equals sign to the modules statement
>> JDK-8352709: Remove bad timing annotations from WhileOpTest.java
>> JDK-8352074: Test MemoryLeak.java seems not to test what it is supposed to test
>> CODETOOLS-7903937: JTREG uses timeout factor on socket timeout but not on KEEPALIVE
>> CODETOOLS-7903961: Make default timeout configurable
>> 
>> After the review, I will update the copyrights.
>> 
>> I have run testing tier1-8. The last time with a timeout factor of 1 instead of 0.7.
>> 
>> I got 4 timing related faults:
>> 1) runtime/Thread/TestThreadDumpMonitorContention.java
>>    This is probably: https://bugs.openjdk.org/browse/JDK-8361370
>> 2) sun/tools/jhsdb/BasicLauncherTest.java
>>    I am unsure about this one, it has not failed on my runs before, even with a timeout factor of 0.7, maybe I was unlucky.
>> 3) gc/stress/TestReclaimStringsLeaksMemory.java
>>    I updated this to 480 seconds, I finish this fairly fast (~14s) on my not very fast computer, but the Macs that fail are old x86-based ones.
>> 4) sun/security/ssl/X509KeyManager/CertChecking.java
>>    This is a new test that I got on last rebase. I have added a timeout of 480 to it.
>> 
>> In addition to these four tests, I have another one "java/lang/ThreadLocal/MemoryLeak.java" that earlier failed with a timeout factor of 0.7 but did not fail in the last run. I will *not* update that test case, because the extra time spent is strange and should be looked at. I have created JDK-8352074 on that test case, and I will probably create another bug describing the timeout I got.
>> 
>> From the review of the cancelled "8356171: ...
>
> Leo Korinth has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update testing.md, remove makefile link, fix bad text

> Before the default of 120 was multiplied by the timeoutFactor of 4 to given 480. Now the value 480 is multiplied by the timeoutFactor of 1 to give 480.

I identified some cases that doesn't follow this. Unclear whether they are intentional.

test/hotspot/jtreg/compiler/tiered/Level2RecompilationTest.java line 36:

> 34:  * @build jdk.test.whitebox.WhiteBox
> 35:  * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
> 36:  * @run main/othervm/timeout=960 -Xbootclasspath/a:. -XX:+TieredCompilation

Why not `480` in this case?

test/hotspot/jtreg/runtime/cds/appcds/LotsOfSyntheticClasses.java line 40:

> 38:  * @requires vm.cds
> 39:  * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
> 40:  * @run driver/timeout=8000 LotsOfSyntheticClasses

I was expecting `500 * 4 = 2000`, instead of `8000` here.

test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendResume2/SuspendResume2.java line 31:

> 29:  * @compile SuspendResume2.java
> 30:  * @run driver jdk.test.lib.FileInstaller . .
> 31:  * @run main/othervm/native/timeout=700

Why `700` instead of `480` in this file?

test/jdk/java/rmi/transport/dgcDeadLock/DGCDeadLock.java line 59:

> 57: public class DGCDeadLock implements Runnable {
> 58:     final static public int HOLD_TARGET_TIME = 25000;
> 59:     public static final double TEST_FAIL_TIME = (HOLD_TARGET_TIME + 30000) * Math.max(TestLibrary.getTimeoutFactor(), 4);

Why `max(...)`? If it's for backwards compatibility, shouldn't it be `(HOLD_TARGET_TIME + 30000)  * 4 * TestLibrary.getTimeoutFactor()`?

test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 60:

> 58:  * @comment skip running this test on 32 bit VM
> 59:  * @requires vm.bits == "64"
> 60:  * @run testng/othervm/timeout=960 -Xmx2g WhiteBoxResizeTest

Why not `480`?

test/jdk/java/util/PluggableLocale/LocaleNameProviderTest.java line 34:

> 32:  * @build com.foobar.Utils
> 33:  *        com.bar.*
> 34:  * @run junit/othervm/timeout=960 -Djava.locale.providers=CLDR,SPI LocaleNameProviderTest

Why not `480`?

test/jdk/jdk/jfr/event/oldobject/TestObjectDescription.java line 49:

> 47:  * @library /test/lib /test/jdk
> 48:  * @modules jdk.jfr/jdk.jfr.internal.test
> 49:  * @run main/othervm/timeout=960 -XX:TLABSize=2k jdk.jfr.event.oldobject.TestObjectDescription

Why not `480`?

test/jdk/tools/jpackage/share/InstallDirTest.java line 69:

> 67:  * @compile -Xlint:all -Werror InstallDirTest.java
> 68:  * @requires (jpackage.test.SQETest != null)
> 69:  * @run main/othervm/timeout=4000 -Xmx512m jdk.jpackage.test.Main

Why more than `4x` in this file?

test/langtools/jdk/jshell/HangingRemoteAgent.java line 38:

> 36: class HangingRemoteAgent extends RemoteExecutionControl {
> 37: 
> 38:     private static final int TIMEOUT = (int)(2000 * Double.parseDouble(System.getProperty("test.timeout.factor", "1.0")));

why not `Utils.TIMEOUT_FACTOR`?

test/langtools/jdk/jshell/UITesting.java line 148:

> 146:     }
> 147: 
> 148:     private static final long TIMEOUT = (long) (60_000 * Double.parseDouble(System.getProperty("test.timeout.factor", "1.0")));

Why not `Utils.TIMEOUT_FACTOR`?

-------------

PR Review: https://git.openjdk.org/jdk/pull/26749#pullrequestreview-3144985957
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294077875
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294085201
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294089550
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294108202
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294110136
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294113670
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294116148
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294119800
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294128741
PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294129243