RFR(s-m): 8218988: Improve metaspace verifications
Thomas Stüfe
thomas.stuefe at gmail.com
Fri Feb 15 10:12:03 UTC 2019
Hi all,
may I please have reviews for the following change:
Bug: https://bugs.openjdk.java.net/browse/JDK-8218988
cr:
http://cr.openjdk.java.net/~stuefe/webrevs/8218988--improve-metaspace-verifications/webrev.00/webrev/
In short, this reworks metaspace verifications.
Thorough verifications were added for JDK-8198423, but turned out too slow
in general, so were made switchable later (-XX:+VerifyMetaspace) with a
switch which is usually off. However, it would be good to have these
verifications at least sometimes, so this patch changes VerifyMetaspace
from a boolean to an interval which causes verifications every n passes
thru a verification callsite. The default is 500, which does not slow down
metaspace-heavy tests like SelectionResolution noticably.
This patch also contains a number of cleanups and simplifications. In
particular:
class ChunkManager:
- removed sum_free_chunks(), sum_free_chunks_count(),
locked_verify_free_chunks_total() and locked_verify_free_chunks_count().
These functions were used to validate the counters in ChunkManager. But
this way was inefficient, since we walk the lists twice. The counter
validation is now done in ChunkManager::verify() and only requires one list
walk. Also, less code.
- Now we only have one verification function left, ChunkManager::verify().
To distinguish between quick counter verify and deep costly verify, I gave
it a "bool slow" argument.
- all verifications are now #ifdef ASSERT. In fact, they were before too
since we used assert() inside the verification functions.
- expensive verifications are now done depending on VerifyMetaspaceInterval.
- I removed expensive verifications from some hot paths (e.g.
ChunkManager::chunk_freelist_allocate). Verifications are now done only
when interesting things happen: chunk fuses or splits, or a Classloader
died and releases its SpaceManager.
minor stuff:
- made getters for counters (free_chunks_total_words(),
free_chunks_total_bytes(), free_chunks_count()) inline
- removed locked_print_sum_free_chunks(), which was nowhere used.
- constified versions of free_chunks() and humongous_dictionary()
class VirtualSpaceNode:
- removed container_count_slow() and verify_container_count() since it
duplicates the verifications we do in VirtualSpaceNode::verify().
- similar to ChunkManager, gave VirtualSpaceNode::verify a "bool slow"
parameter to distinguish between quick and thorough verification.
- modified VirtualSpaceNode::verify_free_chunks_are_ideally_merged() to
print out a map of the node if it fails, before asserting.
Miscellanous:
- added some new internal counters to the statistic (see
metaspaceCommon.hpp)
Gtests: test_virtualSpaceNode.cpp
- renamed ChunkManagerTest to ChunkManagerTestAccessor to make its (narrow)
intent clearer
- removed parts which had been commented out since chunk coalescation
(8198423) and which would never really work, plus code which relied on it.
Note: IMHO the value of these tests is a bit diminished and probably not
worth the trouble of maintaining? To compensate for that I did add
test_metaspace_allocation.cpp some time ago which is more thorough and less
vulnerable against changes in the metaspace implementation.
----
Tests: I stressed my patch locally (linux x64) with my own metaspace tests
as well as hotspot/jtreg/runtime/*, including the SelectionResolution
tests. Before pushing, I will put the patch through our nightlies.
Thank you, Thomas
More information about the hotspot-runtime-dev
mailing list