RFR[9u-dev]: 8054326: Confusing message in "Current rem set statistics"
Cheleswer Sahu
cheleswer.sahu at oracle.com
Mon May 2 04:41:08 UTC 2016
Hi,
Thanks Per for review. I will correct the formatting before pushing the changes.
Regards,
Cheleswer
> -----Original Message-----
> From: Per Liden
> Sent: Friday, April 29, 2016 7:06 PM
> To: Cheleswer Sahu; Mattis Castegren; 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-29 14:48, Cheleswer Sahu wrote:
> > 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/
>
> Looks good to me. Just one minor thing, please format this line like you did
> with the others.
>
> 289 out->print_cr(" Total per region rem sets sizes = "
> SIZE_FORMAT "%s."
> 290 " Max = " SIZE_FORMAT "%s.",
> 291 byte_size_in_proper_unit(total_rs_mem_sz()),
> proper_unit_for_byte_size(total_rs_mem_sz()),
> byte_size_in_proper_unit(max_rs_mem_sz()),
> 292 proper_unit_for_byte_size(max_rs_mem_sz()));
>
> cheers,
> Per
>
> >
> >
> > 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
> >>>>>>
> >>>>>>
> >>>>>>
More information about the hotspot-gc-dev
mailing list