[RFR] 8202427: Enhance os::print_memory_info on Windows

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Fri May 18 05:45:07 UTC 2018 

Hi Matthias, 

The output looks good now. Reviewed.

Could you still please break the lines of the st->print statements
in the code?  Just after the format string for example.
I don't need a new webrev for that.

Best regards,
  Goetz


> -----Original Message-----
> From: Baesken, Matthias
> Sent: Thursday, May 17, 2018 5:20 PM
> To: Thomas Stüfe <thomas.stuefe at gmail.com>; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>
> Cc: Doerr, Martin <martin.doerr at sap.com>; hotspot-dev at openjdk.java.net;
> hotspot-runtime-dev at openjdk.java.net
> Subject: RE: [RFR] 8202427: Enhance os::print_memory_info on Windows
> 
> > > > I also think that one number is enough. Adaptive numbers would be
> great.
> 
> 
> Hi  Thomas/Goetz/Martin  ,  thanks for looking into it.
> Adaptive numbers could be done  in a follow-up as suggested by Thomas ,
> this would also touch the other platforms .
> 
> I added more line feeds to the output  and  reduced output to just printing
> M  .
> 
> 
> Example output from a Windows desktop PC :
> 
> Memory: 4k page, system-wide physical 32520M (20165M free)
> TotalPageFile size 34568M (AvailPageFile size 18170M)
> current process WorkingSet (physical memory assigned to process): 79M,
> peak: 79M
> current process commit charge ("private bytes"): 148M, peak: 148M
> 
> 
> new webrev :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8202427.1/
> 
> 
> Best regards, Matthias
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
> > Sent: Mittwoch, 16. Mai 2018 16:35
> > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> > Cc: Baesken, Matthias <matthias.baesken at sap.com>; Doerr, Martin
> > <martin.doerr at sap.com>; hotspot-dev at openjdk.java.net; hotspot-
> > runtime-dev at openjdk.java.net
> > Subject: Re: [RFR] 8202427: Enhance os::print_memory_info on Windows
> >
> > On Wed, May 16, 2018 at 3:48 PM, Lindenmaier, Goetz
> > <goetz.lindenmaier at sap.com> wrote:
> > > Hi Matthias,
> > >
> > > I appreciate the additional information in the hs_err file.
> > > Could you please add an example output (of the final version) to the bug
> > description?
> > >
> > > As Martin, I would like more line feeds, both in the code and the output.
> > > Currently, the output in the hs_err file looks like this (where the first line
> is
> > too long):
> > >     Memory: 4k page, system-wide physical 16776692k [16383M] (8795032k
> > [8588M] free), TotalPageFile size 49949460k [48778M] (AvailPageFile size
> > 38942152k [38029M]), user-mode portion of virtual address-space 2097024k
> > [2047M] (6940k [6M] free)
> > >     current process WorkingSet (physical memory assigned to process):
> > 991804k [968M], peak: 991804k [968M]
> > >
> > >     current process commit charge ("private bytes"): 1523632k [1487M],
> > peak: 1523632k [1487M]
> > >
> > > I also think that one number is enough. Adaptive numbers would be
> great.
> > > I know about
> > >     src/hotspot/share/memory/metaspace/metaspaceCommon.hpp:
> > print_human_readable_size()
> > > for this, but this seems a bit hidden in metaspace.
> > >
> >
> > Guys, lets not worry about human readable numbers for now. We can
> move
> > print_human_readable_size() out from metaspace to something generic
> > and use it to improve this code (and others) in a later patch. That
> > was my original intention when adding print_human_readable_size().
> >
> > The patch is good. Thanks for doing this.
> >
> > ..Thoams
> >
> > > I don't like usage of _M_IX86. Common in this file seems #ifndef _WIN64
> > for 32-bit windows
> > > dependent code.  But don't care here.
> > >
> > > Best regards,
> > >   Goetz.
> > >
> > >
> > >> -----Original Message-----
> > >> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net]
> On
> > >> Behalf Of Baesken, Matthias
> > >> Sent: Mittwoch, 16. Mai 2018 15:13
> > >> To: Doerr, Martin <martin.doerr at sap.com>; 'hotspot-
> > >> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>; hotspot-
> > >> runtime-dev at openjdk.java.net
> > >> Subject: RE: [RFR] 8202427: Enhance os::print_memory_info on
> Windows
> > >>
> > >> Ping :   could I get a second review ?
> > >>
> > >> Bug :
> > >> https://bugs.openjdk.java.net/browse/JDK-8202427
> > >> Change :
> > >> http://cr.openjdk.java.net/~mbaesken/webrevs/8202427.0/
> > >>
> > >>
> > >> > We could also  just print  the  MB  value  , let's see what other think.
> > >> > Another option might be to have a flexible output   (kB for  smaller
> > >> memory
> > >> > values , MB   (or GB)  for larger ) ?
> > >>
> > >> Martin suggested to just print  the  MB values,   what do you think ?
> > >>
> > >>
> > >> Thanks, Matthias
> > >>
> > >>
> > >> > -----Original Message-----
> > >> > From: Baesken, Matthias
> > >> > Sent: Mittwoch, 2. Mai 2018 13:00
> > >> > To: Doerr, Martin <martin.doerr at sap.com>; 'hotspot-
> > >> > dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>; hotspot-
> > >> > runtime-dev at openjdk.java.net
> > >> > Subject: RE: [RFR] 8202427: Enhance os::print_memory_info on
> > Windows
> > >> >
> > >> > Hi Martin,  thanks for your input .
> > >> > I add  hotspot-runtime-dev  .
> > >> >
> > >> > >
> > >> > > I wonder if we really need the sizes in kB in addition to MB. Maybe
> > other
> > >> > > reviewers would like to comment on this, too.
> > >> > >
> > >> >
> > >> > We could also  just print  the  MB  value  , let's see what other think.
> > >> > Another option might be to have a flexible output   (kB for  smaller
> > >> memory
> > >> > values , MB   (or GB)  for larger ) ?
> > >> >
> > >> > Best regards, Matthias
> > >> >
> > >> >
> > >> > > -----Original Message-----
> > >> > > From: Doerr, Martin
> > >> > > Sent: Mittwoch, 2. Mai 2018 12:53
> > >> > > To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> > >> > > dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> > >> > > Subject: RE: [RFR] 8202427: Enhance os::print_memory_info on
> > Windows
> > >> > >
> > >> > > Hi Matthias,
> > >> > >
> > >> > > looks like a nice enhancement. We can get substantially more
> > >> information.
> > >> > >
> > >> > > I wonder if we really need the sizes in kB in addition to MB. Maybe
> > other
> > >> > > reviewers would like to comment on this, too.
> > >> > >
> > >> > > We should have line breaks.
> > >> > >
> > >> > > Best regards,
> > >> > > Martin
> > >> > >
> > >> > >
> > >> > > -----Original Message-----
> > >> > > From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net]
> > On
> > >> > > Behalf Of Baesken, Matthias
> > >> > > Sent: Montag, 30. April 2018 16:53
> > >> > > To: 'hotspot-dev at openjdk.java.net' <hotspot-
> > dev at openjdk.java.net>
> > >> > > Subject: [RFR] 8202427: Enhance os::print_memory_info on
> Windows
> > >> > >
> > >> > > On Windows,
> > >> > > the os::print_memory_info  misses a few  memory-related infos
> that
> > >> might
> > >> > > be interesting :
> > >> > > - current and peak WorkingSet size  (= physical memory assigned to
> > the
> > >> > > process)
> > >> > > - current and peak commit charge  (also known as "private bytes" in
> > >> > Windows
> > >> > > tools)
> > >> > > - on 32bit Windows :
> > >> > > user-mode portion/free user mode portion  of virtual address-space
> > >> > > (Total/AvailVirtual) because it shows how close do we get to the 2-4
> > GB
> > >> per
> > >> > > process border.
> > >> > >
> > >> > >
> > >> > > - the current naming of "swap/free-swap" memory is a bit
> misleading;
> > >> > > in the Windows world swap is a special page file used for UWP apps.
> > >> > > (see Windows Internals :  "The swap file"  (chapter 5 Memory
> > >> > management)
> > >> > > Windows 8 added another page file called a swap file. It is ...
> > exclusively
> > >> > used
> > >> > > for UWP (Universal Windows Platform) apps.
> > >> > > Our output (ullTotalPageFile and ullAvailPageFile) is NOT about the
> > UWP
> > >> > > related values, it is about page file sizes
> > >> > > (so calling it TotalPageFile/AvailPageFile in "Windows-speak" or just
> > >> virtual
> > >> > > memory might be more appropriate).
> > >> > >
> > >> > >
> > >> > > https://msdn.microsoft.com/de-
> > >> > > de/library/windows/desktop/aa366770(v=vs.85).aspx
> > >> > >
> > >> > > documents it in the following way:
> > >> > > ullTotalPageFile:
> > >> > > The current committed memory limit for the system or the current
> > >> process,
> > >> > > whichever is smaller,...
> > >> > >
> > >> > > ullAvailPageFile :
> > >> > > The maximum amount of memory the current process can commit,
> in
> > >> > bytes.
> > >> > > This value is equal to or smaller than the system-wide available
> > commit
> > >> > value
> > >> > >
> > >> > >
> > >> > >
> > >> > > Aditionally I suggest having output of the various memory-values  in
> M
> > >> > > (megabyte) as well , the k  (kilobyte) output sometimes gives very
> > high
> > >> and
> > >> > > unreadable numbers).
> > >> > >
> > >> > >
> > >> > > Could you please  review my change ?
> > >> > >
> > >> > >
> > >> > > Bug :
> > >> > >
> > >> > > https://bugs.openjdk.java.net/browse/JDK-8202427
> > >> > >
> > >> > >
> > >> > > Change :
> > >> > >
> > >> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8202427.0/
> > >> > >
> > >> > >
> > >> > > Thanks, Matthias
> > >> > >
> > >

More information about the hotspot-runtime-dev mailing list