8369491: Temporarily revert default TIMEOUT_FACTOR back to 4

[stefank]

JDK-8260555 changed the default TIMEOUT_FACTOR from 1 to 4 to make it easier to understand how our timeouts worked. Together with that small change, we also bumped a number of tests that relied on the previous extended timeout. The anticipation was that there would be some fallout from that change and that we for a short time after it had been integrated would have to tweak some tests that intermittently needed a larger timeout.

It turns out that the way we run tests concurrently causes the tests to sometimes run in a resource-constrained manner. This has the effect that even simple tests that usually don't take a long time to execute run the risk of hitting the default 120s timeout limit. This invalidates the earlier plan to fix the additional, few tests that now times out, because it's probably not the tests themselves that are the problem, but rather how we run the tests.

Hunting down the reasons for the new set of timeouts we are seeing is good and figuring out how to fix our testing to not over-strain our testing machines is also something that we want to do. The problem is that there's enough of these timeouts that it affects more than just a limited set of JDK devs.

The proposal is that we, for now, revert back to the default timeout factor of 4 to relive the pressure to investigate and fix these intermittent timeouts. And revert back to 1 once enough investigation and tweaks have been made to the test infrastructure.

I will run this through Oracle's tier1-tier8 testing.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8369491: Temporarily revert default TIMEOUT_FACTOR back to 4 (Enhancement - P4)

Reviewers

• Leo Korinth (@lkorinth - Reviewer)
• Christian Stein (@sormuras - Committer)
• Jaikiran Pai (@jaikiran - Reviewer)
• SendaoYan (@sendaoYan - Committer)
• Sergey Bylokhov (@mrserb - Reviewer)
• Phil Race (@prrace - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27721/head:pull/27721
$ git checkout pull/27721

Update a local copy of the PR:
$ git checkout pull/27721
$ git pull https://git.openjdk.org/jdk.git pull/27721/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27721

View PR using the GUI difftool:
$ git pr show -t 27721

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27721.diff

Using Webrev

Link to Webrev Comment

[stefank]

/label add jdk

[bridgekeeper]

👋 Welcome back stefank! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@stefank This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8369491: Temporarily revert default TIMEOUT_FACTOR back to 4

Reviewed-by: lkorinth, cstein, jpai, syan, serb, prr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 96 new commits pushed to the master branch:

• 1bd814c: 8364660: ClassVerifier::ends_in_athrow() should be removed
• 784af43: 8351595: JVM_FindClassFromCaller: unused var may be removed
• 5191d72: 8369894: Remove javax/management/remote/mandatory/loading/RMIDownloadTest.java from problemlist
• ... and 93 more: https://git.openjdk.org/jdk/compare/af2fbd5a7182cabdd88764b5653d2ce666f05d70...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@stefank
The jdk label was successfully added.

[openjdk]

@stefank The following label will be automatically applied to this pull request:

• build

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (618f78ed)

[lkorinth]

Looks good to me.

[jaikiran]

This looks good to me.

[sendaoYan]

Glad to see this proposed change.

[prrace]

I don't have much visibility into how this affected other areas but I was surprised that a test that for me runs in a couple of seconds was apparently timing out on some systems (not the Oracle CI but some external one).
In the PR to resolve this I suggested (#27397 (comment))
that we may be using too high a concurrency as chosen by the build as tests could be more sensitive to this.
But I also see that if you have a system with N CPUs it is a waste to only use (eg) N/2 because "some" test might time out.
A compromise may be needed.

[stefank]

Tier1-8 testing passed. My intention is to get this integrated tomorrow.

[stefank]

Thanks for the reviews!
/integrate

[openjdk]

Going to push as commit 5fc3904.
Since your change was applied there have been 111 commits pushed to the master branch:

• ead35a7: 8358942: HttpClient adds Content-Length: 0 for a GET request with a BodyPublishers.noBody()
• d6c122b: 8369982: ProblemList jdk/jfr/jvm/TestWaste.java
• 6e911d8: 8368205: [TESTBUG] VectorMaskCompareNotTest.java crashes when MaxVectorSize=8
• ... and 108 more: https://git.openjdk.org/jdk/compare/af2fbd5a7182cabdd88764b5653d2ce666f05d70...master

Your commit was automatically rebased without conflicts.

[openjdk]

@stefank Pushed as commit 5fc3904.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.