RFR: 8251158: Implementation of JEP 387: Elastic Metaspace

Sat Aug 22 12:17:34 UTC 2020

Excellent, Roman, thanks a bunch!

..Thomas

On Sat, Aug 22, 2020 at 2:17 PM Roman Kennke <rkennke at redhat.com> wrote:

> 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
> >
>
>