[RFR] 8202427: Enhance os::print_memory_info on Windows
Thomas Stüfe
thomas.stuefe at gmail.com
Fri May 18 06:16:10 UTC 2018
Hi Matthias,
I took another closer look and have some minor nits. Sorry for not
looking better the first time, my head was not clear :)
Since that was my fault I leave it up to you if you follow up. The fix
in its current form is also fine to me.
- I am not a big fan of ">> 20". I would change that to " / M" to make
the code more readable.
- Strictly speaking GetProcessMemoryInfo could fail (I have never seen
that though) and return FALSE. In which case we should probably not
output the results.
Best Regards, Thomas
On Thu, May 17, 2018 at 5:20 PM, Baesken, Matthias
<matthias.baesken at sap.com> wrote:
>> > > 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