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

Baesken, Matthias matthias.baesken at sap.com
Tue Apr 2 11:04:32 UTC 2019


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/native/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
> 



More information about the hotspot-dev mailing list