RFR: 8251158: Implementation of JEP 387: Elastic Metaspace
stuefe at openjdk.java.net
Thu Sep 24 13:09:36 UTC 2020
this is the continuation of the ongoing review for the JEP387 implementation (last rounds see  ). Sorry for the
delay, had vacation then the entrance of Skara delayed things a bit.
For the delta diff please see .
This is the first time I do a large PR after Skara, so if something is wrong please bear with me. I cannot answer all
feedback individually in this PR body, but I incorporated almost all into the new revision.
What changed since the last version:
- I renamed most metaspace files back to the original naming scheme or to something similar, hopefully capturing the
- I changed the way allocation guards are checked if MetaspaceGuardAllocations is enabled. Before, I would test for
overwrites upon CLD destruction, but since that check was subject to VerifyMetaspaceInterval it only ran for every nth
class loader which made it rather pointless. Now I run it always.
- I also improved the printout on block corruption, and log block corruption unconditionally before asserting.
- I also fixed up and commented the death test which tests for allocation overwriters (test_allocationGuard.cpp)
Side note, I find the corruption check very useful but if you guys think it is too much I still can remove the feature.
- In ChunkManager::purge() I improved the comments after discussions with Leo.
- I fixed a bug with VerifyMetaspaceInterval: if set to 1 the "SOMETIMES" sections were supposed to fire always, but due
to a one-off error they only fired every second time. Now, if -XX:VerifyMetaspaceInterval=1, the checks really run
- Fixed indentation issues as Leo requested
- Rewrote the condition and the assert in VirtualSpaceList::allocate_root_chunk() as Leo requested
- I removed the "can_purge" logic from VirtualSpaceList. The list does not need to know. It just should iterate all nodes
and attempt purging, and if a node does not own its ReservedSpace, it refuses to be purged. That is simpler and more
flexible since it allows us to have list with purge-able and non-purge-able nodes.
- and various smaller fixes, mainly on request of Leo.
>102 // Start pointer of the area.
>103 MetaWord* const _base;
>How does this differ from _rs._base? Really needed?
>105 // Size, in words, of the whole node
>106 const size_t _word_size;
>Can we not calculate this from _rs.size()?
You are right, _base and _word_size are directly related to the underlying space. But I'd prefer to leave it the way it
is. Mainly because ReservedSpace::_base and ::_size are nonconst and theoretically can change under me. It is highly
improbable but I'd like to know. Note that VirtualSpaceNode::verify checks that.
Should we clean up ReservedSpace at some point and make those members const - as they should be - then I would rewrite
this as you suggest.
Thanks, again, for all your review work!
- Remove trailing whitespaces
- JEP387 review feedback round 2
- Review version 2 (2020-09-04)
Stats: 23038 lines in 170 files changed: 14916 ins; 6912 del; 1210 mod
Fetch: git fetch https://git.openjdk.java.net/jdk pull/336/head:pull/336
More information about the hotspot-runtime-dev