-
Notifications
You must be signed in to change notification settings - Fork 6.2k
JDK-8374395 : Improve MemoryUsage.toString() and constructor error messages for better clarity #29009
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
base: master
Are you sure you want to change the base?
JDK-8374395 : Improve MemoryUsage.toString() and constructor error messages for better clarity #29009
Conversation
|
👋 Welcome back thswlsqls! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@thswlsqls 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. |
Webrevs
|
Note that you can't use the same JBS issue for different PRs/commits. If there are other changes proposed then they will need their own JBS issues. If this issue is only for changing MemoryUsage::toString then it should be renamed as it is confusing to see "constructor error messages" in the title when there is no change to the constructor. MemoryUsage's methods are specified to return -1 for undefined so I don't think it's too surprising that the toString method includes "-1" values. BTW: test/jdk/java/lang/management/MemoryUsage would be a better place for unit tests for this API. |
|
@thswlsqls |
|
@AlanBateman Thanks for the review. Addressed. Would it be possible to review this PR for the first issue of JDK-8374395 only? |
|
/label remove core-libs |
|
@AlanBateman |
|
/integrate |
|
@thswlsqls This pull request has not yet been marked as ready for integration. |
kevinjwalls
left a comment
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.
When
initormaxis-1(undefined),MemoryUsage.toString()outputs"-1(-1K)", which is confusing. While Javadoc states that-1means undefined, the output does not clearly indicate this, making it difficult for users to distinguish undefined values from actual negative values.
Hi. When does this happen? When is a MemoryUsage created with init or max as -1 ? When is it then printed using toString() for a user to understand, and when would the user then have to distinguish between the -1 which suggests a problem, and "actual negative values"?
Summary
MemoryUsage.toString() now displays "N/A" for undefined init and max values (-1) instead of "-1(-1K)".
This addresses the first of three issues in JDK-8374395.
Description
When init or max is -1 (undefined), toString() outputs "-1(-1K)". The Javadoc states that -1 means undefined, but the output does not clearly indicate this.
toString() checks if init or max equals -1 and outputs "init = N/A" or "max = N/A" respectively. Output format for valid values remains unchanged.
Added test cases to the existing jtreg test file. All tests pass.
Bug-ID : JDK-8374395
https://bugs.java.com/bugdatabase/view_bug?bug_id=JDK-8374395
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/29009/head:pull/29009$ git checkout pull/29009Update a local copy of the PR:
$ git checkout pull/29009$ git pull https://git.openjdk.org/jdk.git pull/29009/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 29009View PR using the GUI difftool:
$ git pr show -t 29009Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/29009.diff
Using Webrev
Link to Webrev Comment