RFR(L): 8201572: Improve Metaspace Statistics

Tue Apr 24 16:28:16 UTC 2018

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/