RFR: 8251158: Implementation of JEP 387: Elastic Metaspace
Albert Yang
albert.m.yang at oracle.com
Sun Aug 23 22:03:43 UTC 2020
Hi Thomas,
> error handling is just simpler and cleaner if ChunkManager::get_chunk() never hands out
a fully uncommitted chunk in the first place
I see. Then, when the current chunk is salvaged, it must have a positive used words, as
the first request will always be satisfied, right? I added `assert(false)` in
`MetaspaceArena::salvage_chunk()`, running `make test TEST='gtest:metaspace
test/hotspot/jtreg/gtest/MetaspaceGtests.java
test/hotspot/jtreg/runtime/Metaspace/elastic'` doesn't trigger the assertion. Is it truly
unreachable or am I missing some tests?
```
if (c->used_words() == 0) {
assert(false, ""); // newly added
UL2(trace, "salvage: returning empty chunk " METACHUNK_FORMAT ".",
METACHUNK_FORMAT_ARGS(c));
_chunk_manager->return_chunk(c);
return;
}
```
Memory allocation in metaspace involves both virtual (potentially enlarging a chunk) and
physical (potentially committing memory) address space, and either one could fail. The
second step, if failed, needs to undo the first step as well. Such tight-coupling is the
main reason (to me) to not expose this as two steps to the caller. As I read it,
`MetaspaceArena::allocate()` actually doesn't release the virtual memory used in the first
if the second step fails. Not sure if there are any consequences or the severity.
I wonder if the problem could be simpler if chunks smaller/larger than the commit granule
are dealt with in two different ways. This way when we deal with chunks smaller than the
commit granule, we know it's already committed.
> In other words, chunks can be smaller than a commit granule and that makes sense since
for small loaders we want to share a granule among multiple loaders.
It makes sense. Somehow, I got the impression that MetaspaceArena has 1:1 mapping to
ChunkManager, but actually all arenas share the singleton class/non-class chunk manager.
The diagram in
https://cr.openjdk.java.net/~stuefe/JEP-Improve-Metaspace-Allocator/review-guide/review-guide-1.0.html#2-subsystems
was quite clear, but I think I ignored it after failing to find "SpaceManager" in the
code. (Maybe the diagram is out of sync with the code?) Thank you for the explanation,
which, I believe, should be included in the comments for documentation purpose. (I didn't
find the rational for using a buddy allocator in the code comments.)
Thank you once again for the clarification; I have got a better understanding of the patch.
--
/Albert
More information about the hotspot-runtime-dev
mailing list