jdk8u-dev icon indicating copy to clipboard operation
jdk8u-dev copied to clipboard

8217612: (CL)HSDB cannot show some JVM flags

Open ktakakuri opened this issue 1 year ago • 30 comments

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

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

Link to Webrev Comment

ktakakuri avatar Jan 25 '24 11:01 ktakakuri

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

bridgekeeper[bot] avatar Jan 25 '24 11:01 bridgekeeper[bot]

This backport pull request has now been updated with issue from the original commit.

openjdk[bot] avatar Jan 25 '24 11:01 openjdk[bot]

Webrevs

mlbridge[bot] avatar Jan 25 '24 11:01 mlbridge[bot]

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

bridgekeeper[bot] avatar Feb 22 '24 13:02 bridgekeeper[bot]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

openjdk[bot] avatar Mar 13 '24 20:03 openjdk[bot]

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 avatar Apr 10 '24 11:04 ktakakuri

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

bridgekeeper[bot] avatar May 08 '24 17:05 bridgekeeper[bot]

Can someone please review this backport?

ktakakuri avatar May 21 '24 08:05 ktakakuri

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

bridgekeeper[bot] avatar Jun 18 '24 12:06 bridgekeeper[bot]

Could anyone please review this fix?

ktakakuri avatar Jun 23 '24 23:06 ktakakuri

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

bridgekeeper[bot] avatar Jul 21 '24 23:07 bridgekeeper[bot]

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 avatar Aug 05 '24 12:08 ktakakuri

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.

ktakakuri avatar Aug 06 '24 13:08 ktakakuri

I added a comment.

ktakakuri avatar Aug 19 '24 12:08 ktakakuri

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

openjdk[bot] avatar Aug 19 '24 15:08 openjdk[bot]

/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 avatar Aug 20 '24 06:08 ktakakuri

@ktakakuri 8217612: The approval request has been created successfully.

openjdk[bot] avatar Aug 20 '24 06:08 openjdk[bot]

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

bridgekeeper[bot] avatar Sep 17 '24 11:09 bridgekeeper[bot]

This pull request is pending approval of the Fix Request. I comment to not close.

ktakakuri avatar Sep 18 '24 10:09 ktakakuri