8360160: ubuntu-22-04 machine is failing client tests

[DamonGuy]

There was a prior PR that attempted to fix this issue, but it was found that JDK-8280987 was the more reasonable cause. The issue was caused by Ubuntu instances having a large amount of graphics configurations, hence a large amount of concurrent frames being open and tested on at a time. As such, the newly suggested fix was to instead break apart the test into batches.

This change creates and disposes of the test frames in batches of 20, up to the number of GC's identified. The original problem was recreated when testing on Ubuntu 24.04, and I have re-tested this with the updated test on the same device, and it passes. When debugging, 140 GC's were identified and all 140 frames were created and disposed of. Also tested this on macOS 15 and the test also passes. CI looks good as well.

---

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-8360160: ubuntu-22-04 machine is failing client tests (Bug - P4)

Reviewers

• Phil Race (@prrace - Reviewer)
• Alexander Zvegintsev (@azvegint - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28646

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back dnguyen! 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]

@DamonGuy 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:

8360160: ubuntu-22-04 machine is failing client tests

Reviewed-by: prr, azvegint

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 331 new commits pushed to the master branch:

• 629bf20: 8371408: [Linux] VM.info output for container information is confusing
• 3559eec: 8373428: Refine variables with the same name in nested scopes in PhaseChaitin::gather_lrg_masks
• ad6611a: 8371347: Move the ObjectMonitorTable to a separate new file
• ... and 328 more: https://git.openjdk.org/jdk/compare/2d815ac61b437835163d4014bfed16f0f3faea50...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]

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

• client

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

• 03: Full - Incremental (f0e7302a)
• 02: Full - Incremental (7f623fc7)
• 01: Full - Incremental (a493bce6)
• 00: Full (865dc7a2)

[prrace]

maybe we could have a var for this ? Perhaps even allow it to be set as a program arg even if you default to 20.
Also wouldn't one at a time be the most stable ?
Finally, is it even necessary to test ALL of them ? Isn't it sufficient to test 30-40 and call it a day ?

[DamonGuy]

Good point. I have switched to a single frame at a time and have created a new final var to limit the number of frames to test (30). When testing, the balance of stability and testing duration felt reasonable. The test still creates, tests, and disposes multiple frames on Ubuntu, and the test still passes testing after the changes.

[azvegint]

I have switched to a single frame at a time

This makes the test much slower. On my machine, the execution time is::

• ~33s with the one at a time version
• ~10s for version with all gcs in batches of 20

Few notes on placing several frames:

• If they are close enough, they can cast shadows on each other.
• If the auto-hide dock is disabled, the dock can overlap frames. The dock auto-hide is enabled on the CI, but it would be better if it could be moved away from the screen border.

[DamonGuy]

Is 33 seconds unreasonable for the test? I figured since the upper-limit of frame tests is set to 30, this 33 seconds would be the longest the test would run for. Also, the test only has 1 gc for other OS's so only 1 frame is tested for that. This avoids the shadow issue as well as creates a more stable test.

[prrace]

I will take stability at the cost of some extra time.

[azvegint]

Is 33 seconds unreasonable for the test? I figured since the upper-limit of frame tests is set to 30, this 33 seconds would be the longest the test would run for. Also, the test only has 1 gc for other OS's so only 1 frame is tested for that.

In CI it may be even slower, and we can achieve the same results in a third of the time with showing several windows at once. This can cumulatively save a lot of CI time over many runs. We can still apply the same upper limit to the frame count of frame tests, and this may make it even faster.

This avoids the shadow issue as well as creates a more stable test.

This can be mitigated simply by adding spacing between the frames.

[DamonGuy]

I suppose if this is problematic, I can change it back. But right now it looks OK and the code implementation is cleaner and easier to read this way. Thanks!

[DamonGuy]

/integrate

[openjdk]

Going to push as commit 317788f.
Since your change was applied there have been 339 commits pushed to the master branch:

• 6aeabd4: 8370910: Cleanup terminology of UUID vs Global Identifiers in UUID
• f52d499: 8319589: Attach from root to a user java process not supported in Mac
• 45ee89c: 8373297: Test com/sun/jdi/AfterThreadDeathTest.java failed with unexpected ObjectCollectedException
• ... and 336 more: https://git.openjdk.org/jdk/compare/2d815ac61b437835163d4014bfed16f0f3faea50...master

Your commit was automatically rebased without conflicts.

[openjdk]

@DamonGuy Pushed as commit 317788f.

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