jdk8u-dev
jdk8u-dev copied to clipboard
8217612: (CL)HSDB cannot show some JVM flags
Hi all, this is a backport of JDK-8217612: (CL)HSDB cannot show some JVM flags. The bug reported is reproducible in JDK8, so this patch should be applied. This patch requires the follow-up patch JDK-8217850 and the correspoding pull request has been submitted. The original patch does not apply cleanly, and the following modifications are needed:
hotspot/agent/src/share/classes/sun/jvm/hotspot/runtime/VM.java
- Long.toUnsignedString method was introduced in JDK8, and the build fails with JDK7 as the boot-jdk when using this method. Consequently, an alternative private method to Long.toUnsignedString(long i, 10) is required in VM.java.
- The size_t type was introduced in JDK9, so the related fix is skipped.
- To return a String value in the getValue method, the original fix uses a static method, whereas this fix creates a new instance each time. The use of a static method was introduced to JDK-9 in the enhancement JDK-8145061. This enhancement has not been backported, so the same format as the other part of the getValue() method should be used.
- Replace var with String.
hotspot/src/share/vm/runtime/globals.hpp
- A comment on the flag type double and uint64_t is added, which matches teh implementation. This comment was added in JDK-8059847, but this fix is an enhancement and not necessary to backport, so I believe it is valid to fix the comment in this PR.
hotspot/test/serviceability/sa/ClhsdbFlags.java
- This test is backported from the original commit with
@testremoved. This test will not function because the serviceability/sa test framework has not been backported. This follows the discussion in backporting JDK-8196969 to JDK8 (https://mail.openjdk.org/pipermail/jdk8u-dev/2020-May/011785.html ). Backporting SA-related tests is excessive and may require some follow-up test fixes, but it is beneficial to indicate that this test append should be integrated when the test framework is backported in the future.
Testing
- manually check the behaviour
Consider Running java program with option-XX:NativeMemoryTracking=off -XX:OnError=echo -XX:MaxRAMPercentage=20.0 -XX:MaxRAM=10000000. The flag types correspond to ccstr, ccstrlist, double, and uint64_t, respectively. When attaching the process using clhsdb, the value is corrected by this patch as follows. Note that the default value of MaxMetaspaceSize is max_uintx.
without patch
hsdb> flags NativeMemoryTracking
NativeMemoryTracking = null 1
hsdb> flags OnError
OnError = null 1
hsdb> flags MaxRAMPercentage
MaxRAMPercentage = null 1
hsdb> flags MaxRAM
MaxRAM = null 1
hsdb> flags MaxMetaspaceSize
MaxMetaspaceSize = -4096 0
with patch
hsdb> flags NativeMemoryTracking
NativeMemoryTracking = "off" 1
hsdb> flags OnError
OnError = "echo" 1
hsdb> flags MaxRAMPercentage
MaxRAMPercentage = 20.0 1
hsdb> flags MaxRAM
MaxRAM = 10000000 1
hsdb> flags MaxMetaspaceSize
MaxMetaspaceSize = 18446744073709547520 0
- hotspot_tier1 after applying the follow-up patch JDK-8217850
Thank you.
Progress
- [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [ ] JDK-8217612 needs maintainer approval
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8217612: (CL)HSDB cannot show some JVM flags (Bug - P4 - Requested)
Reviewers
- Paul Hohensee (@phohensee - Reviewer)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/431/head:pull/431
$ git checkout pull/431
Update a local copy of the PR:
$ git checkout pull/431
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/431/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 431
View PR using the GUI difftool:
$ git pr show -t 431
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/431.diff
Using Webrev
:wave: Welcome back ktakakuri! 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.
@ktakakuri 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!
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
Could anyone review this backport please? Failure of gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java in GHA will be resolved in https://github.com/openjdk/jdk8u-dev/pull/433.
@ktakakuri 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!
Can someone please review this backport?
@ktakakuri 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!
Could anyone please review this fix?
@ktakakuri 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!
Could anyone review this backport please? Failure of gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java in GHA will be resolved in https://github.com/openjdk/jdk8u-dev/pull/433.
In my environment, I am building JDK8 using JDK7 according to the N-1 rule. As for me, I would like to support builds using JDK7.
I added a comment.
⚠️ @ktakakuri 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.
/approval request This bug is reproducible in JDK8, and backporting this patch is needed. The patch does not apply cleanly due to enhancements introduced since JDK9 and the method that can't be used for building with JDK7. Low risk, tool bug fix. Backport requires follow-up issue JDK-8217850 and the correspoding backport has been submitted. Tesing: GHA, hotspot_tier1, and manual behaviour checks.
@ktakakuri 8217612: The approval request has been created successfully.
@ktakakuri 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!
This pull request is pending approval of the Fix Request. I comment to not close.