jdk icon indicating copy to clipboard operation
jdk copied to clipboard

Published 20 hours ago • verified publisher icon openjdk

8330814: Cleanups for KeepAliveCache tests

Open RealCLanger opened this issue 1 year ago • 8 comments

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

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

Issue

  • JDK-8330814: Cleanups for KeepAliveCache tests (Bug - P4)

Reviewers

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

RealCLanger avatar Apr 22 '24 11:04 RealCLanger

:wave: 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.

bridgekeeper[bot] avatar Apr 22 '24 11:04 bridgekeeper[bot]

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

  • 957eb611ce2531a3fcc764813ad1e0776887fdda: 8331429: [JVMCI] Cleanup JVMCIRuntime allocation routines
  • 2f10a316ff0c5a4c124b94f6fabb38fb119d2c82: 8302850: Implement C1 clone intrinsic that reuses arraycopy code for primitive arrays
  • c642f44bbe1e4cdbc23496a34ddaae30990ce7c0: 8329839: Cleanup ZPhysicalMemoryBacking trace logging
  • d04ac14bdbab4187d0be98b8471f90be8a14f649: 8332236: javac crashes with module imports and implicitly declared class
  • 4e77cf881d031e5b0320915b3eabd7702e560291: 8330795: C2: assert((uint)type <= T_CONFLICT && _zero_type[type] != nullptr) failed: bad type with -XX:-UseCompressedClassPointers
  • 7b4ba7f90ab9ea5e1070c79534c587dad17d1bdd: 8325932: Replace ATTRIBUTE_NORETURN with direct [[noreturn]]
  • 0bb5ae645165b97527ecccf02308df6072c363d8: 8332248: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java failed with RuntimeException
  • 4d32c607a4b496bf2bb09e54167ecbbab5569a0c: 8322008: Exclude some CDS tests from running with -Xshare:off
  • e91492ab4333c61f39b50eb428fa932131a5b908: 8313674: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java should test for more block devices
  • 95a601316de06b4b0fbf6e3c7777be5d2a1ca978: 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[bot] avatar Apr 22 '24 11:04 openjdk[bot]

@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.

openjdk[bot] avatar Apr 22 '24 11:04 openjdk[bot]

Webrevs

mlbridge[bot] avatar Apr 22 '24 11:04 mlbridge[bot]

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

RealCLanger avatar May 07 '24 06:05 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.

RealCLanger avatar May 07 '24 08:05 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 avatar May 08 '24 08:05 RealCLanger

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

@dfuch May I merge now?

RealCLanger avatar May 13 '24 12:05 RealCLanger

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

RealCLanger avatar May 15 '24 08:05 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!

dfuch avatar May 15 '24 09:05 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!

Cool. Thanks for the confirmation.

/integrate

RealCLanger avatar May 15 '24 10:05 RealCLanger

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

  • 1a944478a26a766f5a573a1236b642d8e7b0685c: 8332111: [BACKOUT] A way to align already compiled methods with compiler directives
  • 957eb611ce2531a3fcc764813ad1e0776887fdda: 8331429: [JVMCI] Cleanup JVMCIRuntime allocation routines
  • 2f10a316ff0c5a4c124b94f6fabb38fb119d2c82: 8302850: Implement C1 clone intrinsic that reuses arraycopy code for primitive arrays
  • c642f44bbe1e4cdbc23496a34ddaae30990ce7c0: 8329839: Cleanup ZPhysicalMemoryBacking trace logging
  • d04ac14bdbab4187d0be98b8471f90be8a14f649: 8332236: javac crashes with module imports and implicitly declared class
  • 4e77cf881d031e5b0320915b3eabd7702e560291: 8330795: C2: assert((uint)type <= T_CONFLICT && _zero_type[type] != nullptr) failed: bad type with -XX:-UseCompressedClassPointers
  • 7b4ba7f90ab9ea5e1070c79534c587dad17d1bdd: 8325932: Replace ATTRIBUTE_NORETURN with direct [[noreturn]]
  • 0bb5ae645165b97527ecccf02308df6072c363d8: 8332248: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java failed with RuntimeException
  • 4d32c607a4b496bf2bb09e54167ecbbab5569a0c: 8322008: Exclude some CDS tests from running with -Xshare:off
  • e91492ab4333c61f39b50eb428fa932131a5b908: 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[bot] avatar May 15 '24 10:05 openjdk[bot]

@RealCLanger Pushed as commit a5005c87c4d5598eb54e9824105767d833f9660b.

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

openjdk[bot] avatar May 15 '24 10:05 openjdk[bot]