-
Notifications
You must be signed in to change notification settings - Fork 5.8k
8307783: runtime/reflect/ReflectOutOfMemoryError.java timed out #14070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…kenConnectionTest.java on linux-all
…gClassTest.java on windows-x64 w/ loom
…nloadTest.java on windows-x64 w/ loom
…a with virtual threads
👋 Welcome back dcubed! A progress list of the required criteria for merging this PR into |
@dcubed-ojdk The following label will be automatically applied to this pull request:
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. |
/issue JDK-8307783 |
@dcubed-ojdk The primary solved issue for a PR is set through the PR title. Since the current title does not contain an issue reference, it will now be updated. |
/label add serviceability |
@dcubed-ojdk |
@dcubed-ojdk |
@dcubed-ojdk |
@dcubed-ojdk |
@dcubed-ojdk |
@dcubed-ojdk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small proposal inline.
@@ -25,7 +25,7 @@ | |||
* @test | |||
* @bug 8297977 | |||
* @summary Test that throwing OOM from reflected method gets InvocationTargetException | |||
* @run main/othervm ReflectOutOfMemoryError | |||
* @run main/othervm/timeout=150 ReflectOutOfMemoryError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it make a sense to exclude this test from tier1, if it might took more than 2 minutes to complete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not that simple. Here's my comment from the bug:
In jdk-21+24-2010 (which has finished Tier[1-7]), there are 79 passing runs of
this test and the times range from 31s -> 7m 37s. The slowest is on linux-arch64,
but the fastest linux-aarch64 is 44s so it's not simply that linux-aarch64 is slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know why this test do not set Xmx to some reasonable value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lmesnik - Thanks for fast review! Especially for a Saturday AM... /integrate |
@dcubed-ojdk This pull request has not yet been marked as ready for integration. |
Oops... forgot to mark it ready for review. |
@dcubed-ojdk 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:
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 no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
/integrate |
Going to push as commit a0f4a94. |
@dcubed-ojdk Pushed as commit a0f4a94. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
A trivial fix to increase a timeout:
JDK-8307783 runtime/reflect/ReflectOutOfMemoryError.java timed out
Trivial fixes to ProblemList tests:
JDK-8308468 ProblemList containers/docker/TestMemoryAwareness.java on linux-x64
JDK-8308470 ProblemList javax/management/remote/mandatory/connection/BrokenConnectionTest.java on linux-all
JDK-8308471 ProblemList javax/management/remote/mandatory/loading/MissingClassTest.java on windows-x64 w/ loom
JDK-8308472 ProblemList javax/management/remote/mandatory/loading/RMIDownloadTest.java on windows-x64 w/ loom
JDK-8308473 ProblemList java/lang/instrument/NativeMethodPrefixAgent.java with loom
Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14070/head:pull/14070
$ git checkout pull/14070
Update a local copy of the PR:
$ git checkout pull/14070
$ git pull https://git.openjdk.org/jdk.git pull/14070/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 14070
View PR using the GUI difftool:
$ git pr show -t 14070
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14070.diff