Skip to content

Commit

Permalink
8307958: Metaspace verification is slow causing extreme class unloadi…
Browse files Browse the repository at this point in the history
…ng times

Reviewed-by: stuefe, coleenp
  • Loading branch information
xmas92 committed May 25, 2023
1 parent d877134 commit 8d8153e
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 34 deletions.
22 changes: 11 additions & 11 deletions src/hotspot/share/memory/metaspace/chunkManager.cpp
Expand Up @@ -49,7 +49,7 @@ namespace metaspace {
// Return a single chunk to the freelist and adjust accounting. No merge is attempted.
void ChunkManager::return_chunk_simple_locked(Metachunk* c) {
assert_lock_strong(Metaspace_lock);
DEBUG_ONLY(c->verify());
SOMETIMES(c->verify();)
_chunks.add(c);
c->reset_used_words();
// Tracing
Expand Down Expand Up @@ -82,7 +82,7 @@ void ChunkManager::split_chunk_and_add_splinters(Metachunk* c, chunklevel_t targ
assert(c->prev() == nullptr && c->next() == nullptr, "Chunk must be outside of any list.");

DEBUG_ONLY(chunklevel::check_valid_level(target_level);)
DEBUG_ONLY(c->verify();)
SOMETIMES(c->verify();)

UL2(debug, "splitting chunk " METACHUNK_FORMAT " to " CHKLVL_FORMAT ".",
METACHUNK_FORMAT_ARGS(c), target_level);
Expand All @@ -101,8 +101,8 @@ void ChunkManager::split_chunk_and_add_splinters(Metachunk* c, chunklevel_t targ
} else {
assert(c->committed_words() == committed_words_before, "Sanity");
}
c->verify();
verify_locked();
SOMETIMES(c->verify();)
SOMETIMES(verify_locked();)
SOMETIMES(c->vsnode()->verify_locked();)
#endif
InternalStats::inc_num_chunk_splits();
Expand Down Expand Up @@ -136,7 +136,7 @@ Metachunk* ChunkManager::get_chunk(chunklevel_t preferred_level, chunklevel_t ma
// This may be either the GC threshold or MaxMetaspaceSize.
Metachunk* ChunkManager::get_chunk_locked(chunklevel_t preferred_level, chunklevel_t max_level, size_t min_committed_words) {
assert_lock_strong(Metaspace_lock);
DEBUG_ONLY(verify_locked();)
SOMETIMES(verify_locked();)
DEBUG_ONLY(chunklevel::check_valid_level(max_level);)
DEBUG_ONLY(chunklevel::check_valid_level(preferred_level);)

Expand Down Expand Up @@ -255,8 +255,8 @@ void ChunkManager::return_chunk(Metachunk* c) {
void ChunkManager::return_chunk_locked(Metachunk* c) {
assert_lock_strong(Metaspace_lock);
UL2(debug, ": returning chunk " METACHUNK_FORMAT ".", METACHUNK_FORMAT_ARGS(c));
DEBUG_ONLY(c->verify();)
assert(contains_chunk(c) == false, "A chunk to be added to the freelist must not be in the freelist already.");
SOMETIMES(c->verify();)
ASSERT_SOMETIMES(contains_chunk(c) == false, "A chunk to be added to the freelist must not be in the freelist already.");
assert(c->is_in_use() || c->is_free(), "Unexpected chunk state");
assert(!c->in_list(), "Remove from list first");

Expand All @@ -272,15 +272,15 @@ void ChunkManager::return_chunk_locked(Metachunk* c) {

if (merged != nullptr) {
InternalStats::inc_num_chunk_merges();
DEBUG_ONLY(merged->verify());
SOMETIMES(merged->verify();)
// We did merge chunks and now have a bigger chunk.
assert(merged->level() < orig_lvl, "Sanity");
UL2(debug, "merged into chunk " METACHUNK_FORMAT ".", METACHUNK_FORMAT_ARGS(merged));
c = merged;
}

return_chunk_simple_locked(c);
DEBUG_ONLY(verify_locked();)
SOMETIMES(verify_locked();)
SOMETIMES(c->vsnode()->verify_locked();)
InternalStats::inc_num_chunks_returned_to_freelist();
}
Expand Down Expand Up @@ -373,8 +373,8 @@ void ChunkManager::purge() {
ls.cr();
}
}
DEBUG_ONLY(_vslist->verify_locked());
DEBUG_ONLY(verify_locked());
SOMETIMES(_vslist->verify_locked();)
SOMETIMES(verify_locked();)
}

// Convenience methods to return the global class-space chunkmanager
Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/share/memory/metaspace/freeChunkList.hpp
Expand Up @@ -31,6 +31,7 @@
#include "memory/metaspace/counters.hpp"
#include "memory/metaspace/metachunk.hpp"
#include "memory/metaspace/metachunkList.hpp"
#include "memory/metaspace/metaspaceCommon.hpp"

class outputStream;

Expand Down Expand Up @@ -102,7 +103,7 @@ class FreeChunkList {

// Remove given chunk from anywhere in the list.
Metachunk* remove(Metachunk* c) {
assert(contains(c), "Must be contained here");
ASSERT_SOMETIMES(contains(c), "Must be contained here");
Metachunk* pred = c->prev();
Metachunk* succ = c->next();
if (pred) {
Expand All @@ -124,7 +125,7 @@ class FreeChunkList {
}

void add(Metachunk* c) {
assert(contains(c) == false, "Chunk already in freelist");
ASSERT_SOMETIMES(contains(c) == false, "Chunk already in freelist");
assert(_first == nullptr || _first->level() == c->level(),
"List should only contains chunks of the same level.");
// Uncommitted chunks go to the back, fully or partially committed to the front.
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/share/memory/metaspace/metaspaceArena.cpp
Expand Up @@ -129,9 +129,9 @@ MetaspaceArena::MetaspaceArena(ChunkManager* chunk_manager, const ArenaGrowthPol

MetaspaceArena::~MetaspaceArena() {
#ifdef ASSERT
verify();
SOMETIMES(verify();)
if (Settings::use_allocation_guard()) {
verify_allocation_guards();
SOMETIMES(verify_allocation_guards();)
}
#endif

Expand All @@ -156,7 +156,7 @@ MetaspaceArena::~MetaspaceArena() {
return_counter.count(), return_counter.total_size());

_total_used_words_counter->decrement_by(return_counter.total_size());
DEBUG_ONLY(chunk_manager()->verify();)
SOMETIMES(chunk_manager()->verify();)
delete _fbl;
UL(debug, ": dies.");

Expand Down Expand Up @@ -368,7 +368,7 @@ void MetaspaceArena::deallocate_locked(MetaWord* p, size_t word_size) {
size_t raw_word_size = get_raw_word_size_for_requested_word_size(word_size);
add_allocation_to_fbl(p, raw_word_size);

DEBUG_ONLY(verify_locked();)
SOMETIMES(verify_locked();)
}

// Prematurely returns a metaspace allocation to the _block_freelists because it is not
Expand Down
22 changes: 11 additions & 11 deletions src/hotspot/share/memory/metaspace/rootChunkArea.cpp
Expand Up @@ -98,7 +98,7 @@ void RootChunkArea::split(chunklevel_t target_level, Metachunk* c, FreeChunkList
//

DEBUG_ONLY(check_pointer(c->base());)
DEBUG_ONLY(c->verify();)
SOMETIMES(c->verify();)
assert(c->is_free(), "Can only split free chunks.");

DEBUG_ONLY(chunklevel::check_valid_level(target_level));
Expand Down Expand Up @@ -139,8 +139,8 @@ void RootChunkArea::split(chunklevel_t target_level, Metachunk* c, FreeChunkList

assert(c->level() == target_level, "Sanity");

DEBUG_ONLY(verify();)
DEBUG_ONLY(c->verify();)
SOMETIMES(verify();)
SOMETIMES(c->verify();)
}

// Given a chunk, attempt to merge it recursively with its neighboring chunks.
Expand Down Expand Up @@ -194,7 +194,7 @@ Metachunk* RootChunkArea::merge(Metachunk* c, FreeChunkListVector* freelists) {
assert(!c->is_root_chunk(), "Cannot be merged further.");
assert(c->is_free(), "Can only merge free chunks.");

DEBUG_ONLY(c->verify();)
SOMETIMES(c->verify();)

log_trace(metaspace)("Attempting to merge chunk " METACHUNK_FORMAT ".", METACHUNK_FORMAT_ARGS(c));

Expand All @@ -208,7 +208,7 @@ Metachunk* RootChunkArea::merge(Metachunk* c, FreeChunkListVector* freelists) {

// Note: this is either our buddy or a splinter of the buddy.
Metachunk* const buddy = c->is_leader() ? c->next_in_vs() : c->prev_in_vs();
DEBUG_ONLY(buddy->verify();)
SOMETIMES(buddy->verify();)

// A buddy chunk must be of the same or higher level (so, same size or smaller)
// never be larger.
Expand Down Expand Up @@ -270,14 +270,14 @@ Metachunk* RootChunkArea::merge(Metachunk* c, FreeChunkListVector* freelists) {
}

result = c = leader;
DEBUG_ONLY(leader->verify();)
SOMETIMES(leader->verify();)
}
} while (!stop);

#ifdef ASSERT
verify();
SOMETIMES(verify();)
if (result != nullptr) {
result->verify();
SOMETIMES(result->verify();)
}
#endif // ASSERT
return result;
Expand All @@ -299,15 +299,15 @@ bool RootChunkArea::attempt_enlarge_chunk(Metachunk* c, FreeChunkListVector* fre
// There is no real reason for this limitation other than it is not
// needed on free chunks since they should be merged already:
assert(c->is_in_use(), "Can only enlarge in use chunks.");
DEBUG_ONLY(c->verify();)
SOMETIMES(c->verify();)

if (!c->is_leader()) {
return false;
}

// We are the leader, so the buddy must follow us.
Metachunk* const buddy = c->next_in_vs();
DEBUG_ONLY(buddy->verify();)
SOMETIMES(buddy->verify();)

// Of course buddy cannot be larger than us.
assert(buddy->level() >= c->level(), "Sanity");
Expand Down Expand Up @@ -352,7 +352,7 @@ bool RootChunkArea::attempt_enlarge_chunk(Metachunk* c, FreeChunkListVector* fre

log_debug(metaspace)("Enlarged chunk " METACHUNK_FULL_FORMAT ".", METACHUNK_FULL_FORMAT_ARGS(c));

DEBUG_ONLY(verify());
SOMETIMES(verify();)
return true;
}

Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp
Expand Up @@ -322,7 +322,7 @@ Metachunk* VirtualSpaceNode::allocate_root_chunk() {
Metachunk* c = rca->alloc_root_chunk_header(this);
assert(c->base() == loc && c->vsnode() == this &&
c->is_free(), "Sanity");
DEBUG_ONLY(c->verify();)
SOMETIMES(c->verify();)

UL2(debug, "new root chunk " METACHUNK_FORMAT ".", METACHUNK_FORMAT_ARGS(c));
return c;
Expand All @@ -338,7 +338,7 @@ void VirtualSpaceNode::split(chunklevel_t target_level, Metachunk* c, FreeChunkL
assert_lock_strong(Metaspace_lock);
// Get the area associated with this chunk and let it handle the splitting
RootChunkArea* rca = _root_chunk_area_lut.get_area_by_address(c->base());
DEBUG_ONLY(rca->verify_area_is_ideally_merged();)
SOMETIMES(rca->verify_area_is_ideally_merged();)
rca->split(target_level, c, freelists);
}

Expand All @@ -358,7 +358,7 @@ Metachunk* VirtualSpaceNode::merge(Metachunk* c, FreeChunkListVector* freelists)
// Get the rca associated with this chunk and let it handle the merging
RootChunkArea* rca = _root_chunk_area_lut.get_area_by_address(c->base());
Metachunk* c2 = rca->merge(c, freelists);
DEBUG_ONLY(rca->verify_area_is_ideally_merged();)
SOMETIMES(rca->verify_area_is_ideally_merged();)
return c2;
}

Expand All @@ -379,7 +379,7 @@ bool VirtualSpaceNode::attempt_enlarge_chunk(Metachunk* c, FreeChunkListVector*
RootChunkArea* rca = _root_chunk_area_lut.get_area_by_address(c->base());

bool rc = rca->attempt_enlarge_chunk(c, freelists);
DEBUG_ONLY(rca->verify_area_is_ideally_merged();)
SOMETIMES(rca->verify_area_is_ideally_merged();)
if (rc) {
InternalStats::inc_num_chunks_enlarged();
}
Expand Down
4 changes: 2 additions & 2 deletions test/hotspot/jtreg/gtest/MetaspaceGtests.java
Expand Up @@ -37,7 +37,7 @@
* java.xml
* @requires vm.debug
* @requires vm.flagless
* @run main/native GTestWrapper --gtest_filter=metaspace* -XX:+UnlockDiagnosticVMOptions -XX:VerifyMetaspaceInterval=3
* @run main/native GTestWrapper --gtest_filter=metaspace* -XX:+UnlockDiagnosticVMOptions -XX:VerifyMetaspaceInterval=1
*/

/* @test id=balanced-with-guards
Expand All @@ -47,7 +47,7 @@
* java.xml
* @requires vm.debug
* @requires vm.flagless
* @run main/native GTestWrapper --gtest_filter=metaspace* -XX:VerifyMetaspaceInterval=3 -XX:+MetaspaceGuardAllocations
* @run main/native GTestWrapper --gtest_filter=metaspace* -XX:VerifyMetaspaceInterval=1 -XX:+MetaspaceGuardAllocations
*/

/* @test id=balanced-no-ccs
Expand Down

1 comment on commit 8d8153e

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.