8330814: Cleanups for KeepAliveCache tests

[RealCLanger]

While working in that area I found some potential for cleanup of a few tests.

Most notably:

B5045306.java:

• does not need to run in othervm
• the executor service that it uses should be shut down eventually to free resources

B8291637.java:

• use just one instead of two test VM invocations
  KeepAliveTimerThread.java:
  call to grp.destroy() at the end is pointless (API is void & deprecated for removal)

Generally:
The deprecated URL constructor is used. It can be switched to the handy URIBuilder
Some more try with resources here and there

---

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-8330814: Cleanups for KeepAliveCache tests (Bug - P4)

Reviewers

• Jaikiran Pai (@jaikiran - Reviewer) ⚠️ Review applies to 3296795b
• Daniel Fuchs (@dfuch - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18884

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

8330814: Cleanups for KeepAliveCache tests

Reviewed-by: jpai, dfuchs

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

• 957eb61: 8331429: [JVMCI] Cleanup JVMCIRuntime allocation routines
• 2f10a31: 8302850: Implement C1 clone intrinsic that reuses arraycopy code for primitive arrays
• c642f44: 8329839: Cleanup ZPhysicalMemoryBacking trace logging
• d04ac14: 8332236: javac crashes with module imports and implicitly declared class
• 4e77cf8: 8330795: C2: assert((uint)type <= T_CONFLICT && _zero_type[type] != nullptr) failed: bad type with -XX:-UseCompressedClassPointers
• 7b4ba7f: 8325932: Replace ATTRIBUTE_NORETURN with direct [[noreturn]]
• 0bb5ae6: 8332248: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java failed with RuntimeException
• 4d32c60: 8322008: Exclude some CDS tests from running with -Xshare:off
• e91492a: 8313674: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java should test for more block devices
• 95a6013: 8332042: Move MEMFLAGS to its own include file
• ... and 82 more: https://git.openjdk.org/jdk/compare/df1ff056f19ce569e05b0b87584e289840fc5d5c...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]

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

• net

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

• 06: Full - Incremental (7ff8a28b)
• 05: Full - Incremental (3296795b)
• 04: Full - Incremental (4a85d5b8)
• 03: Full - Incremental (42a55baa)
• 02: Full - Incremental (a89416f0)
• 01: Full - Incremental (92bddff8)
• 00: Full (ab993202)

[RealCLanger]

ping (@dfuch, @jaikiran) - any updates/approvals 😄

[jaikiran]

Hello Christoph, the latest changes look fine to me. I've added a trivial comment about reading the inputstream in one of the test, but you can leave it in its current form if you prefer to.
Please run tier2 tests once before integrating.

[RealCLanger]

Hello Christoph, the latest changes look fine to me. I've added a trivial comment about reading the inputstream in one of the test, but you can leave it in its current form if you prefer to. Please run tier2 tests once before integrating.

Thanks for the review. I added your suggestion. Will run it once more through our test suite tonight and then integrate tomorrow if all is good.

[dfuch]

Looks reasonable. Give us some time to test that in our CI.

[RealCLanger]

Looks reasonable. Give us some time to test that in our CI.

Our CI testing looks good. Please let me know when you are done.

[RealCLanger]

Looks reasonable. Give us some time to test that in our CI.

@dfuch May I merge now?

[RealCLanger]

If I don't hear back, I'll integrate tomorrow. Should we see problems later on, I'm available for following up. 😄

[dfuch]

@RealCLanger sorry about that: I ran the tests then got distracted and forgot to come back. You're good to go - no issue found!

[RealCLanger]

@RealCLanger sorry about that: I ran the tests then got distracted and forgot to come back. You're good to go - no issue found!

Cool. Thanks for the confirmation.

/integrate

[openjdk]

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

• 1a94447: 8332111: [BACKOUT] A way to align already compiled methods with compiler directives
• 957eb61: 8331429: [JVMCI] Cleanup JVMCIRuntime allocation routines
• 2f10a31: 8302850: Implement C1 clone intrinsic that reuses arraycopy code for primitive arrays
• c642f44: 8329839: Cleanup ZPhysicalMemoryBacking trace logging
• d04ac14: 8332236: javac crashes with module imports and implicitly declared class
• 4e77cf8: 8330795: C2: assert((uint)type <= T_CONFLICT && _zero_type[type] != nullptr) failed: bad type with -XX:-UseCompressedClassPointers
• 7b4ba7f: 8325932: Replace ATTRIBUTE_NORETURN with direct [[noreturn]]
• 0bb5ae6: 8332248: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java failed with RuntimeException
• 4d32c60: 8322008: Exclude some CDS tests from running with -Xshare:off
• e91492a: 8313674: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java should test for more block devices
• ... and 83 more: https://git.openjdk.org/jdk/compare/df1ff056f19ce569e05b0b87584e289840fc5d5c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@RealCLanger Pushed as commit a5005c8.

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