RFR(L): 8201572: Improve Metaspace Statistics

Wed May 2 05:36:54 UTC 2018

Hi Coleen,

On Tue, May 1, 2018 at 3:54 PM,  <coleen.phillimore at oracle.com> wrote:
>
> Hi, This isn't a full review but I like that you've added a metaspace
> namespace and directory under memory for it.
> This will make a good foundation for doing this change:
>
> https://bugs.openjdk.java.net/browse/JDK-8176808
>

Yes, that is what I thought.

When ripping up metaspace.cpp, should we be concerned that we loose
history for the ripped out code?

..Thomas

> Thanks,
> Coleen
>
>
> On 4/26/18 2:03 AM, Thomas Stüfe wrote:
>>
>> Hi all,
>>
>> I still need an additional reviewer.
>>
>> new Webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/webrev.02/webrev/index.html
>> incremental to v1:
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/webrev-01-to-02/webrev/
>>
>> new: just build- and test fixes:
>> - I added final cr() to a number of places where LogStream was used to
>> pipe output from functions I changed. Otherwise ~LogStream will assert
>> complaining about unfinished lines. I plan to change this, see
>> https://bugs.openjdk.java.net/browse/JDK-8202303, however for now,
>> lets go with the additional cr()
>> - build fixes for MacOS and Solaris.
>>
>> jdk-submit test suite ran clean through. We are currently running the
>> patch through our test suites, so far all looks good.
>>
>> Thanks, Thomas
>>
>>
>>
>>
>> On Tue, Apr 24, 2018 at 6:28 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
>> wrote:
>>>
>>> Hi all,
>>>
>>> New webrev:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/webrev-01/webrev/
>>> Incremental:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/webrev-00-to-01/webrev/
>>>
>>> This one now cleanly applies to jdk-jdk and does not need other patches.
>>>
>>> Changes:
>>> - removed MetaWord occurrences in ostream.cpp, replaces with BytesPerWord
>>> - added per-spacetype printing to PrintNMTStatistics output to make it
>>> more closely resemble the output before.  Example output:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/PrintNMTStatistics.txt
>>> - Fixed Windows build (missed precompiled.hpp)
>>>
>>> Thanks, Thomas
>>>
>>>
>>> On Mon, Apr 23, 2018 at 6:32 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
>>> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> may I please have opinions and reviews for the following improvement.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8201572
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/webrev.00/webrev/
>>>>
>>>> (Note: it needs patch for 8202074 [3] to apply cleanly)
>>>>
>>>> This is a combined feature-add and cleanup for the metaspace
>>>> reporting. It should have no effect on metaspace allocation itself.
>>>>
>>>> This change does a number of things:
>>>>
>>>> - It improves the "VM.metaspace" jcmd, making it hopefully much more
>>>> versatile.
>>>>
>>>> - It improves the way statistics are collected: The number of times we
>>>> walk the CLDG or the internal metaspace structures is reduced. It also
>>>> adds another global counter to MetaspaceUtils.
>>>>
>>>> - Though this was not primarily intended as a cleanup change, a number
>>>> of cleanups are part of this change. A number of functions were
>>>> removed. Where new code was added, I attempted to define and follow a
>>>> new scheme using namespaces and separate files. See below for details.
>>>>
>>>> ---
>>>>
>>>> Details:
>>>>
>>>> -> Improvements of jcmd:
>>>>
>>>> The jcmd "VM.metaspace" command now features a number of sub command
>>>> which control the details printed:
>>>>
>>>> jcmd VM.metaspace [basic] [show-loaders] [by-chunktype] [by-spacetype]
>>>> [vslist] [vsmap] [scale]
>>>>
>>>> All of them except for "basic" can be combined with each other.
>>>>
>>>> "basic"
>>>>    shows a basic report; it needs no CLDG walk and no locks, so it
>>>> should be safe under most conditions.
>>>> "show-loaders"
>>>>    this shows metaspace usage by loaders (basically what VM.metaspace
>>>> did before - this was originally added by Zhengyu Gu)
>>>> "by-chunktype"
>>>>    Numbers are broken down by chunk type (small, medium...) in addition
>>>> to total numbers
>>>> "by-spacetype"
>>>>    Numbers are broken down by space type (bootloader, anonymous,
>>>> reflection...) in addition to total numbers
>>>> "vslist"
>>>>    The virtual space list is printed in detail
>>>> "vsmap"
>>>>    Prints the virtual space map (ascii map of chunk placements)
>>>>
>>>> When no arguments are given, a summary is displayed.
>>>>
>>>> In addition to the above mentioned features we now print
>>>> - a "Waste" section detailing wastage.
>>>> - in debug mode, a section with some internal counters. This was just
>>>> curiosity on my part, I wanted to see how "hot" some of the metaspace
>>>> allocation paths actually get.
>>>> - We now count overhead too, i.e. space needed for chunk headers (was
>>>> counted as "used" before)
>>>> - We now also show deallocated space (block freelist)
>>>>
>>>>
>>>> Some output examples:
>>>>
>>>> <No arguments>
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace.txt
>>>>
>>>> "basic"
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace_basic.txt
>>>>
>>>> "by-chunktype"
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace_by-chunktype.txt
>>>>
>>>> "by-spacetype"
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace_by_spacetype.txt
>>>>
>>>> "show-loaders"
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace_showloaders.txt
>>>>
>>>> "show-loaders" and "by-chunktype combined"
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace_showloaders_by-chunktype.txt
>>>>
>>>> "vslist"
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace_vslist.txt
>>>>
>>>> "vsmap"
>>>>
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/examples/VM.metaspace_vsmap.txt
>>>>
>>>> ---
>>>>
>>>> -> The way we collect statistics was streamlined. I tried to get rid
>>>> of all functions which were walking structures to just sum up a single
>>>> value; not only was that unnecessary code duplication, it was also
>>>> inefficient.
>>>>
>>>> This means most of the SpaceManager::sum_xxxx() functions are gone, as
>>>> well as all the different MetaspaceUtils::xxx_xxx_slow() functions.
>>>>
>>>> They were replaced by functions named "collect_statistics" or
>>>> "add_statistics", which accumulate all needed data in one go into a
>>>> statistics structure. This is more efficient since we usually need
>>>> more than one value and so it makes sense to collect them in one go.
>>>>
>>>> The newly added statistics objects are:
>>>>
>>>> - UsedChunksStatistics
>>>> - SpaceManagerStatistics
>>>> - ClassLoaderMetaspaceStatistics
>>>>
>>>> They live in a new file (memory/metaspace/metaspaceStatistics).
>>>>
>>>> I also ripped out the preexisting ChunkManagerStatistics, modified it
>>>> to follow the same convention, and moved it into this new file.
>>>>
>>>> -> We now have a new central reporting function,
>>>> MetaspaceUtils::print_report() - this was once Zhengyu's
>>>> print_for_nmt() function, but I expanded it and renamed it to be the
>>>> central entry point. Reporting options are handed into it via flags.
>>>>
>>>> In addition to that, we have a new
>>>> MetaspaceUtils::print_basic_report(), which prints a short summary
>>>> under the promise of not walking or locking anything. This is used at
>>>> error reporting time to print a short summary to the hs-err file. It
>>>> is also used for VM.info().
>>>>
>>>> -> All new functions now accept the "scale" parameter known from NMT.
>>>> I expanded it a bit to have a "dynamic" setting. In that setting, the
>>>> printer chooses the best unit for a given value. I find it works quite
>>>> well, so I made it the default. I also changed printing to correctly
>>>> print very-small-but-not-0 numbers or percentages, where before they
>>>> were rounded to 0. See linked output examples.
>>>>
>>>> -> Other things to note:
>>>>
>>>> - MetaspaceUtils::dump was used to print some details at Metaspace
>>>> OOM. This now calls MetaspaceUtils::print_basic_report() directly, and
>>>> I removed the dump() function.
>>>>
>>>> - I moved printing the summary out of the PrintCLDMetaspaceInfoClosure
>>>> and into the print_report function. PrintCLDMetaspaceInfoClosure still
>>>> prints the individual loader metaspace usage numbers.
>>>>
>>>> - I streamlined naming of values somewhat. Where we refer to in-use
>>>> chunks, I now use consistently "capacity", "free", "waste", "used" and
>>>> the new "overhead", and the assumption is that capacity is the sum of
>>>> all the other values.
>>>>
>>>> - About the newly added namespaces: Since the patch is already quite
>>>> big, I did not want to load it with more cleanup. However, neither did
>>>> I want to just dump the new classes and functions into metaspace.cpp.
>>>> So as a compromise I tried to follow a new scheme with newly added
>>>> code. If you agree on this organization, this may be the way to split
>>>> metaspace.cpp in some follow up change.
>>>>
>>>> The idea was to move all metaspace-internal stuff into
>>>> memory/metaspace and hide it from the outside. Also, I did put
>>>> metaspace coding into an own namespace ("metaspace") and
>>>> metaspace-internal coding into "metaspace::internal". This is just a
>>>> proposal, we can bike shed. Future changes could move more classes out
>>>> of metaspace.cpp into metaspace/ and the metaspace::internal namespace
>>>> to make metaspace.cpp malleable again.
>>>>
>>>> Thank you,
>>>>
>>>> Kind Regards, Thomas
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> --------------------
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8201572
>>>> [2]
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8201572-improve-metaspace-reporting/webrev.00/webrev/
>>>> [3]
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8202074-metaspace-retire-after-humongous-chunks-are-allocated/webrev.00/webrev/
>
>