AW: RFR[9u-dev]: 8054326: Confusing message in "Current rem setstatistics"

[Per Liden]

Hi,

On 2016-04-13 11:28, Bernd Eckenfels wrote:
> Hello,
>
> Isn’t formatting changes and units part of the UL GC logging changes,
> anyway?

That's a slightly different topic. This is about printing units in a 
uniform way during a single run.

cheers,
Per

>
>
> Gruss
> Bernd
> --
> http://bernd.eckenfels.net
>  From Win 10 Mobile
>
> *Von: *Per Liden <mailto:per.liden at oracle.com>
> *Gesendet: *Mittwoch, 13. April 2016 11:05
> *An: *Mattis Castegren <mailto:mattis.castegren at oracle.com>; Cheleswer
> Sahu <mailto:cheleswer.sahu at oracle.com>; Mikael Gerdin
> <mailto:mikael.gerdin at oracle.com>; hotspot-gc-dev at openjdk.java.net
> <mailto:hotspot-gc-dev at openjdk.java.net>
> *Betreff: *Re: RFR[9u-dev]: 8054326: Confusing message in "Current rem
> setstatistics"
>
> 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=19356019
>
>  > 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
>
>  >>>
>
>  >>>
>
>  >>>
>