Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8294677: chunklevel::MAX_CHUNK_WORD_SIZE too small for some applications 
Reviewed-by: simonis, phh
  • Loading branch information
@tstuefe
tstuefe committed Jan 24, 2023
1 parent 6dd8723 commit 2292ce1
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 65 deletions.
54 changes: 25 additions & 29 deletions src/hotspot/share/memory/metaspace/chunklevel.hpp
View file
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);

/////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/memory/metaspace/metaspaceSettings.hpp
View file
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/hotspot/gtest/metaspace/test_chunkManager_stress.cpp
View file
Expand Up @@ -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() {

Expand Down
53 changes: 40 additions & 13 deletions test/hotspot/gtest/metaspace/test_metachunk.cpp
View file
Expand Up @@ -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);
}
}

}
}
}
Expand Down
24 changes: 15 additions & 9 deletions test/hotspot/gtest/metaspace/test_metaspace_misc.cpp
View file
Expand Up @@ -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());

Expand All @@ -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) {
Expand Down
10 changes: 5 additions & 5 deletions test/hotspot/gtest/metaspace/test_virtualspacenode.cpp
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down
6 changes: 3 additions & 3 deletions test/hotspot/jtreg/runtime/CompressedOops/CompressedClassSpaceSize.java
View file
Expand Up @@ -65,14 +65,14 @@ public static void main(String[] args) throws Exception {


// 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);


Expand Down
2 changes: 1 addition & 1 deletion test/hotspot/jtreg/runtime/Metaspace/elastic/Settings.java
View file
Expand Up @@ -34,7 +34,7 @@ final public boolean doesReclaim() {
return reclaimPolicy.equals("balanced") || reclaimPolicy.equals("aggessive");
}

final static long rootChunkWordSize = 512 * 1024;
final static long rootChunkWordSize = 2048 * 1024;

static Settings theSettings;

Expand Down
2 changes: 1 addition & 1 deletion test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT1.java
View file
Expand Up @@ -175,7 +175,7 @@ public static void main(String[] args) throws Exception {

// 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);
Expand Down
3 changes: 1 addition & 2 deletions test/hotspot/jtreg/runtime/Metaspace/elastic/TestMetaspaceAllocationMT2.java
View file
Expand Up @@ -174,8 +174,7 @@ public static void main(String[] args) throws Exception {
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);
Expand Down

5 comments on commit 2292ce1

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

@tstuefe
Copy link
Member Author

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 2292ce1 Jan 24, 2023

Choose a reason for hiding this comment

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

@tstuefe Could not automatically backport 2292ce13 to openjdk/jdk17u-dev due to conflicts in the following files:

  • src/hotspot/share/memory/metaspace/metaspaceSettings.hpp
  • test/hotspot/gtest/metaspace/test_metaspace_misc.cpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b tstuefe-backport-2292ce13

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk 2292ce137c16accf0622600d5a096403b8a8058d

# Backport the commit
$ git cherry-pick --no-commit 2292ce137c16accf0622600d5a096403b8a8058d
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 2292ce137c16accf0622600d5a096403b8a8058d'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport 2292ce137c16accf0622600d5a096403b8a8058d.

@tstuefe
Copy link
Member Author

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 2292ce1 Jan 30, 2023

Choose a reason for hiding this comment

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

@tstuefe Could not automatically backport 2292ce13 to openjdk/jdk17u-dev due to conflicts in the following files:

  • test/hotspot/gtest/metaspace/test_metaspace_misc.cpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b tstuefe-backport-2292ce13

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk 2292ce137c16accf0622600d5a096403b8a8058d

# Backport the commit
$ git cherry-pick --no-commit 2292ce137c16accf0622600d5a096403b8a8058d
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 2292ce137c16accf0622600d5a096403b8a8058d'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport 2292ce137c16accf0622600d5a096403b8a8058d.

Please sign in to comment.