RFR(L): 8176808: Split up metaspace.cpp
Thomas Stüfe
thomas.stuefe at gmail.com
Wed May 9 15:08:03 UTC 2018
Hi all,
This was a long time coming. Lets cut up this beast :)
Bug: https://bugs.openjdk.java.net/browse/JDK-8176808
Webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8176808-split-metaspace-cpp/webrev.00/webrev/
This change splits up metaspace.cpp into multiple parts. Note that I
did not change much code (exceptions see below in details) - so this
is mostly moving code around.
This change follows the scheme tentatively outlined in JDK-8201572:
- metaspace internal classes go into the sub directory
share/memory/metaspace. Ideally, those should never be accessed from
outside, apart from other metaspace classes and metaspace tests. They
also go into the namespace ::metaspace::internals.
- metaspace external classes (MetaspaceUtils, ClassLoaderMetaspace,
etc) remain inside share/memory and, for now, remain in the global
namespace.
Note: the original idea was to move metaspace external classes to
namespace ::metaspace. But I shied away from that, since that would
mean fixing up all usages of these classes either with metaspace::
scope or with using declarations. I had to make a cut at some point.
So here is something to decide:
- should we then get rid of the ::metaspace::internals namespace, move
metaspace-internal classes to the enclosing ::metaspace namespace and
leave external classes in the global namespace ?
- or should we follow through (maybe in a future patch): internal
classes in ::metaspace::internal, and move external classes to
::metaspace ?
Some more details:
- the following classes moved out of metaspace.cpp into namespace
"metaspace::internal" and the metaspace sub directory:
MetaDebug, ChunkManager, SpaceManager, BlockFreeList and SmallBlocks,
OccupancyMap, VirtualSpaceNode and VirtualSpaceList, the
PrintCLDMetaspaceInfoClosure.
- I also moved metachunk.hpp to internals, since class Metachunk is
not used externally. What is used externally is Metablock though, so I
split up metachunk.hpp into metabase.hpp, metablock.hpp and
metachunk.hpp, holding Metabase, Metablock and Metachunk,
respectively.
- Now we see some ugly seams: metaspace.hpp is supposed to be the
outside API contract, however, it contains implementation details
which should not be there and which manifest now by having to use the
metaspace::internals namespace here and there. If we care, we could
solve this with a bit redesigning.
- The ChunkManager gtest and the SpaceManager gtest had been
implemented inside metaspace.cpp, since it was not possible to access
the internal classes those tests needed in any other way. Since that
is now possible, I moved the coding out to gtest-land and made them
real gtests (exchanging the asserts for real gtest ASSERTS, among
other things).
Note that there are some tests left in metaspace.cpp
(TestMetaspaceUtilsTest, TestVirtualSpaceNodeTest) - those were no
gtests-in-disguise but were part of -XX:+ExecuteInternalVMTests. I
guess these tests precede the gtest framework? I leave it up to others
to decide what to do with them and to potentially move them out of
metaspace.cpp.
Side note: -XX:+ExecuteInternalVMTests seems to have bitrotted, see
https://bugs.openjdk.java.net/browse/JDK-8202848. Does this get tested
regularly?
- metaspace.cpp is quite a bit smaller now - from ~5000 loc down to
~1700 loc. Arguably, one could split out more and clean up more, but I
think this is a good start.
---
I built locally on linux (release, fastdebug with and without pch,
zero) and windows (64, 32bit). jdk-submit tests ran through with a
build error on solaris sparc - I currently try to reproduce that build
error with our very slow solaris machine.
Thanks, Thomas
More information about the hotspot-runtime-dev
mailing list