RFR[9u-dev]: 8054326: Confusing message in "Current rem set statistics"

Fri Apr 29 12:48:21 UTC 2016

Hi,

I made changes by replacing the "round_k()" function with byte_size_in_proper_unit() and proper_unit_for_byte_size().
This will take care of printing the values in correct format.

Please review the code changes from below link:
Webrev: http://cr.openjdk.java.net/~csahu/8054326/webrev.01/

Regards,
Cheleswer

> -----Original Message-----
> From: Mattis Castegren
> Sent: Tuesday, April 19, 2016 2:45 PM
> To: Liden; Cheleswer Sahu; Mikael Gerdin; hotspot-gc-
> dev at openjdk.java.net
> Cc: Mattis Castegren
> Subject: RE: RFR[9u-dev]: 8054326: Confusing message in "Current rem set
> statistics"
> 
> Hi
> 
> Sorry, maybe I misunderstood the current workings. Will the current
> methods print bytes under 10k? I thought you suggested that we changed it
> so that it did. I agree that is a feature, I just didn't know if it was how it
> worked now or if it was a suggestion for further improvement.
> 
> Kind Regards
> /Mattis
> 
> -----Original Message-----
> From: Per Liden
> Sent: den 19 april 2016 09:49
> To: Mattis Castegren; Cheleswer Sahu; Mikael Gerdin; hotspot-gc-
> dev at openjdk.java.net
> Subject: Re: RFR[9u-dev]: 8054326: Confusing message in "Current rem set
> statistics"
> 
> Hi,
> 
> On 2016-04-18 13:27, Mattis Castegren wrote:
> > Ok, so the benefit of more granularity and less unclear messages for low
> numbers stand against the ease to parse the logs.
> >
> > I think option 3 seems reasonable, to change both this and similar places.
> Per, in that case, do you think we should change the
> proper_unit_for_byte_size/byte_size_in_proper_unit functions to print
> bytes under 10k in the same bug?
> 
> You mean under 1K? I don't know the full history of these functions, but I
> think people see the "print B up to 10K" as a feature and not a bug, to limit
> the amount of rounding/loss you'll see. Is there a reason why you wouldn't
> want that in this case too?
> 
> cheers,
> Per
> 
> >
> > How big do you all think the parsing issue will be, though? Is that so big it's a
> stopper, or can Cheleswer go ahead and change a few of these places.
> >
> > Kind Regards
> > /Mattis
> >
> > -----Original Message-----
> > From: Per Liden
> > Sent: den 13 april 2016 10:56
> > To: Mattis Castegren; Cheleswer Sahu; Mikael Gerdin;
> > hotspot-gc-dev at openjdk.java.net
> > Subject: Re: RFR[9u-dev]: 8054326: Confusing message in "Current rem set
> statistics"
> >
> > Hi,
> >
> > On 2016-04-13 09:57, Mattis Castegren wrote:
> >> Hi
> >>
> >> This request came in some time ago as a low priority request from PSR
> (the Oracle performance team):
> >>
> >>
> https://bug.oraclecorp.com/pls/bug/webbug_edit.edit_info_top?rptno=19
> >> 3
> >> 56019
> >> PSR:FUNC: Confusing msg in "Current rem set statistics"
> >>
> >> Their main complaint is that 0 usually means exactly 0. So, I think the
> balance is between 0 being confusing when the value is non-zero vs the ease
> of parsing the logs. I see three ways to do this:
> >>
> >> 1) Close the bug as Won't Fix
> >> 2) Change this specific issue, print bytes if the value is lower than
> >> 1k
> >> 3) Change this and a few other places.
> >>
> >> We in Sustaining don't have too strong of an opinion here. Stanley Guan
> from PSR recently commented the BugDB bug making the case for changing
> the output, but if the GC team is of the opinion that we should close this as
> Won't Fix with the motivation that parsing becomes harder with this change,
> then we can close the bug.
> >>
> >> Anyone else who has an opinion or a motivation for any of the options
> above?
> >
> > I would prefer 1 or 3, where 3 is be using the existing
> proper_unit_for_byte_size/byte_size_in_proper_unit functions, unless
> there's a good reason not to. I can't see any problems with printing B for sizes
> under 10K rather than 1K, it's actually a feature.
> >
> > cheers,
> > Per
> >
> >>
> >> Kind Regards
> >> /Mattis
> >>
> >> -----Original Message-----
> >> From: Cheleswer Sahu
> >> Sent: den 11 april 2016 15:33
> >> To: Liden; Mikael Gerdin; hotspot-gc-dev at openjdk.java.net
> >> Cc: Mattis Castegren
> >> Subject: RE: RFR[9u-dev]: 8054326: Confusing message in "Current rem
> set statistics"
> >>
> >> Hi,
> >>
> >>
> >> -----Original Message-----
> >> From: Per Liden
> >> Sent: Friday, April 08, 2016 1:57 PM
> >> To: Mikael Gerdin; Cheleswer Sahu; hotspot-gc-dev at openjdk.java.net
> >> Subject: Re: RFR[9u-dev]: 8054326: Confusing message in "Current rem
> set statistics"
> >>
> >> Hi,
> >>
> >> On 2016-04-06 13:09, Mikael Gerdin wrote:
> >> [...]
> >>>> Hi,
> >>>>
> >>>>
> >>>> Please review the code changes for
> >>>> https://bugs.openjdk.java.net/browse/JDK-8054326.
> >>>>
> >>>>
> >>>> Webrev Link: http://cr.openjdk.java.net/~csahu/8054326/
> >>>>
> >>
> >> We try to avoid printing sizes with dynamically selected binary
> >> prefixes
> >> (K/M/G) for the simple reason that it makes it harder to grep through and
> compare GC logs. If that "rule" applies here can be discussed.
> >>
> >> We already have the functions
> >> proper_unit_for_byte_size/byte_size_in_proper_unit to do roughly what
> you've done, but they are very rarely used for the reason stated above.
> >>
> >> Mikael had directed me towards this function when we had offline chat
> regarding this, but this function prints in KB only if size is  greater than 10k.
> Therefore I had written my own code to print this.
> >>
> >> There are a other places in this code/function where we're also printing
> potentially small values as K, which you haven't addressed here. Just
> addressing one of the places seems like an incomplete fix.
> >>
> >> Yes, I agree there are other places too which can be considered. I just
> took the "Max" part because I was not sure about others.
> >>
> >> I'm personally not sure if this is a problem big enough to worth addressing
> at all. Maybe others have a different opinion, Thomas/Bengt/Mikeal/Stefan?
> >>
> >> Does any have any other opinion regarding this ? We in sustaining team
> although feel that showing "Max" as 0k is ambiguous.
> >>
> >>
> >> Regards,
> >> Cheleswer
> >> cheers,
> >> Per
> >>
> >>>>
> >>>> Bug Brief: In output of remset statistics "Max"  is printing as 0k,
> >>>> which is confusing for user.
> >>>>
> >>>>
> >>>>
> >>>> Problem Identified : "Max" value is rounded to KB, which result  in
> >>>> 0k, if the value is less than 1024B.
> >>>>
> >>>>
> >>>>
> >>>> Solution Proposed:  If the value for "Max" is less than 1 KB (1024
> >>>> B),  print the value in bytes (i.e. 1023B.) else
> >>>>
> >>>> print the value in KB.
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> Regards,
> >>>>
> >>>> Cheleswer
> >>>>
> >>>>
> >>>>