7506: Incorrect numeric formatting of PID by JMC
This PR is to fix the formatting issue of the JVM PID on JVM information screen. JVM PID attribute is received as NUMBER from JFR and hence the JMC application is treating is as number. When the number is smaller , the PID is displayed using commas and when the number is larger, its converted to exponential format. This is a long pending bug in product.
As part of this PR we have introduced a new aggregator JVM_PID_ID which will contain the value of Identifier in plain text format instead of number format. This new aggregator is displayed on JVM Information page so that formatting is not applied as per NUMBER formatting rules.
JVM PID before the change:
JVM PID after the change:
The issue is more prominent when the identifier value is converted to exponential.
Progress
- [x] Commit message must refer to an issue
- [ ] Change must be properly reviewed (1 review required, with at least 1 Committer)
Issue
- JMC-7506: Incorrect numeric formatting of PID by JMC (Bug - P3)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmc.git pull/557/head:pull/557
$ git checkout pull/557
Update a local copy of the PR:
$ git checkout pull/557
$ git pull https://git.openjdk.org/jmc.git pull/557/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 557
View PR using the GUI difftool:
$ git pr show -t 557
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jmc/pull/557.diff
Webrev
:wave: Welcome back schaturvedi! 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.
@Suchitainf 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:
7506: Incorrect numeric formatting of PID by JMC
Reviewed-by: bdutheil
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 3 new commits pushed to the master branch:
- 92750098a2deac54b8165ec5c6d368be00707856: 8220: Make LocalJVMToolkit reflective
- 57e386adb82b81d228a5f5ab114ef02c28cc8930: 7948: Update ASM version in the Agent
- 12dba597abd009f41e97f47ac4197f0c5ee9fc0e: 8212: Update trunk to 9.1.0
Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.
Webrevs
- 04: Full - Incremental (e488abb2)
- 03: Full - Incremental (ac80167c)
- 02: Full - Incremental (43abce3a)
- 01: Full - Incremental (265d1a83)
- 00: Full (e3a35004)
For the JVM PID 2177:
Before the Change:
JVM Internal Page
Event Browser Page
After the Change:
JVM Internal Page
Event Browser Page
I posted a diff for an alternative solution for this in the JMC slack. @Suchitainf is looking at it.
@Suchitainf 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!
I posted a diff for an alternative solution for this in the JMC slack. @Suchitainf is looking at it.
Oh I missed that message.
MIght want to check so that there isn't any use (except for the attribute in the SyntheticAttributeExtension) of JVM_PID (attribute or aggregator) anywhere else in JMC. For example in the JVMInformationPage@237.
MIght want to check so that there isn't any use (except for the attribute in the SyntheticAttributeExtension) of JVM_PID (attribute or aggregator) anywhere else in JMC. For example in the JVMInformationPage@237.
There is only one usage of JVM_PID which is in JVMInformationPage@237. I have searched entire workspace on eclipse.
MIght want to check so that there isn't any use (except for the attribute in the SyntheticAttributeExtension) of JVM_PID (attribute or aggregator) anywhere else in JMC. For example in the JVMInformationPage@237.
There is only one usage of JVM_PID which is in JVMInformationPage@237. I have searched entire workspace on eclipse.
One could argue that there should be a PID aggregator, and that the JVM_PID one should be deprecated.
New PR https://github.com/openjdk/jmc/pull/565