RFR(L): 8201572: Improve Metaspace Statistics

Thu Apr 26 06:03:50 UTC 2018

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/