From 2292ce137c16accf0622600d5a096403b8a8058d Mon Sep 17 00:00:00 2001 From: Thomas Stuefe Date: Tue, 24 Jan 2023 06:35:26 +0000 Subject: [PATCH] 8294677: chunklevel::MAX_CHUNK_WORD_SIZE too small for some applications Reviewed-by: simonis, phh --- .../share/memory/metaspace/chunklevel.hpp | 54 +++++++++---------- .../memory/metaspace/metaspaceSettings.hpp | 2 +- .../metaspace/test_chunkManager_stress.cpp | 2 +- .../gtest/metaspace/test_metachunk.cpp | 53 +++++++++++++----- .../gtest/metaspace/test_metaspace_misc.cpp | 24 +++++---- .../gtest/metaspace/test_virtualspacenode.cpp | 10 ++-- .../CompressedClassSpaceSize.java | 6 +-- .../runtime/Metaspace/elastic/Settings.java | 2 +- .../elastic/TestMetaspaceAllocationMT1.java | 2 +- .../elastic/TestMetaspaceAllocationMT2.java | 3 +- 10 files changed, 93 insertions(+), 65 deletions(-) diff --git a/src/hotspot/share/memory/metaspace/chunklevel.hpp b/src/hotspot/share/memory/metaspace/chunklevel.hpp index 8dbc2467fd7..559ff1fa46c 100644 --- a/src/hotspot/share/memory/metaspace/chunklevel.hpp +++ b/src/hotspot/share/memory/metaspace/chunklevel.hpp @@ -46,19 +46,13 @@ namespace metaspace { // From there on it goes: // // size level -// 4MB 0 -// 2MB 1 -// 1MB 2 -// 512K 3 -// 256K 4 -// 128K 5 -// 64K 6 -// 32K 7 -// 16K 8 -// 8K 9 -// 4K 10 -// 2K 11 -// 1K 12 +// 16MB 0 +// 8MB 1 +// 4MB 2 +// ... +// 4K 12 +// 2K 13 +// 1K 14 // Metachunk level (must be signed) typedef signed char chunklevel_t; @@ -67,8 +61,8 @@ typedef signed char chunklevel_t; namespace chunklevel { -static const size_t MAX_CHUNK_BYTE_SIZE = 4 * M; -static const int NUM_CHUNK_LEVELS = 13; +static const size_t MAX_CHUNK_BYTE_SIZE = 16 * M; +static const int NUM_CHUNK_LEVELS = 15; static const size_t MIN_CHUNK_BYTE_SIZE = (MAX_CHUNK_BYTE_SIZE >> ((size_t)NUM_CHUNK_LEVELS - 1)); static const size_t MIN_CHUNK_WORD_SIZE = MIN_CHUNK_BYTE_SIZE / sizeof(MetaWord); @@ -101,22 +95,24 @@ inline size_t word_size_for_level(chunklevel_t level) { chunklevel_t level_fitting_word_size(size_t word_size); // Shorthands to refer to exact sizes -static const chunklevel_t CHUNK_LEVEL_4M = ROOT_CHUNK_LEVEL; -static const chunklevel_t CHUNK_LEVEL_2M = (ROOT_CHUNK_LEVEL + 1); -static const chunklevel_t CHUNK_LEVEL_1M = (ROOT_CHUNK_LEVEL + 2); -static const chunklevel_t CHUNK_LEVEL_512K = (ROOT_CHUNK_LEVEL + 3); -static const chunklevel_t CHUNK_LEVEL_256K = (ROOT_CHUNK_LEVEL + 4); -static const chunklevel_t CHUNK_LEVEL_128K = (ROOT_CHUNK_LEVEL + 5); -static const chunklevel_t CHUNK_LEVEL_64K = (ROOT_CHUNK_LEVEL + 6); -static const chunklevel_t CHUNK_LEVEL_32K = (ROOT_CHUNK_LEVEL + 7); -static const chunklevel_t CHUNK_LEVEL_16K = (ROOT_CHUNK_LEVEL + 8); -static const chunklevel_t CHUNK_LEVEL_8K = (ROOT_CHUNK_LEVEL + 9); -static const chunklevel_t CHUNK_LEVEL_4K = (ROOT_CHUNK_LEVEL + 10); -static const chunklevel_t CHUNK_LEVEL_2K = (ROOT_CHUNK_LEVEL + 11); -static const chunklevel_t CHUNK_LEVEL_1K = (ROOT_CHUNK_LEVEL + 12); +static const chunklevel_t CHUNK_LEVEL_16M = ROOT_CHUNK_LEVEL; +static const chunklevel_t CHUNK_LEVEL_8M = (ROOT_CHUNK_LEVEL + 1); +static const chunklevel_t CHUNK_LEVEL_4M = (ROOT_CHUNK_LEVEL + 2); +static const chunklevel_t CHUNK_LEVEL_2M = (ROOT_CHUNK_LEVEL + 3); +static const chunklevel_t CHUNK_LEVEL_1M = (ROOT_CHUNK_LEVEL + 4); +static const chunklevel_t CHUNK_LEVEL_512K = (ROOT_CHUNK_LEVEL + 5); +static const chunklevel_t CHUNK_LEVEL_256K = (ROOT_CHUNK_LEVEL + 6); +static const chunklevel_t CHUNK_LEVEL_128K = (ROOT_CHUNK_LEVEL + 7); +static const chunklevel_t CHUNK_LEVEL_64K = (ROOT_CHUNK_LEVEL + 8); +static const chunklevel_t CHUNK_LEVEL_32K = (ROOT_CHUNK_LEVEL + 9); +static const chunklevel_t CHUNK_LEVEL_16K = (ROOT_CHUNK_LEVEL + 10); +static const chunklevel_t CHUNK_LEVEL_8K = (ROOT_CHUNK_LEVEL + 11); +static const chunklevel_t CHUNK_LEVEL_4K = (ROOT_CHUNK_LEVEL + 12); +static const chunklevel_t CHUNK_LEVEL_2K = (ROOT_CHUNK_LEVEL + 13); +static const chunklevel_t CHUNK_LEVEL_1K = (ROOT_CHUNK_LEVEL + 14); STATIC_ASSERT(CHUNK_LEVEL_1K == HIGHEST_CHUNK_LEVEL); -STATIC_ASSERT(CHUNK_LEVEL_4M == LOWEST_CHUNK_LEVEL); +STATIC_ASSERT(CHUNK_LEVEL_16M == LOWEST_CHUNK_LEVEL); STATIC_ASSERT(ROOT_CHUNK_LEVEL == LOWEST_CHUNK_LEVEL); ///////////////////////////////////////////////////////// diff --git a/src/hotspot/share/memory/metaspace/metaspaceSettings.hpp b/src/hotspot/share/memory/metaspace/metaspaceSettings.hpp index d2dc3f97441..598fe33ecfe 100644 --- a/src/hotspot/share/memory/metaspace/metaspaceSettings.hpp +++ b/src/hotspot/share/memory/metaspace/metaspaceSettings.hpp @@ -47,7 +47,7 @@ class Settings : public AllStatic { // Note that this only affects the non-class metaspace. Class space ignores this size (it is one // single large mapping). static const size_t _virtual_space_node_default_word_size = - chunklevel::MAX_CHUNK_WORD_SIZE * NOT_LP64(2) LP64_ONLY(16); // 8MB (32-bit) / 64MB (64-bit) + chunklevel::MAX_CHUNK_WORD_SIZE * NOT_LP64(1) LP64_ONLY(4); // 16MB (32-bit) / 64MB (64-bit) // Alignment of the base address of a virtual space node static const size_t _virtual_space_node_reserve_alignment_words = chunklevel::MAX_CHUNK_WORD_SIZE; diff --git a/test/hotspot/gtest/metaspace/test_chunkManager_stress.cpp b/test/hotspot/gtest/metaspace/test_chunkManager_stress.cpp index 1ba522c1169..d62ec6c055e 100644 --- a/test/hotspot/gtest/metaspace/test_chunkManager_stress.cpp +++ b/test/hotspot/gtest/metaspace/test_chunkManager_stress.cpp @@ -190,7 +190,7 @@ class ChunkManagerRandomChunkAllocTest { // adjust test if we change levels STATIC_ASSERT(HIGHEST_CHUNK_LEVEL == CHUNK_LEVEL_1K); - STATIC_ASSERT(LOWEST_CHUNK_LEVEL == CHUNK_LEVEL_4M); + STATIC_ASSERT(LOWEST_CHUNK_LEVEL == CHUNK_LEVEL_16M); void one_test() { diff --git a/test/hotspot/gtest/metaspace/test_metachunk.cpp b/test/hotspot/gtest/metaspace/test_metachunk.cpp index 7faffea96dc..aebf07ea9a5 100644 --- a/test/hotspot/gtest/metaspace/test_metachunk.cpp +++ b/test/hotspot/gtest/metaspace/test_metachunk.cpp @@ -62,24 +62,51 @@ TEST_VM(metaspace, get_chunk) { // Test ChunkManager::get_chunk, but with a commit limit. TEST_VM(metaspace, get_chunk_with_commit_limit) { - const size_t commit_limit_words = 1 * M; - ChunkGtestContext context(commit_limit_words); - Metachunk* c = NULL; + // A commit limit that is smaller than the largest possible chunk size. - for (chunklevel_t pref_lvl = LOWEST_CHUNK_LEVEL; pref_lvl <= HIGHEST_CHUNK_LEVEL; pref_lvl++) { + // Here we test different combinations of commit limit, preferred and highest chunk level, and min_committed_size. - for (chunklevel_t max_lvl = pref_lvl; max_lvl <= HIGHEST_CHUNK_LEVEL; max_lvl++) { + for (size_t commit_limit_words = Settings::commit_granule_words(); + commit_limit_words < MAX_CHUNK_WORD_SIZE * 2; commit_limit_words *= 2) { - for (size_t min_committed_words = Settings::commit_granule_words(); - min_committed_words <= word_size_for_level(max_lvl); min_committed_words *= 2) { + ChunkGtestContext context(commit_limit_words); + Metachunk* c = NULL; - if (min_committed_words <= commit_limit_words) { - context.alloc_chunk_expect_success(&c, pref_lvl, max_lvl, min_committed_words); - context.return_chunk(c); - } else { - context.alloc_chunk_expect_failure(pref_lvl, max_lvl, min_committed_words); + for (chunklevel_t pref_lvl = LOWEST_CHUNK_LEVEL; pref_lvl <= HIGHEST_CHUNK_LEVEL; pref_lvl++) { + + for (chunklevel_t max_lvl = pref_lvl; max_lvl <= HIGHEST_CHUNK_LEVEL; max_lvl++) { + + for (size_t min_committed_words = Settings::commit_granule_words(); + min_committed_words <= word_size_for_level(max_lvl); min_committed_words *= 2) { + + // When should commit work? As long as min_committed_words is smaller than commit_limit_words. + bool commit_should_work = min_committed_words <= commit_limit_words; + + // Exception: MetaspaceReclaimPolicy=none. Here, chunks are fully committed from the get go and + // min_committed_words is effectively ignored. So commit would fail if the chunk is larger than + // the commit limit. Unfortunately, the chunk size is difficult to predict (it will be between + // [pref_lvl, max_lvl]. To make matters simple, we skip the test if we don't know the level for + // sure. + if (Settings::new_chunks_are_fully_committed()) { + if (pref_lvl == max_lvl) { + commit_should_work = word_size_for_level(max_lvl) <= commit_limit_words; + } else { + continue; + } + } + + // printf("commit_limit: " SIZE_FORMAT ", min_committed_words: " SIZE_FORMAT + // ", max chunk level: " CHKLVL_FORMAT ", preferred chunk level: " CHKLVL_FORMAT ", should work: %d\n", + // commit_limit_words, min_committed_words, max_lvl, pref_lvl, commit_should_work); + // fflush(stdout); + + if (commit_should_work) { + context.alloc_chunk_expect_success(&c, pref_lvl, max_lvl, min_committed_words); + context.return_chunk(c); + } else { + context.alloc_chunk_expect_failure(pref_lvl, max_lvl, min_committed_words); + } } - } } } diff --git a/test/hotspot/gtest/metaspace/test_metaspace_misc.cpp b/test/hotspot/gtest/metaspace/test_metaspace_misc.cpp index 144a3d73b82..6a2282960e6 100644 --- a/test/hotspot/gtest/metaspace/test_metaspace_misc.cpp +++ b/test/hotspot/gtest/metaspace/test_metaspace_misc.cpp @@ -46,8 +46,7 @@ TEST_VM(metaspace, misc_sizes) { ASSERT_EQ(Settings::commit_granule_bytes(), Metaspace::commit_alignment()); ASSERT_TRUE(is_aligned(Settings::virtual_space_node_default_word_size(), metaspace::chunklevel::MAX_CHUNK_WORD_SIZE)); - ASSERT_EQ(Settings::virtual_space_node_default_word_size(), - metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * NOT_LP64(2) LP64_ONLY(16)); + ASSERT_EQ(Settings::virtual_space_node_default_word_size() * BytesPerWord, NOT_LP64(16) LP64_ONLY(64) * M); ASSERT_EQ(Settings::virtual_space_node_reserve_alignment_words(), Metaspace::reserve_alignment_words()); @@ -56,13 +55,20 @@ TEST_VM(metaspace, misc_sizes) { TEST_VM(metaspace, misc_max_alloc_size) { // Make sure we can allocate what we promise to allocate... - const size_t sz = Metaspace::max_allocation_word_size(); - ClassLoaderData* cld = ClassLoaderData::the_null_class_loader_data(); - MetaWord* p = cld->metaspace_non_null()->allocate(sz, Metaspace::NonClassType); - ASSERT_NOT_NULL(p); - // And also, successfully deallocate it. - cld->metaspace_non_null()->deallocate(p, sz, false); - + for (int i = 0; i < 2; i ++) { + const bool in_class_space = (i == 0); + const Metaspace::MetadataType mdType = in_class_space ? Metaspace::ClassType : Metaspace::NonClassType; + const size_t sz = Metaspace::max_allocation_word_size(); + ClassLoaderData* cld = ClassLoaderData::the_null_class_loader_data(); + MetaWord* p = cld->metaspace_non_null()->allocate(sz, mdType); + if (p == nullptr) { + // Have we run into the GC threshold? + p = cld->metaspace_non_null()->expand_and_allocate(sz, mdType); + ASSERT_NOT_NULL(p); + } + // And also, successfully deallocate it. + cld->metaspace_non_null()->deallocate(p, sz, in_class_space); + } } TEST_VM(metaspace, chunklevel_utils) { diff --git a/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp b/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp index 5166eede645..251dd5994e8 100644 --- a/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp +++ b/test/hotspot/gtest/metaspace/test_virtualspacenode.cpp @@ -496,7 +496,7 @@ TEST_VM(metaspace, virtual_space_node_test_basics) { MutexLocker fcl(Metaspace_lock, Mutex::_no_safepoint_check_flag); - const size_t word_size = metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 10; + const size_t word_size = metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 3; SizeCounter scomm; SizeCounter sres; @@ -586,13 +586,13 @@ TEST_VM(metaspace, virtual_space_node_test_5) { TEST_VM(metaspace, virtual_space_node_test_7) { // Test large allocation and freeing. { - VirtualSpaceNodeTest test(metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 100, - metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 100); + VirtualSpaceNodeTest test(metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 25, + metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 25); test.test_exhaust_node(); } { - VirtualSpaceNodeTest test(metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 100, - metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 100); + VirtualSpaceNodeTest test(metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 25, + metaspace::chunklevel::MAX_CHUNK_WORD_SIZE * 25); test.test_exhaust_node(); } diff --git a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassSpaceSize.java b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassSpaceSize.java index aaa6b1f03dd..5eaced668f0 100644 --- a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassSpaceSize.java +++ b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassSpaceSize.java @@ -65,14 +65,14 @@ public class CompressedClassSpaceSize { // Make sure the minimum size is set correctly and printed - // (Note: ccs size shall be rounded up to the minimum size of 4m since metaspace reservations - // are done in a 4m granularity. Note that this is **reserved** size and does not affect rss. + // (Note: ccs size are rounded up to the next larger root chunk boundary (16m). + // Note that this is **reserved** size and does not affect rss. pb = ProcessTools.createJavaProcessBuilder("-XX:+UnlockDiagnosticVMOptions", "-XX:CompressedClassSpaceSize=1m", "-Xlog:gc+metaspace=trace", "-version"); output = new OutputAnalyzer(pb.start()); - output.shouldMatch("Compressed class space.*4194304") + output.shouldMatch("Compressed class space.*16777216") .shouldHaveExitValue(0); diff --git a/test/hotspot/jtreg/runtime/Metaspace/elastic/Settings.java b/test/hotspot/jtreg/runtime/Metaspace/elastic/Settings.java index 04e2f538bca..cca5b7f59ed 100644 --- a/test/hotspot/jtreg/runtime/Metaspace/elastic/Settings.java +++ b/test/hotspot/jtreg/runtime/Metaspace/elastic/Settings.java @@ -34,7 +34,7 @@ public final class Settings { return reclaimPolicy.equals("balanced") || reclaimPolicy.equals("aggessive"); } - final static long rootChunkWordSize = 512 * 1024; + final static long rootChunkWordSize = 2048 * 1024; static Settings theSettings; diff --git a/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT1.java b/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT1.java index 02ec29b0cfa..aefd627572c 100644 --- a/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT1.java +++ b/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT1.java @@ -175,7 +175,7 @@ public class TestMetaspaceAllocationMT1 { // Note: reserve limit must be a multiple of Metaspace::reserve_alignment_words() // (512K on 64bit, 1M on 32bit) - long reserveLimit = (i == 2) ? 1024 * 1024 : 0; + long reserveLimit = (i == 2) ? Settings.rootChunkWordSize * 2 : 0; System.out.println("#### Test: "); System.out.println("#### testAllocationCeiling: " + testAllocationCeiling); diff --git a/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT2.java b/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT2.java index 94b7af7ec76..ff1ccc0df22 100644 --- a/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT2.java +++ b/test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT2.java @@ -174,8 +174,7 @@ public class TestMetaspaceAllocationMT2 { long commitLimit = (i == 1) ? 1024 * 256 : 0; // Note: reserve limit must be a multiple of Metaspace::reserve_alignment_words() - // (512K on 64bit, 1M on 32bit) - long reserveLimit = (i == 2) ? 1024 * 1024 : 0; + long reserveLimit = (i == 2) ? Settings.rootChunkWordSize * 2 : 0; System.out.println("#### Test: "); System.out.println("#### testAllocationCeiling: " + testAllocationCeiling);