RFR(xxs): 8251257: NMT: jcmd VM.native_memory scale=1 crashes target VM

[David Holmes]

On 9/08/2020 1:01 am, Thomas Stüfe wrote:
> Hi David,
> 
> On Sat, Aug 8, 2020 at 2:32 PM David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     Hi Thomas,
> 
>     Wouldn't a scale name of "b" be more appropriate given the
>     acceptance of
>     "scale=b"? It seems weird to me that scale=1 is even accepted in the
>     first place.
> 
> 
> I am not sure what you refer to, the added case in the switch construct? 

Yes. I expected to see

   37   switch(scale) {
   38     case B: return "B";
   39     case K: return "KB";
   40     case M: return "MB";
   41     case G: return "GB";
   42   }

...

> If yes, this is the string of the unit NMT prints its numbers with. For 
> byte sizes, we normally omit the unit, so I do it here too.

... but if the convention is to omit the unit for bytes then that is 
fine. The use of "1" in the switch rather than B made this look even 
odder to me. And I recognise that for some reason the code uses the 
alias "1" instead of "B" as the primary way to refer to the "byte" scale.

> If you object against the name of the scale as used in the scale 
> argument, that has been there before and changing it is not subject of 
> the patch. I just don't want the target VM to crash.

Sorry I didn't intend to suggest that you change it, just commenting 
that it seems quite odd to me and inconsistent with the other scale values.

> 
>     And can you please add some information to the bug report.
> 
> 
> Done. I thought subject + assert printout would be sufficient, sorry for 
> the brevity.

Thanks.

David

>     Thanks,
>     David
> 
> 
> Thank you, Thomas
> 
> 
>     On 8/08/2020 4:15 pm, Thomas Stüfe wrote:
>      > Hi,
>      >
>      > may I have a second Review please?
>      >
>      > All tests at SAP went through successfully, as did the jdk-submit
>     tests at
>      > Oracle.
>      >
>      > Thanks!
>      >
>      > ..Thomas
>      >
>      > On Thu, Aug 6, 2020 at 5:07 PM Thomas Stüfe
>     <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>> wrote:
>      >
>      >> Hi,
>      >>
>      >> very simple small fix.
>      >>
>      >> jcmd VM.native_memory allows for scale=1 to print exact byte
>     values, but
>      >> crashes when printing.
>      >>
>      >> JBS: https://bugs.openjdk.java.net/browse/JDK-8251257
>      >> webrev:
>      >>
>     http://cr.openjdk.java.net/~stuefe/webrevs/8251257-nmt-scale-1-crash/webrev.00/webrev/
>      >>
>      >> Thanks, Thomas
>      >>
>