Skip to content

8354145: G1: UseCompressedOops boundary is calculated on maximum heap region size instead of maxiumum ergonomic heap region size #24541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/hotspot/share/gc/g1/g1Arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ void G1Arguments::initialize_alignments() {
}

size_t G1Arguments::conservative_max_heap_alignment() {
if (FLAG_IS_DEFAULT(G1HeapRegionSize)) {
return G1HeapRegion::max_ergonomics_size();
}
return G1HeapRegion::max_region_size();
}

Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/gc/g1/g1HeapRegion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ size_t G1HeapRegion::max_region_size() {
return G1HeapRegionBounds::max_size();
}

size_t G1HeapRegion::max_ergonomics_size() {
return G1HeapRegionBounds::max_ergonomics_size();
}

size_t G1HeapRegion::min_region_size_in_words() {
return G1HeapRegionBounds::min_size() >> LogHeapWordSize;
}
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/gc/g1/g1HeapRegion.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ class G1HeapRegion : public CHeapObj<mtGC> {
}

static size_t max_region_size();
static size_t max_ergonomics_size();
static size_t min_region_size_in_words();

// It sets up the heap region size (GrainBytes / GrainWords), as well as
Expand Down
86 changes: 86 additions & 0 deletions test/hotspot/jtreg/gc/arguments/TestG1CompressedOops.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (C) 2025 THL A29 Limited, a Tencent company. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package gc.arguments;

/*
* @test TestG1CompressedOops
* @bug 8354145
* @requires vm.flagless
* @summary Verify that the flag UseCompressedOops is updated properly
* @library /test/lib
* @library /
Copy link
Member

Choose a reason for hiding this comment

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

Why this line is needed? I don't see any dependencies on "/"
If you use some test code outside directory, better to build them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why this line is needed? I don't see any dependencies on "/" If you use some test code outside directory, better to build them.

Yes, the GCArguments depends on the @library / , many tests in test/hotspot/jtreg/gc/arguments use this

Copy link
Contributor

Choose a reason for hiding this comment

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

Afaict GCArguments only depends on test.lib.* too.
Other than that, the use of the @librarydirectives is often just copy&paste without particular meaning.

* @run driver gc.arguments.TestG1CompressedOops
*/

import java.util.regex.Matcher;
import java.util.regex.Pattern;

import java.util.ArrayList;
import java.util.Arrays;

import jdk.test.lib.process.OutputAnalyzer;

public class TestG1CompressedOops {

private static void checkG1CompressedOops(String[] flags, boolean expectedValue, int exitValue) throws Exception {
ArrayList<String> flagList = new ArrayList<String>();
flagList.addAll(Arrays.asList(flags));
flagList.add("-XX:+UseG1GC");
flagList.add("-XX:+PrintFlagsFinal");
flagList.add("-version");

OutputAnalyzer output = GCArguments.executeTestJava(flagList);
output.shouldHaveExitValue(exitValue);

if (exitValue == 0) {
String stdout = output.getStdout();
boolean flagValue = getFlagValue("UseCompressedOops", stdout);
if (flagValue != expectedValue) {
throw new RuntimeException("Wrong value for UseCompressedOops. Expected " + expectedValue + " but got " + flagValue);
}
}
}

private static boolean getFlagValue(String flag, String where) {
Matcher m = Pattern.compile(flag + "\\s+:?=\\s+\\D+").matcher(where);
if (!m.find()) {
throw new RuntimeException("Could not find value for flag " + flag + " in output string");
}
String match = m.group();
return match.contains("true");
}

public static void main(String args[]) throws Exception {
checkG1CompressedOops(new String[] { "-Xmx64m" /* default is 1m */ }, true, 0);
checkG1CompressedOops(new String[] { "-Xmx64m", "-XX:G1HeapRegionSize=2m" }, true, 0);
checkG1CompressedOops(new String[] { "-Xmx32768m" /* 32g will turn off the usecompressedoops */ }, false, 0);
checkG1CompressedOops(new String[] { "-Xmx32760m" }, false, 0);
checkG1CompressedOops(new String[] { "-Xmx32736m", /* 32g - 32m will turn on the usecomppressedoops */ }, true, 0);

// if set G1HeapRegionSize explicitly with -Xmx32736m will turn off the UseCompressedOops
checkG1CompressedOops(new String[] { "-Xmx32736m", "-XX:G1HeapRegionSize=1m" }, false, 0);
checkG1CompressedOops(new String[] { "-Xmx32256m", "-XX:G1HeapRegionSize=512m" }, true, 0);
}
}