RFR: 8251158: Implementation of JEP 387: Elastic Metaspace

Sat Aug 22 12:16:52 UTC 2020

Hi Thomas,

this looks like very useful work! Thank you!

Not a review, just a note: I put the original patch through a bunch of
my regular (Shenandoah) testing on x86 and aarch64, and haven't found
any issues.

I will try to give it a deeper look next week (no promises).

Cheers,
Roman

> (Am Mittwoch, den 12.08.2020, 19:59 +0200 schrieb Thomas Stüfe:
> Dear all,
> 
> I would like to start the review for the implementation of JEP 387
> "Elastic
> Metaspace".
> 
> The relevant JBS items are:
> 
> JEP: https://openjdk.java.net/jeps/387
> 
> Implementation Issue (pretty much only a placeholder currently):
> https://bugs.openjdk.java.net/browse/JDK-8251158
> 
> --
> 
> Reminder of why we do this:
> 
> 1. The new metaspace saves memory. How much depends on context: if it
> is
> not a problem today it will continue to be none. But in cases where
> today
> we face large metaspace consumption we may get reductions, sometimes
> drastic ones. These reductions are caused by two facts:
> - after mass class unloading we release memory more promptly to the
> OS
> - even without class unloading chunk allocation is just plain
> smarter,
> which reduces the overhead per class loader. This is especially true
> for
> scenarios involving masses of small loaders which only load very few
> classes.
> 
> As an example, see [1] where two VMs - one stock, one patched - run
> the
> same test program which creates tons of small loaders. The second run
> shows
> a much reduced memory footprint and increased elasticity.
> 
> 2. The rewritten metaspace code base got cleaner and less complex and
> thus
> should be much easier to maintain and to extend. It also would be
> possible
> to easily reuse the allocator for different parts of the VM, since it
> is
> less tightly tied to the notion of just storing class metadata.
> 
> --
> 
> In preparation of this review I prepared a guide [2], [3], which
> gives an
> architectural overview and should be the starting point for an
> interested
> Reviewer.
> 
> The prototype has been tested extensively for quite some time in
> SAP's CI
> system. We regularly run JCK test, JTReg tests and a whole battery of
> SAP
> internal tests on 8 platforms. Tests are also currently ongoing at
> Oracle
> and Red Hat.
> 
> --
> 
> The full webrev [4] is somewhat large. In order to make this more
> bearable
> I broke the patch up into three parts:
> 
> 1) The core parts [5]
> 
> This is the heart of the metaspace, mostly rewritten from scratch. I
> propose any Reviewer should not look at the diff but rather just
> examine
> the new implementation. One possible exception is metaspace.hpp,
> which is
> the outside interface to metaspace.
> 
> There are several ways to get a feeling for this code (after reading
> at
> least the concept part of the provided guide [2]). The central, most
> "beefy" classes are:
> 
> - VirtualSpaceNode - does all the work of reserving, committing,
> uncommitting memory
> - RootChunkArea - does the grunt work of chunk splitting and merging
> - ChunkManager - which takes care of the chunk freelists, of
> directing
> chunk splits and merges, of enlarging chunks in place
> - MetaspaceArena - which takes care of fine granular allocation on
> behalf
> of a CLD, and of managing deallocated blocks.
> 
> One way to review could be bottom up: starting at
> VirtualSpaceNode/CommitMask/RootChunkArea(LUT), working your way up
> to
> ChunkManager and Metachunk; finally up to to MetaspaceArena while
> taking a
> side stroll to FreeBlocks/BinList/BlockTree.
> 
> Another way would be to follow typical paths:
> 
> Allocation path: Starting at MetaspaceArena::allocate() ->
> Metachunk::allocate() (note the committing-on-demand code path
> starting
> here) -> ChunkManager::get_chunk() ->
> VirtualSpaceList/Node->allocate_root_chunk().
> 
> Destruction: ~MetaspaceArena() -> ChunkManager::return_chunk() ->
> (merging
> chunks) -> (uncommitting chunks)
> 
> Premature deallocation: -> MetaspaceArena::deallocate() -> see
> freeblock
> list handling
> 
> 2) The tests [6]
> 
> This part of the patch contains all the new tests. There are a lot;
> the
> test coverage of the new metaspace is very thorough.
> 
> New gtests have been added under 'test/hotspot/gtest/metaspace'.
> New jtreg have been added under
> 'test/hotspot/jtreg/runtime/Metaspace/elastic' and under
> 'test/hotspot/jtreg/gtest/MetaspaceGtests.java'.
> 
> 4) Miscellaneous diffs [7]
> 
> This is the part of the patch which is neither considered core nor
> test.
> These changes are small, often uninteresting, and can be reviewed via
> diff.
> 
> ---
> 
> Out of scope for this patch is revamping outside metaspace
> statistics:
> monitoring of metaspace statistics is mostly left untouched, beyond
> the
> work needed to keep existing tests green. I wanted to keep those
> changes
> separate from the core changes for JEP387. They are tracked in JDK-
> 8251392
> [8] and JDK-8251342 [9], respectively.
> 
> ---
> 
> Code complexity:
> 
> Numerically, lines of code went ever so slightly down with this
> patch:
> 
> Before: (cloc hotspot/share/memory)
> -------------------------------------------------------------------
> ------------
> C++ 36 2533 3097 12735
> C/C++ Header 54 1728 2867 6278
> -------------------------------------------------------------------
> ------------
> SUM: 90 4261 5964 19013
> 
> After:
> -------------------------------------------------------------------
> ------------
> C++ 50 3048 3774 13127
> C/C++ Header 63 2006 3575 5738
> -------------------------------------------------------------------
> ------------
> SUM: 113 5054 7349 18865
> 
> But since the new implementation is more powerful than the old one -
> doing
> things like committing/uncommitting on demand, guarding allocated
> blocks
> etc - one could argue that the new solution does quite a lot more
> with
> slightly less code, which I think is a good result.
> 
> ---
> 
> Idle musing: it would simplify matters quite a bit if we were to
> unify
> class space and non-class metaspace. If we keep the current narrow
> Klass
> pointer encoding scheme, we would still need to keep the space they
> are
> stored in contiguous. But we could use the class space for
> everything, in
> effect dropping the "class" moniker from the name. It would have to
> be
> larger than it is currently, of course.
> 
> Cons:
> - we would have to abandon the notion of "limitless metaspace" - but
> if we
> run with class space this has never been true anyway since the number
> of
> classes is limited by the size of the compressed class space.
> - virtual process size would go up since it now needs to be as large
> as the
> total projected metaspace size.
> - we may need to expand narrow Klass pointer encoding to include
> shifting,
> if 4G are not enough to hold all metaspace data.
> 
> Pros:
> - this would simplify a lot of code.
> - this would save real (committed) memory, since we save quite a bit
> of
> overhead.
> - we could narrow-pointer-encode other metadata too, not only Klass
> pointers, should that ever be interesting
> 
> ... but that is not part of this JEP.
> 
> ---
> 
> With this patch, we have a much stronger separation of concerns, and
> it
> should be easy to reuse the metaspace allocator in other hotspot
> areas as
> well. For instance, ResourceAreas and friends could be replaced by
> MetaspaceArenas. They do almost the same thing. And as we have seen
> in the
> past at SAP, the C-heap backed hotspot Arenas can be a pain since
> spikes in
> Arena usage lingers forever as process footprint (we even once
> rewrote
> parts of the arena code to use mmap, which is just on a more
> primitive
> level what Metaspace does).
> 
> ---
> 
> I know this is short notice, but there will be a call for interested
> people
> tomorrow at 11AM ET. In case any potential Reviewers are interested
> just
> drop me a short note.
> 
> ---
> 
> Thank you, Thomas
> 
> 
> [1]
> http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/guide/reduction-small-loaders.pdf
> [2]
> http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/guide/review-guide.pdf
> [3]
> http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/guide/review-guide.html
> [4]
> http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/webrev-all/webrev/
> [5]
> http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/webrev-core/webrev/
> [6]
> http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/webrev-test/webrev/
> [7]
> http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/webrev-misc/webrev/
> [8] https://bugs.openjdk.java.net/browse/JDK-8251342
> [9] https://bugs.openjdk.java.net/browse/JDK-8251392
> 