# HG changeset patch # User stuefe # Date 1549786242 -3600 # Sun Feb 10 09:10:42 2019 +0100 # Node ID fa534b143c5ff72df750cf3d19a617b075756099 # Parent d667de4734acdb6e530859bb38a368eb21c88cad imported patch metaspace-cleanups diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace.cpp --- a/src/hotspot/share/memory/metaspace.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -440,7 +440,6 @@ if (chunk_manager == NULL) { return 0; } - chunk_manager->slow_verify(); return chunk_manager->free_chunks_total_words(); } @@ -793,6 +792,13 @@ out->print_cr("Number of times virtual space nodes were expanded: " UINTX_FORMAT ".", g_internal_statistics.num_committed_space_expanded); out->print_cr("Number of deallocations: " UINTX_FORMAT " (" UINTX_FORMAT " external).", g_internal_statistics.num_deallocs, g_internal_statistics.num_external_deallocs); out->print_cr("Allocations from deallocated blocks: " UINTX_FORMAT ".", g_internal_statistics.num_allocs_from_deallocated_blocks); + out->print_cr("Number of chunks added to freelist: " UINTX_FORMAT ".", + g_internal_statistics.num_chunks_added_to_freelist); + out->print_cr("Number of chunks removed from freelist: " UINTX_FORMAT ".", + g_internal_statistics.num_chunks_removed_from_freelist); + out->print_cr("Number of chunk merges: " UINTX_FORMAT ", split-ups: " UINTX_FORMAT ".", + g_internal_statistics.num_chunk_merges, g_internal_statistics.num_chunk_splits); + out->cr(); #endif @@ -844,10 +850,12 @@ } void MetaspaceUtils::verify_free_chunks() { - Metaspace::chunk_manager_metadata()->verify(); +#ifdef ASSERT + Metaspace::chunk_manager_metadata()->verify(false); if (Metaspace::using_class_space()) { - Metaspace::chunk_manager_class()->verify(); + Metaspace::chunk_manager_class()->verify(false); } +#endif } void MetaspaceUtils::verify_metrics() { diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/chunkManager.cpp --- a/src/hotspot/share/memory/metaspace/chunkManager.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/chunkManager.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -29,6 +29,7 @@ #include "memory/freeList.inline.hpp" #include "memory/metaspace/chunkManager.hpp" #include "memory/metaspace/metachunk.hpp" +#include "memory/metaspace/metaDebug.hpp" #include "memory/metaspace/metaspaceCommon.hpp" #include "memory/metaspace/metaspaceStatistics.hpp" #include "memory/metaspace/occupancyMap.hpp" @@ -140,15 +141,21 @@ _free_chunks_count -= num_chunks_removed; _free_chunks_count ++; - // VirtualSpaceNode::container_count does not have to be modified: + // VirtualSpaceNode::chunk_count does not have to be modified: // it means "number of active (non-free) chunks", so merging free chunks // should not affect that count. // At the end of a chunk merge, run verification tests. - if (VerifyMetaspace) { - DEBUG_ONLY(this->locked_verify()); - DEBUG_ONLY(vsn->verify()); - } +#ifdef ASSERT + + EVERY_NTH(VerifyMetaspaceInterval) + locked_verify(true); + vsn->verify(true); + END_EVERY_NTH + + g_internal_statistics.num_chunk_merges ++; + +#endif return true; } @@ -189,14 +196,6 @@ return num_removed; } -size_t ChunkManager::free_chunks_total_words() { - return _free_chunks_total; -} - -size_t ChunkManager::free_chunks_total_bytes() { - return free_chunks_total_words() * BytesPerWord; -} - // Update internal accounting after a chunk was added void ChunkManager::account_for_added_chunk(const Metachunk* c) { assert_lock_strong(MetaspaceExpand_lock); @@ -216,19 +215,6 @@ _free_chunks_total -= c->word_size(); } -size_t ChunkManager::free_chunks_count() { -#ifdef ASSERT - if (!UseConcMarkSweepGC && !MetaspaceExpand_lock->is_locked()) { - MutexLockerEx cl(MetaspaceExpand_lock, - Mutex::_no_safepoint_check_flag); - // This lock is only needed in debug because the verification - // of the _free_chunks_totals walks the list of free chunks - slow_locked_verify_free_chunks_count(); - } -#endif - return _free_chunks_count; -} - ChunkIndex ChunkManager::list_index(size_t size) { return get_chunk_type_by_size(size, is_class()); } @@ -239,43 +225,48 @@ return get_size_for_nonhumongous_chunktype(index, is_class()); } -void ChunkManager::locked_verify_free_chunks_total() { - assert_lock_strong(MetaspaceExpand_lock); - assert(sum_free_chunks() == _free_chunks_total, - "_free_chunks_total " SIZE_FORMAT " is not the" - " same as sum " SIZE_FORMAT, _free_chunks_total, - sum_free_chunks()); +#ifdef ASSERT +void ChunkManager::verify(bool slow) const { + MutexLockerEx cl(MetaspaceExpand_lock, + Mutex::_no_safepoint_check_flag); + locked_verify(slow); } -void ChunkManager::locked_verify_free_chunks_count() { +void ChunkManager::locked_verify(bool slow) const { + log_trace(gc, metaspace, freelist)("verifying %s chunkmanager (%s).", + (is_class() ? "class space" : "metaspace"), (slow ? "slow" : "quick")); + assert_lock_strong(MetaspaceExpand_lock); - assert(sum_free_chunks_count() == _free_chunks_count, - "_free_chunks_count " SIZE_FORMAT " is not the" - " same as sum " SIZE_FORMAT, _free_chunks_count, - sum_free_chunks_count()); -} -void ChunkManager::verify() { - MutexLockerEx cl(MetaspaceExpand_lock, - Mutex::_no_safepoint_check_flag); - locked_verify(); -} - -void ChunkManager::locked_verify() { - locked_verify_free_chunks_count(); - locked_verify_free_chunks_total(); + size_t chunks_counted = 0; + size_t wordsize_chunks_counted = 0; for (ChunkIndex i = ZeroIndex; i < NumberOfFreeLists; i = next_chunk_index(i)) { - ChunkList* list = free_chunks(i); + const ChunkList* list = free_chunks(i); if (list != NULL) { Metachunk* chunk = list->head(); while (chunk) { - DEBUG_ONLY(do_verify_chunk(chunk);) + if (slow) { + do_verify_chunk(chunk); + } assert(chunk->is_tagged_free(), "Chunk should be tagged as free."); + chunks_counted ++; + wordsize_chunks_counted += chunk->size(); chunk = chunk->next(); } } } + + chunks_counted += humongous_dictionary()->total_free_blocks(); + wordsize_chunks_counted += humongous_dictionary()->total_size(); + + assert(chunks_counted == _free_chunks_count && wordsize_chunks_counted == _free_chunks_total, + "freelist accounting mismatch: " + "we think: " SIZE_FORMAT " chunks, total " SIZE_FORMAT " words, " + "reality: " SIZE_FORMAT " chunks, total " SIZE_FORMAT " words.", + _free_chunks_count, _free_chunks_total, + chunks_counted, wordsize_chunks_counted); } +#endif void ChunkManager::locked_print_free_chunks(outputStream* st) { assert_lock_strong(MetaspaceExpand_lock); @@ -283,49 +274,14 @@ _free_chunks_total, _free_chunks_count); } -void ChunkManager::locked_print_sum_free_chunks(outputStream* st) { - assert_lock_strong(MetaspaceExpand_lock); - st->print_cr("Sum free chunk total " SIZE_FORMAT " count " SIZE_FORMAT, - sum_free_chunks(), sum_free_chunks_count()); -} - ChunkList* ChunkManager::free_chunks(ChunkIndex index) { assert(index == SpecializedIndex || index == SmallIndex || index == MediumIndex, "Bad index: %d", (int)index); - return &_free_chunks[index]; } -// These methods that sum the free chunk lists are used in printing -// methods that are used in product builds. -size_t ChunkManager::sum_free_chunks() { - assert_lock_strong(MetaspaceExpand_lock); - size_t result = 0; - for (ChunkIndex i = ZeroIndex; i < NumberOfFreeLists; i = next_chunk_index(i)) { - ChunkList* list = free_chunks(i); - - if (list == NULL) { - continue; - } - - result = result + list->count() * list->size(); - } - result = result + humongous_dictionary()->total_size(); - return result; -} - -size_t ChunkManager::sum_free_chunks_count() { - assert_lock_strong(MetaspaceExpand_lock); - size_t count = 0; - for (ChunkIndex i = ZeroIndex; i < NumberOfFreeLists; i = next_chunk_index(i)) { - ChunkList* list = free_chunks(i); - if (list == NULL) { - continue; - } - count = count + list->count(); - } - count = count + humongous_dictionary()->total_free_blocks(); - return count; +const ChunkList* ChunkManager::free_chunks(ChunkIndex index) const { + return const_cast(this)->free_chunks(index); } ChunkList* ChunkManager::find_free_chunks_list(size_t word_size) { @@ -427,14 +383,18 @@ } + // Note: at this point, the VirtualSpaceNode is invalid since we split a chunk and + // did not yet hand out part of that split; so, vsn->verify_free_chunks_are_ideally_merged() + // would assert. Instead, do all verifications in the caller. + + DEBUG_ONLY(g_internal_statistics.num_chunk_splits ++); + return target_chunk; } Metachunk* ChunkManager::free_chunks_get(size_t word_size) { assert_lock_strong(MetaspaceExpand_lock); - slow_locked_verify(); - Metachunk* chunk = NULL; bool we_did_split_a_chunk = false; @@ -515,7 +475,7 @@ // Chunk has been removed from the chunk manager; update counters. account_for_removed_chunk(chunk); do_update_in_use_info_for_chunk(chunk, true); - chunk->container()->inc_container_count(); + chunk->container()->inc_chunk_count(); chunk->inc_use_count(); // Remove it from the links to this freelist @@ -524,14 +484,19 @@ // Run some verifications (some more if we did a chunk split) #ifdef ASSERT - if (VerifyMetaspace) { - locked_verify(); + + EVERY_NTH(VerifyMetaspaceInterval) + // Be extra verify-y when chunk split happened. + locked_verify(true); VirtualSpaceNode* const vsn = chunk->container(); - vsn->verify(); + vsn->verify(true); if (we_did_split_a_chunk) { vsn->verify_free_chunks_are_ideally_merged(); } - } + END_EVERY_NTH + + g_internal_statistics.num_chunks_removed_from_freelist ++; + #endif return chunk; @@ -539,7 +504,6 @@ Metachunk* ChunkManager::chunk_freelist_allocate(size_t word_size) { assert_lock_strong(MetaspaceExpand_lock); - slow_locked_verify(); // Take from the beginning of the list Metachunk* chunk = free_chunks_get(word_size); @@ -570,9 +534,17 @@ } void ChunkManager::return_single_chunk(Metachunk* chunk) { + +#ifdef ASSERT + EVERY_NTH(VerifyMetaspaceInterval) + this->locked_verify(false); + do_verify_chunk(chunk); + END_EVERY_NTH +#endif + const ChunkIndex index = chunk->get_chunk_type(); assert_lock_strong(MetaspaceExpand_lock); - DEBUG_ONLY(do_verify_chunk(chunk);) + DEBUG_ONLY(g_internal_statistics.num_chunks_added_to_freelist ++;) assert(chunk != NULL, "Expected chunk."); assert(chunk->container() != NULL, "Container should have been set."); assert(chunk->is_tagged_free() == false, "Chunk should be in use."); @@ -583,6 +555,9 @@ // keeps tree node pointers in the chunk payload area which mangle will overwrite. DEBUG_ONLY(chunk->mangle(badMetaWordVal);) + // may need node for verification later after chunk may have been merged away. + DEBUG_ONLY(VirtualSpaceNode* vsn = chunk->container(); ) + if (index != HumongousIndex) { // Return non-humongous chunk to freelist. ChunkList* list = free_chunks(index); @@ -599,7 +574,7 @@ log_trace(gc, metaspace, freelist)("returned one %s chunk at " PTR_FORMAT " (word size " SIZE_FORMAT ") to freelist.", chunk_size_name(index), p2i(chunk), chunk->word_size()); } - chunk->container()->dec_container_count(); + chunk->container()->dec_chunk_count(); do_update_in_use_info_for_chunk(chunk, false); // Chunk has been added; update counters. @@ -618,6 +593,16 @@ } } + // From here on do not access chunk anymore, it may have been merged with another chunk. + +#ifdef ASSERT + EVERY_NTH(VerifyMetaspaceInterval) + this->locked_verify(true); + vsn->verify(true); + vsn->verify_free_chunks_are_ideally_merged(); + END_EVERY_NTH +#endif + } void ChunkManager::return_chunk_list(Metachunk* chunks) { diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/chunkManager.hpp --- a/src/hotspot/share/memory/metaspace/chunkManager.hpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/chunkManager.hpp Sun Feb 10 09:10:42 2019 +0100 @@ -33,7 +33,7 @@ #include "memory/metaspaceChunkFreeListSummary.hpp" #include "utilities/globalDefinitions.hpp" -class ChunkManagerTest; +class ChunkManagerTestAccessor; namespace metaspace { @@ -42,7 +42,7 @@ // Manages the global free lists of chunks. class ChunkManager : public CHeapObj { - friend class ::ChunkManagerTest; + friend class ::ChunkManagerTestAccessor; // Free list of chunks of different sizes. // SpecializedChunk @@ -55,6 +55,7 @@ // Return non-humongous chunk list by its index. ChunkList* free_chunks(ChunkIndex index); + const ChunkList* free_chunks(ChunkIndex index) const; // Returns non-humongous chunk list for the given chunk word size. ChunkList* find_free_chunks_list(size_t word_size); @@ -63,9 +64,8 @@ ChunkTreeDictionary _humongous_dictionary; // Returns the humongous chunk dictionary. - ChunkTreeDictionary* humongous_dictionary() { - return &_humongous_dictionary; - } + ChunkTreeDictionary* humongous_dictionary() { return &_humongous_dictionary; } + const ChunkTreeDictionary* humongous_dictionary() const { return &_humongous_dictionary; } // Size, in metaspace words, of all chunks managed by this ChunkManager size_t _free_chunks_total; @@ -76,22 +76,6 @@ void account_for_added_chunk(const Metachunk* c); void account_for_removed_chunk(const Metachunk* c); - size_t sum_free_chunks(); - size_t sum_free_chunks_count(); - - void locked_verify_free_chunks_total(); - void slow_locked_verify_free_chunks_total() { - if (VerifyMetaspace) { - locked_verify_free_chunks_total(); - } - } - void locked_verify_free_chunks_count(); - void slow_locked_verify_free_chunks_count() { - if (VerifyMetaspace) { - locked_verify_free_chunks_count(); - } - } - // Given a pointer to a chunk, attempts to merge it with neighboring // free chunks to form a bigger chunk. Returns true if successful. bool attempt_to_coalesce_around_chunk(Metachunk* chunk, ChunkIndex target_chunk_type); @@ -147,11 +131,11 @@ void return_chunk_list(Metachunk* chunk); // Total of the space in the free chunks list - size_t free_chunks_total_words(); - size_t free_chunks_total_bytes(); + size_t free_chunks_total_words() const { return _free_chunks_total; } + size_t free_chunks_total_bytes() const { return free_chunks_total_words() * BytesPerWord; } // Number of chunks in the free chunks list - size_t free_chunks_count(); + size_t free_chunks_count() const { return _free_chunks_count; } // Remove from a list by size. Selects list based on size of chunk. Metachunk* free_chunks_get(size_t chunk_word_size); @@ -195,22 +179,14 @@ size_free_chunks_in_bytes(HumongousIndex)); } +#ifdef ASSERT // Debug support - void verify(); - void slow_verify() { - if (VerifyMetaspace) { - verify(); - } - } - void locked_verify(); - void slow_locked_verify() { - if (VerifyMetaspace) { - locked_verify(); - } - } + // Verify free list integrity. slow=true: verify chunk-internal integrity too. + void verify(bool slow) const; + void locked_verify(bool slow) const; +#endif void locked_print_free_chunks(outputStream* st); - void locked_print_sum_free_chunks(outputStream* st); // Fill in current statistic values to the given statistics object. void collect_statistics(ChunkManagerStatistics* out) const; diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/metaDebug.cpp --- a/src/hotspot/share/memory/metaspace/metaDebug.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/metaDebug.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -57,7 +57,6 @@ } #endif - } // namespace metaspace diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/metaDebug.hpp --- a/src/hotspot/share/memory/metaspace/metaDebug.hpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/metaDebug.hpp Sun Feb 10 09:10:42 2019 +0100 @@ -41,6 +41,17 @@ #endif }; +#ifdef ASSERT +#define EVERY_NTH(n) \ +{ static int counter_ = 0; \ + if (n > 0) { \ + counter_ ++; \ + if (counter_ > n) { \ + counter_ = 0; \ + +#define END_EVERY_NTH } } } +#endif // ASSERT + } // namespace metaspace #endif // SHARE_MEMORY_METASPACE_METADEBUG_HPP diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/metaspaceCommon.hpp --- a/src/hotspot/share/memory/metaspace/metaspaceCommon.hpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/metaspaceCommon.hpp Sun Feb 10 09:10:42 2019 +0100 @@ -85,6 +85,14 @@ uintx num_external_deallocs; // Number of times an allocation was satisfied from deallocated blocks. uintx num_allocs_from_deallocated_blocks; + // Number of times a chunk was added to the freelist + uintx num_chunks_added_to_freelist; + // Number of times a chunk was removed from the freelist + uintx num_chunks_removed_from_freelist; + // Number of chunk merges + uintx num_chunk_merges; + // Number of chunk splits + uintx num_chunk_splits; }; extern internal_statistics_t g_internal_statistics; #endif @@ -111,6 +119,16 @@ // Returns a descriptive name for a chunk type. const char* chunk_size_name(ChunkIndex index); +// Verify chunk sizes. +inline bool is_valid_chunksize(bool is_class, size_t size) { + const size_t reasonable_maximum_humongous_chunk_size = 1 * G; + return is_aligned(size, sizeof(MetaWord)) && + size < reasonable_maximum_humongous_chunk_size && + is_class ? + (size == ClassSpecializedChunk || size == ClassSmallChunk || size >= ClassMediumChunk) : + (size == SpecializedChunk || size == SmallChunk || size >= MediumChunk); +} + // Verify chunk type. inline bool is_valid_chunktype(ChunkIndex index) { return index == SpecializedIndex || index == SmallIndex || diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/spaceManager.cpp --- a/src/hotspot/share/memory/metaspace/spaceManager.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/spaceManager.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -287,8 +287,6 @@ MutexLockerEx fcl(MetaspaceExpand_lock, Mutex::_no_safepoint_check_flag); - chunk_manager()->slow_locked_verify(); - account_for_spacemanager_death(); Log(gc, metaspace, freelist) log; @@ -313,7 +311,11 @@ _current_chunk = NULL; #endif - chunk_manager()->slow_locked_verify(); +#ifdef ASSERT + EVERY_NTH(VerifyMetaspaceInterval) + chunk_manager()->locked_verify(true); + END_EVERY_NTH +#endif if (_block_freelists != NULL) { delete _block_freelists; @@ -405,8 +407,6 @@ BlockFreelist* fl = block_freelists(); MetaWord* p = NULL; - DEBUG_ONLY(if (VerifyMetaspace) verify_metrics_locked()); - // Allocation from the dictionary is expensive in the sense that // the dictionary has to be searched for a size. Don't allocate // from the dictionary until it starts to get fat. Is this @@ -422,6 +422,12 @@ p = allocate_work(raw_word_size); } +#ifdef ASSERT + EVERY_NTH(VerifyMetaspaceInterval) + verify_metrics_locked(); + END_EVERY_NTH +#endif + return p; } diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/virtualSpaceList.cpp --- a/src/hotspot/share/memory/metaspace/virtualSpaceList.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/virtualSpaceList.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -92,45 +92,45 @@ assert_lock_strong(MetaspaceExpand_lock); // Don't use a VirtualSpaceListIterator because this // list is being changed and a straightforward use of an iterator is not safe. - VirtualSpaceNode* purged_vsl = NULL; - VirtualSpaceNode* prev_vsl = virtual_space_list(); - VirtualSpaceNode* next_vsl = prev_vsl; - while (next_vsl != NULL) { - VirtualSpaceNode* vsl = next_vsl; - DEBUG_ONLY(vsl->verify_container_count();) - next_vsl = vsl->next(); + VirtualSpaceNode* purged_node = NULL; + VirtualSpaceNode* prev_node = virtual_space_list(); + VirtualSpaceNode* next_node = prev_node; + while (next_node != NULL) { + VirtualSpaceNode* node = next_node; + DEBUG_ONLY(node->verify(false);) + next_node = node->next(); // Don't free the current virtual space since it will likely // be needed soon. - if (vsl->container_count() == 0 && vsl != current_virtual_space()) { + if (node->chunk_count() == 0 && node != current_virtual_space()) { log_trace(gc, metaspace, freelist)("Purging VirtualSpaceNode " PTR_FORMAT " (capacity: " SIZE_FORMAT - ", used: " SIZE_FORMAT ").", p2i(vsl), vsl->capacity_words_in_vs(), vsl->used_words_in_vs()); + ", used: " SIZE_FORMAT ").", p2i(node), node->capacity_words_in_vs(), node->used_words_in_vs()); DEBUG_ONLY(Atomic::inc(&g_internal_statistics.num_vsnodes_purged)); // Unlink it from the list - if (prev_vsl == vsl) { + if (prev_node == node) { // This is the case of the current node being the first node. - assert(vsl == virtual_space_list(), "Expected to be the first node"); - set_virtual_space_list(vsl->next()); + assert(node == virtual_space_list(), "Expected to be the first node"); + set_virtual_space_list(node->next()); } else { - prev_vsl->set_next(vsl->next()); + prev_node->set_next(node->next()); } - vsl->purge(chunk_manager); - dec_reserved_words(vsl->reserved_words()); - dec_committed_words(vsl->committed_words()); + node->purge(chunk_manager); + dec_reserved_words(node->reserved_words()); + dec_committed_words(node->committed_words()); dec_virtual_space_count(); - purged_vsl = vsl; - delete vsl; + purged_node = node; + delete node; } else { - prev_vsl = vsl; + prev_node = node; } } #ifdef ASSERT - if (purged_vsl != NULL) { + if (purged_node != NULL) { // List should be stable enough to use an iterator here. VirtualSpaceListIterator iter(virtual_space_list()); while (iter.repeat()) { VirtualSpaceNode* vsl = iter.get_next(); - assert(vsl != purged_vsl, "Purge of vsl failed"); + assert(vsl != purged_node, "Purge of vsl failed"); } } #endif diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp --- a/src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/virtualSpaceNode.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -29,6 +29,7 @@ #include "memory/metaspace/metachunk.hpp" #include "memory/metaspace.hpp" #include "memory/metaspace/chunkManager.hpp" +#include "memory/metaspace/metaDebug.hpp" #include "memory/metaspace/metaspaceCommon.hpp" #include "memory/metaspace/occupancyMap.hpp" #include "memory/metaspace/virtualSpaceNode.hpp" @@ -57,7 +58,7 @@ // byte_size is the size of the associated virtualspace. VirtualSpaceNode::VirtualSpaceNode(bool is_class, size_t bytes) : - _next(NULL), _is_class(is_class), _rs(), _top(NULL), _container_count(0), _occupancy_map(NULL) { + _next(NULL), _is_class(is_class), _rs(), _top(NULL), _chunk_count(0), _occupancy_map(NULL) { assert_is_aligned(bytes, Metaspace::reserve_alignment()); bool large_pages = should_commit_large_pages_when_reserving(bytes); _rs = ReservedSpace(bytes, Metaspace::reserve_alignment(), large_pages); @@ -73,8 +74,9 @@ } void VirtualSpaceNode::purge(ChunkManager* chunk_manager) { - DEBUG_ONLY(this->verify();) - Metachunk* chunk = first_chunk(); + // When a node is purged, lets give it a thorough examination. + DEBUG_ONLY(this->verify(true);) + Metachunk* chunk = first_chunk(); Metachunk* invalid_chunk = (Metachunk*) top(); while (chunk < invalid_chunk ) { assert(chunk->is_tagged_free(), "Should be tagged free"); @@ -171,49 +173,39 @@ #ifdef ASSERT -uintx VirtualSpaceNode::container_count_slow() { - uintx count = 0; - Metachunk* chunk = first_chunk(); - Metachunk* invalid_chunk = (Metachunk*) top(); - while (chunk < invalid_chunk ) { - MetaWord* next = ((MetaWord*)chunk) + chunk->word_size(); - do_verify_chunk(chunk); - // Don't count the chunks on the free lists. Those are - // still part of the VirtualSpaceNode but not currently - // counted. - if (!chunk->is_tagged_free()) { - count++; - } - chunk = (Metachunk*) next; - } - return count; -} -#endif -#ifdef ASSERT // Verify counters, all chunks in this list node and the occupancy map. -void VirtualSpaceNode::verify() { +void VirtualSpaceNode::verify(bool slow) { + log_trace(gc, metaspace, freelist)("verifying %s virtual space node (%s).", + (is_class() ? "class space" : "metaspace"), (slow ? "slow" : "quick")); + // Fast mode: just verify chunk counters and basic geometry + // Slow mode: verify chunks and occupancy map uintx num_in_use_chunks = 0; Metachunk* chunk = first_chunk(); Metachunk* invalid_chunk = (Metachunk*) top(); // Iterate the chunks in this node and verify each chunk. while (chunk < invalid_chunk ) { - DEBUG_ONLY(do_verify_chunk(chunk);) - if (!chunk->is_tagged_free()) { - num_in_use_chunks ++; - } - MetaWord* next = ((MetaWord*)chunk) + chunk->word_size(); + if (slow) { + do_verify_chunk(chunk); + } + if (!chunk->is_tagged_free()) { + num_in_use_chunks ++; + } + const size_t s = chunk->word_size(); + // Prevent endless loop on invalid chunk size. + assert(is_valid_chunksize(is_class(), s), "Invalid chunk size: " SIZE_FORMAT ".", s); + MetaWord* next = ((MetaWord*)chunk) + s; chunk = (Metachunk*) next; } - assert(_container_count == num_in_use_chunks, "Container count mismatch (real: " UINTX_FORMAT - ", counter: " UINTX_FORMAT ".", num_in_use_chunks, _container_count); + assert(_chunk_count == num_in_use_chunks, "Container count mismatch (real: " UINTX_FORMAT + ", counter: " UINTX_FORMAT ".", num_in_use_chunks, _chunk_count); // Also verify the occupancy map. - occupancy_map()->verify(this->bottom(), this->top()); + if (slow) { + occupancy_map()->verify(this->bottom(), this->top()); + } } -#endif // ASSERT -#ifdef ASSERT // Verify that all free chunks in this node are ideally merged // (there not should be multiple small chunks where a large chunk could exist.) void VirtualSpaceNode::verify_free_chunks_are_ideally_merged() { @@ -224,23 +216,31 @@ const size_t size_small = (is_class() ? ClassSmallChunk : SmallChunk) * BytesPerWord; int num_free_chunks_since_last_med_boundary = -1; int num_free_chunks_since_last_small_boundary = -1; - while (chunk < invalid_chunk ) { + bool error = false; + char err[256]; + while (!error && chunk < invalid_chunk ) { // Test for missed chunk merge opportunities: count number of free chunks since last chunk boundary. // Reset the counter when encountering a non-free chunk. if (chunk->get_chunk_type() != HumongousIndex) { if (chunk->is_tagged_free()) { // Count successive free, non-humongous chunks. if (is_aligned(chunk, size_small)) { - assert(num_free_chunks_since_last_small_boundary <= 1, - "Missed chunk merge opportunity at " PTR_FORMAT " for chunk size " SIZE_FORMAT_HEX ".", p2i(chunk) - size_small, size_small); - num_free_chunks_since_last_small_boundary = 0; + if (num_free_chunks_since_last_small_boundary > 0) { + error = true; + jio_snprintf(err, sizeof(err), "Missed chunk merge opportunity to merge a small chunk preceding " PTR_FORMAT ".", p2i(chunk)); + } else { + num_free_chunks_since_last_small_boundary = 0; + } } else if (num_free_chunks_since_last_small_boundary != -1) { num_free_chunks_since_last_small_boundary ++; } if (is_aligned(chunk, size_med)) { - assert(num_free_chunks_since_last_med_boundary <= 1, - "Missed chunk merge opportunity at " PTR_FORMAT " for chunk size " SIZE_FORMAT_HEX ".", p2i(chunk) - size_med, size_med); - num_free_chunks_since_last_med_boundary = 0; + if (num_free_chunks_since_last_med_boundary > 0) { + error = true; + jio_snprintf(err, sizeof(err), "Missed chunk merge opportunity to merge a medium chunk preceding " PTR_FORMAT ".", p2i(chunk)); + } else { + num_free_chunks_since_last_med_boundary = 0; + } } else if (num_free_chunks_since_last_med_boundary != -1) { num_free_chunks_since_last_med_boundary ++; } @@ -255,30 +255,27 @@ num_free_chunks_since_last_small_boundary = -1; } + if (error) { + this->print_map(tty, is_class()); + assert(false, "%s", err); + } + MetaWord* next = ((MetaWord*)chunk) + chunk->word_size(); chunk = (Metachunk*) next; } } #endif // ASSERT -void VirtualSpaceNode::inc_container_count() { +void VirtualSpaceNode::inc_chunk_count() { assert_lock_strong(MetaspaceExpand_lock); - _container_count++; + _chunk_count++; } -void VirtualSpaceNode::dec_container_count() { +void VirtualSpaceNode::dec_chunk_count() { assert_lock_strong(MetaspaceExpand_lock); - _container_count--; + _chunk_count--; } -#ifdef ASSERT -void VirtualSpaceNode::verify_container_count() { - assert(_container_count == container_count_slow(), - "Inconsistency in container_count _container_count " UINTX_FORMAT - " container_count_slow() " UINTX_FORMAT, _container_count, container_count_slow()); -} -#endif - VirtualSpaceNode::~VirtualSpaceNode() { _rs.release(); if (_occupancy_map != NULL) { @@ -351,7 +348,7 @@ do_update_in_use_info_for_chunk(padding_chunk, true); // Return Chunk to freelist. - inc_container_count(); + inc_chunk_count(); chunk_manager->return_single_chunk(padding_chunk); // Please note: at this point, ChunkManager::return_single_chunk() // may already have merged the padding chunk with neighboring chunks, so @@ -448,14 +445,15 @@ occupancy_map()->set_chunk_starts_at_address((MetaWord*)result, true); do_update_in_use_info_for_chunk(result, true); - inc_container_count(); + inc_chunk_count(); - if (VerifyMetaspace) { - DEBUG_ONLY(chunk_manager->locked_verify()); - DEBUG_ONLY(this->verify()); - } - - DEBUG_ONLY(do_verify_chunk(result)); +#ifdef ASSERT + EVERY_NTH(VerifyMetaspaceInterval) + chunk_manager->locked_verify(true); + this->verify(true); + END_EVERY_NTH + do_verify_chunk(result); +#endif result->inc_use_count(); @@ -558,8 +556,13 @@ #endif // ASSERT void VirtualSpaceNode::retire(ChunkManager* chunk_manager) { - DEBUG_ONLY(verify_container_count();) assert(this->is_class() == chunk_manager->is_class(), "Wrong ChunkManager?"); +#ifdef ASSERT + verify(false); + EVERY_NTH(VerifyMetaspaceInterval) + verify(true); + END_EVERY_NTH +#endif for (int i = (int)MediumIndex; i >= (int)ZeroIndex; --i) { ChunkIndex index = (ChunkIndex)i; size_t chunk_size = chunk_manager->size_by_index(index); @@ -577,7 +580,6 @@ } chunk_manager->return_single_chunk(chunk); } - DEBUG_ONLY(verify_container_count();) } assert(free_words_in_vs() == 0, "should be empty now"); } diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/memory/metaspace/virtualSpaceNode.hpp --- a/src/hotspot/share/memory/metaspace/virtualSpaceNode.hpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/memory/metaspace/virtualSpaceNode.hpp Sun Feb 10 09:10:42 2019 +0100 @@ -53,7 +53,7 @@ VirtualSpace _virtual_space; MetaWord* _top; // count of chunks contained in this VirtualSpace - uintx _container_count; + uintx _chunk_count; OccupancyMap* _occupancy_map; @@ -79,7 +79,7 @@ VirtualSpaceNode(bool is_class, size_t byte_size); VirtualSpaceNode(bool is_class, ReservedSpace rs) : - _next(NULL), _is_class(is_class), _rs(rs), _top(NULL), _container_count(0), _occupancy_map(NULL) {} + _next(NULL), _is_class(is_class), _rs(rs), _top(NULL), _chunk_count(0), _occupancy_map(NULL) {} ~VirtualSpaceNode(); // Convenience functions for logical bottom and end @@ -112,13 +112,9 @@ MetaWord* top() const { return _top; } void inc_top(size_t word_size) { _top += word_size; } - uintx container_count() { return _container_count; } - void inc_container_count(); - void dec_container_count(); -#ifdef ASSERT - uintx container_count_slow(); - void verify_container_count(); -#endif + uintx chunk_count() { return _chunk_count; } + void inc_chunk_count(); + void dec_chunk_count(); // used and capacity in this single entry in the list size_t used_words_in_vs() const; @@ -152,10 +148,10 @@ // Debug support DEBUG_ONLY(void mangle();) - // Verify counters, all chunks in this list node and the occupancy map. - DEBUG_ONLY(void verify();) + // Verify counters and basic structure. Slow mode: verify all chunks in depth and occupancy map. + DEBUG_ONLY(void verify(bool slow);) // Verify that all free chunks in this node are ideally merged - // (there not should be multiple small chunks where a large chunk could exist.) + // (there should not be multiple small chunks where a large chunk could exist.) DEBUG_ONLY(void verify_free_chunks_are_ideally_merged();) }; diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/runtime/globals.hpp --- a/src/hotspot/share/runtime/globals.hpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/runtime/globals.hpp Sun Feb 10 09:10:42 2019 +0100 @@ -2528,8 +2528,9 @@ "File of size Xmx is pre-allocated for performance reason, so" \ "we need that much space available") \ \ - develop(bool, VerifyMetaspace, false, \ - "Verify metaspace on chunk movements.") \ + develop(int, VerifyMetaspaceInterval, DEBUG_ONLY(10000) NOT_DEBUG(0), \ + "Run periodic metaspace verifications (0 - none, " \ + "1 - always, >1 every nth interval)") \ \ diagnostic(bool, ShowRegistersOnAssert, true, \ "On internal errors, include registers in error report.") \ diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/utilities/debug.cpp --- a/src/hotspot/share/utilities/debug.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/utilities/debug.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -236,6 +236,17 @@ void report_vm_error(const char* file, int line, const char* error_msg, const char* detail_fmt, ...) { if (Debugging || error_is_suppressed(file, line)) return; + + if (ExecutingUnitTests) { + // gtest death test (see TEST_VM_ASSERT_MSG macro) expect an assertion message + // on stderr instead of stdout. They are also brittle and unable to parse our + // standard assertion message, so lets just print message and detail + // to stderr in one line. + // Note: for our simple tests we do not need to resolve the detail message, + // that saves us from having to va_copy the arguments. + fprintf(stderr, "assert failed: %s %s\n", error_msg, detail_fmt); + } + va_list detail_args; va_start(detail_args, detail_fmt); void* context = NULL; @@ -293,21 +304,6 @@ report_vm_error(file, line, "Unimplemented()"); } -#ifdef ASSERT -bool is_executing_unit_tests() { - return ExecutingUnitTests; -} - -void report_assert_msg(const char* msg, ...) { - va_list ap; - va_start(ap, msg); - - fprintf(stderr, "assert failed: %s\n", err_msg(FormatBufferDummy(), msg, ap).buffer()); - - va_end(ap); -} -#endif // ASSERT - void report_untested(const char* file, int line, const char* message) { #ifndef PRODUCT warning("Untested: %s in %s: %d\n", message, file, line); diff -r d667de4734ac -r fa534b143c5f src/hotspot/share/utilities/debug.hpp --- a/src/hotspot/share/utilities/debug.hpp Tue Feb 12 11:23:43 2019 -0800 +++ b/src/hotspot/share/utilities/debug.hpp Sun Feb 10 09:10:42 2019 +0100 @@ -54,9 +54,6 @@ do { \ if (!(p)) { \ TOUCH_ASSERT_POISON; \ - if (is_executing_unit_tests()) { \ - report_assert_msg(__VA_ARGS__); \ - } \ report_vm_error(__FILE__, __LINE__, "assert(" #p ") failed", __VA_ARGS__); \ BREAKPOINT; \ } \ diff -r d667de4734ac -r fa534b143c5f test/hotspot/gtest/memory/test_virtualSpaceNode.cpp --- a/test/hotspot/gtest/memory/test_virtualSpaceNode.cpp Tue Feb 12 11:23:43 2019 -0800 +++ b/test/hotspot/gtest/memory/test_virtualSpaceNode.cpp Sun Feb 10 09:10:42 2019 +0100 @@ -59,14 +59,9 @@ }; } -class ChunkManagerTest { +// Note: class exists to be friend to ChunkManager. +class ChunkManagerTestAccessor { public: - static size_t sum_free_chunks(ChunkManager* cm) { - return cm->sum_free_chunks(); - } - static size_t sum_free_chunks_count(ChunkManager* cm) { - return cm->sum_free_chunks_count(); - } static ChunkList* free_chunks(ChunkManager* cm, ChunkIndex i) { return cm->free_chunks(i); } @@ -79,9 +74,9 @@ int _count_pre_existing; public: ChunkManagerRestorer(metaspace::ChunkManager* cm) : _cm(cm), _count_pre_existing(0) { - _cm->locked_verify(); + DEBUG_ONLY(_cm->locked_verify(true);) for (metaspace::ChunkIndex i = metaspace::ZeroIndex; i < metaspace::NumberOfFreeLists; i = next_chunk_index(i)) { - metaspace::ChunkList* l = ChunkManagerTest::free_chunks(_cm, i); + metaspace::ChunkList* l = ChunkManagerTestAccessor::free_chunks(_cm, i); _count_pre_existing += l->count(); std::vector *v = new std::vector(l->count()); metaspace::Metachunk* c = l->head(); @@ -93,9 +88,9 @@ } } ~ChunkManagerRestorer() { - _cm->locked_verify(); + DEBUG_ONLY(_cm->locked_verify(true);) for (metaspace::ChunkIndex i = metaspace::ZeroIndex; i < metaspace::NumberOfFreeLists; i = next_chunk_index(i)) { - metaspace::ChunkList* l = ChunkManagerTest::free_chunks(_cm, i); + metaspace::ChunkList* l = ChunkManagerTestAccessor::free_chunks(_cm, i); std::vector *v = _free_chunks[i]; ssize_t count = l->count(); for (ssize_t j = 0; j < count; j++) { @@ -118,11 +113,11 @@ } int count_after_cleanup = 0; for (ChunkIndex i = ZeroIndex; i < NumberOfFreeLists; i = next_chunk_index(i)) { - ChunkList* l = ChunkManagerTest::free_chunks(_cm, i); + ChunkList* l = ChunkManagerTestAccessor::free_chunks(_cm, i); count_after_cleanup += l->count(); } EXPECT_EQ(_count_pre_existing, count_after_cleanup); - _cm->locked_verify(); + DEBUG_ONLY(_cm->locked_verify(true);) } }; @@ -159,9 +154,6 @@ chunk_up(words_left, num_medium_chunks, num_small_chunks, num_spec_chunks); EXPECT_EQ(0UL, num_medium_chunks) << "should not get any medium chunks"; - // DISABLED: checks started to fail after 8198423 - // EXPECT_EQ((num_small_chunks + num_spec_chunks), ChunkManagerTest::sum_free_chunks_count(&cm)) << "should be space for 3 chunks"; - // EXPECT_EQ(words_left, ChunkManagerTest::sum_free_chunks(&cm)) << "sizes should add up"; } TEST_VM(VirtualSpaceNodeTest, half_vsn_is_committed_humongous_chunk_is_used) { @@ -181,9 +173,6 @@ ASSERT_NO_FATAL_FAILURE(chunk_up(words_left, num_medium_chunks, num_small_chunks, num_spec_chunks)); EXPECT_EQ(0UL, num_medium_chunks) << "should not get any medium chunks"; - // DISABLED: checks started to fail after 8198423 - // EXPECT_EQ((num_small_chunks + num_spec_chunks), ChunkManagerTest::sum_free_chunks_count(&cm)) << "should be space for 3 chunks"; - // EXPECT_EQ(words_left, ChunkManagerTest::sum_free_chunks(&cm)) << "sizes should add up"; } TEST_VM(VirtualSpaceNodeTest, all_vsn_is_committed_half_is_used_by_chunks) { @@ -198,9 +187,6 @@ vsn.get_chunk_vs(MediumChunk); vsn.retire(&cm); - // DISABLED: checks started to fail after 8198423 - // EXPECT_EQ(2UL, ChunkManagerTest::sum_free_chunks_count(&cm)) << "should have been memory left for 2 chunks"; - // EXPECT_EQ(2UL * MediumChunk, ChunkManagerTest::sum_free_chunks(&cm)) << "sizes should add up"; } TEST_VM(VirtualSpaceNodeTest, no_committed_memory) { @@ -212,7 +198,7 @@ vsn.initialize(); vsn.retire(&cm); - ASSERT_EQ(0UL, ChunkManagerTest::sum_free_chunks_count(&cm)) << "did not commit any memory in the VSN"; + ASSERT_EQ(0UL, cm.free_chunks_count()) << "did not commit any memory in the VSN"; } TEST_VM(VirtualSpaceNodeTest, is_available_positive) { diff -r d667de4734ac -r fa534b143c5f test/hotspot/gtest/unittest.hpp --- a/test/hotspot/gtest/unittest.hpp Tue Feb 12 11:23:43 2019 -0800 +++ b/test/hotspot/gtest/unittest.hpp Sun Feb 10 09:10:42 2019 +0100 @@ -105,7 +105,7 @@ TEST(category, CONCAT(name, _vm_assert)) { \ ASSERT_EXIT(child_ ## category ## _ ## name ## _(), \ ::testing::ExitedWithCode(1), \ - "assert failed: " msg); \ + msg); \ } \ \ void test_ ## category ## _ ## name ## _()