RFR(L): 8201572: Improve Metaspace Statistics

Tue May 1 13:54:13 UTC 2018

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

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/