jdk17u-dev
jdk17u-dev copied to clipboard
8315362: NMT: summary diff reports threads count incorrectly
Progress
- [ ] 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
- [ ] JDK-8315362 needs maintainer approval
Issue
- JDK-8315362: NMT: summary diff reports threads count incorrectly (Bug - P4)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/2084/head:pull/2084
$ git checkout pull/2084
Update a local copy of the PR:
$ git checkout pull/2084
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/2084/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2084
View PR using the GUI difftool:
$ git pr show -t 2084
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/2084.diff
Webrev
:wave: Welcome back zer0chance! 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.
This backport pull request has now been updated with issue from the original commit.
Webrevs
- 03: Full (f6475379)
- 02: Full - Incremental (afc89786)
- 01: Full - Incremental (fbe0e219)
- 00: Full (d4ccc8b1)
@zer0chance This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
Adding comment to keep this PR alive. @GoeLin do I need to do anything else to get your approval on this?
@zer0chance This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@zer0chance 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:
8315362: NMT: summary diff reports threads count incorrectly
Reviewed-by: mdoerr
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 master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.
As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@TheRealMDoerr) but any other Committer may sponsor as well.
➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).
@zer0chance This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@zer0chance This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.
/open
@zer0chance This pull request is now open
This PR shouldn't differ from the 21u version: https://github.com/openjdk/jdk21u/commit/31c759dd7ef69449e225c3e656d8f522e1124852 I currently see the following diffs:
< + _thread_count ++;
---
> + _thread_count++;
< + output.shouldContain("Baseline succeeded");
---
> + output.shouldContain("Baseline taken");
This PR shouldn't differ from the 21u version: openjdk/jdk21u@31c759d I currently see the following diffs:
< + _thread_count ++; --- > + _thread_count++;< + output.shouldContain("Baseline succeeded"); --- > + output.shouldContain("Baseline taken");
For _thread_count is a good catch, but we need to modify the test like this because JDK-8289182 was not backported to the JDK 17 and jcmd VM.native_memory baseline outputs Baseline succeeded not Baseline taken
⚠️ @zer0chance This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.
Should be adapted if https://github.com/openjdk/jdk17u-dev/pull/2415 gets approved and integrated.
Thank you @TheRealMDoerr! Can I please ask you to take a look to the follow up fix backport https://github.com/openjdk/jdk17u-dev/pull/2085?
/approval
@zer0chance usage: /approval [<id>] (request|cancel) [<text>]
/approval request
@zer0chance 8315362: The approval request has been created successfully.
https://github.com/openjdk/jdk17u-dev/pull/2085 is clean and trivial. It doesn't need a review. However, all backports need maintainer approvals (see https://wiki.openjdk.org/display/JDKUpdates/JDK+17u). We typically describe in the approval request the motivation for doing the backport, how risky it is and how we tested it. Having said this, please enable GitHub Actions to get automatic tests running on your PRs. I have requested approval for https://github.com/openjdk/jdk17u-dev/pull/2415, so that one should get integrated first and this one become a clean backport of the JDK 21 version.
Both PRs got maintainer approval. Please change to "Baseline taken" once https://github.com/openjdk/jdk17u-dev/pull/2415 is integrated.
@zer0chance this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout pr/8315362-backport
git fetch https://git.openjdk.org/jdk17u-dev.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
@TheRealMDoerr test is adjusted
/integrate
@zer0chance Your change (at version f647537911374745a217158eddaafa1cd3df73f3) is now ready to be sponsored by a Committer.
Ok, you could have made it clean by naming it "Backport 31c759dd7ef69449e225c3e656d8f522e1124852" which makes it a backport of the JDK 21 version. But, let's ship it! /sponsor
Going to push as commit a058ad151a7c68f65e915856b089ce4064c1caf8.
Since your change was applied there have been 8 commits pushed to the master branch:
- 9eb970264a34c82c11468b15fae89dc39c574682: 8331164: createJMHBundle.sh download jars fail when url needed to be redirected
- 46ba9bbe7e93c561b5af833ff164596955302a9d: 8318479: [jmh] the test security.CacheBench failed for multiple threads run
- 1de0c0a805f68914aec7358fc4aa9c9d2499ec98: 8256291: RunThese30M fails "assert(_class_unload ? true : ((((JfrTraceIdBits::load(class_loader_klass)) & ((1 << 4) << 8)) != 0))) failed: invariant"
- eebb80c4bcec64dc70065b597688cbe12c892a5b: 8317288: [macos] java/awt/Window/Grab/GrabTest.java: Press on the outside area didn't cause ungrab
- cc0c14380337f868d63f4ab746e4746a5dc2f115: 8310108: Skip ReplaceCriticalClassesForSubgraphs when EnableJVMCI is specified
- 7a8cf6c8aec6cd648663def62d9496e516f05d4f: 8319713: Parallel: Remove PSAdaptiveSizePolicy::should_full_GC
- 515bc9a264c1e44cfcada0405a74ab2fba428e10: 8318986: Improve GenericWaitBarrier performance
- dcb2c35151a846b7c44045235b389baf9713885c: 8295111: dpkg appears to have problems resolving symbolically linked native libraries
Your commit was automatically rebased without conflicts.
@TheRealMDoerr @zer0chance Pushed as commit a058ad151a7c68f65e915856b089ce4064c1caf8.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.