RFR: 8221535: add steal tick related information to hs_error file [linux]

[Lindenmaier, Goetz]

Hi Matthias, 

thanks for making this change.  I think it really can 
help analyzing situations on virtualized hardware.
I like that you document the dependency on kernel versions.

I have one comment: 
+      st->print_cr("Steal ticks from vm start: " UINT64_FORMAT, steal_ticks_difference);
+      st->print_cr("Steal ticks percentage from vm start:%7.3f", steal_ticks_perc);

I think you want to say "since", not "from".
Also, a space before the percentage would be nice,
or did you use '7' to generate the space?

Looks good otherwise. I don't need to see another webrev.

Best regards,
  Goetz.




> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> Baesken, Matthias
> Sent: Dienstag, 2. April 2019 13:05
> To: David Holmes <david.holmes at oracle.com>; Thomas Stüfe
> <thomas.stuefe at gmail.com>
> Cc: hotspot-dev at openjdk.java.net
> Subject: RE: RFR: 8221535: add steal tick related information to hs_error file
> [linux]
> 
> Hi David ,  thanks for the review .
> 
> New webrev :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8221535.4/
> 
> I  added the blank after if  you suggested and  also  adjusted  the comments  in
> bool os::Linux::get_tick_information   slightly .
> 
> May I add you as a reviewer ?
> 
> > > I kept the  has_steal_ticks   for now; in case someone  can  confirm
> > > 2.6.11+  for jdk13  I would remove it .
> >
> > I can't comment on that.
> >
> 
> It's a pity  that we do not  have such an info .
> 
> However I found  that we include   <sys/inotify.h>   here :
> 
> http://hg.openjdk.java.net/jdk/jdk/file/69204b98dc3d/src/java.base/linux/nati
> ve/libnio/fs/LinuxWatchService.c#l36
> 
> and this seems to be  2.6.13+ functionality :
> 
> https://en.wikipedia.org/wiki/Inotify
> 
> "August 29, 2005: Linux kernel version 2.6.13 released, containing merged
> inotify code"
> 
> However  it is not hotspot coding, so  I am not 100% sure that we really   can
> claim  2.6.13+   for the whole codebase .
> 
> 
> Best regards , Matthias
> 
> 
> 
> 
> > -----Original Message-----
> > From: David Holmes <david.holmes at oracle.com>
> > Sent: Montag, 1. April 2019 07:01
> > To: Baesken, Matthias <matthias.baesken at sap.com>; Thomas Stüfe
> > <thomas.stuefe at gmail.com>
> > Cc: hotspot-dev at openjdk.java.net
> > Subject: Re: RFR: 8221535: add steal tick related information to hs_error file
> > [linux]
> >
> > Hi Matthias,
> >
> > On 29/03/2019 11:37 pm, Baesken, Matthias wrote:
> > > Hello, new webrev :
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8221535.3/
> > >
> > > changed the function to bool os::Linux::get_tick_information(cpu_ticks*
> > > out, int which_logical_cpu = -1)
> >
> > Okay.
> >
> > > added memset  at beginning of get_tick_information
> >
> > Okay
> >
> > > removed the remaining vm_printf    from
> > > src/hotspot/os/aix/os_perf_aix.cpp      (at first I did not notice it
> > >   was  there as well ).
> >
> > Okay
> >
> > > [ kept the struct  naming CPUPerfTicks    ( I find plenty of such struct
> > > name  styles  in hotspot codebase  ).   ]
> >
> > Agreed - no consistency and you're moving existing code so this is fine
> > IMHO.
> >
> > >>
> > >
> > >>In case  we  are sure that we are always  on  2.6.11+   kernels, then indeed
> > I can remove the  special handling.
> > >
> > >>I was sure  that  we are on 2.6+  but only 95%  sure that we are
> > on  2.6.11+   .
> > >
> > >>
> > >
> > > I kept the  has_steal_ticks   for now; in case someone  can  confirm
> > > 2.6.11+  for jdk13  I would remove it .
> >
> > I can't comment on that.
> >
> > One nit in os_linux.cpp:
> >
> > +   if((fh = fopen("/proc/stat", "r")) == NULL) {
> >
> > Need space after if
> >
> > Otherwise this all seems fine to me.
> >
> > Thanks,
> > David
> >