Skip to content

Use different G1GC options for small heaps #59667

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

Merged
merged 23 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7d987ba
Use the parallel collector for small heaps
ebadyano Jul 15, 2020
ab6059d
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Jul 28, 2020
2655315
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Jul 29, 2020
2003331
Addressed some comments and fixed tests.
ebadyano Jul 29, 2020
3c05949
Fix precomit erorrs
ebadyano Jul 29, 2020
818226a
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Jul 30, 2020
f14d2cd
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Jul 31, 2020
4d033a4
Fix test
ebadyano Jul 31, 2020
9008ede
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Sep 22, 2020
d0e34fe
Changed the ergonomics heuristic to use G1GC with tuned options instead
ebadyano Sep 29, 2020
70b4de9
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Sep 29, 2020
a6f5302
fix precomit errors
ebadyano Sep 29, 2020
ffec016
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Sep 29, 2020
80b713b
Fix precomit errors
ebadyano Sep 29, 2020
411cfd4
Fix indentation
ebadyano Oct 1, 2020
ec2b891
Addressed Henning's and Ryan's comments.
ebadyano Oct 2, 2020
8d3c734
Merge branch 'master' of github.com:elastic/elasticsearch into master…
ebadyano Oct 2, 2020
ac9a568
Address Henning's comments
ebadyano Oct 5, 2020
f326f5b
address Henning's comments
ebadyano Oct 5, 2020
5ddb619
Fix tests
ebadyano Oct 5, 2020
d628ff9
Fix precommit tests
ebadyano Oct 5, 2020
d8a3840
Fix logic
ebadyano Oct 5, 2020
f2e999a
Add an extra test for when non G1GC policy is set
ebadyano Oct 6, 2020
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
2 changes: 0 additions & 2 deletions distribution/src/config/jvm.options
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@
# 8-13:-XX:-UseConcMarkSweepGC
# 8-13:-XX:-UseCMSInitiatingOccupancyOnly
14-:-XX:+UseG1GC
14-:-XX:G1ReservePercent=25
14-:-XX:InitiatingHeapOccupancyPercent=30

## JVM temporary directory
-Djava.io.tmpdir=${ES_TMPDIR}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,62 @@ private JvmErgonomics() {
*/
static List<String> choose(final List<String> userDefinedJvmOptions) throws InterruptedException, IOException {
final List<String> ergonomicChoices = new ArrayList<>();
final Map<String, Optional<String>> finalJvmOptions = finalJvmOptions(userDefinedJvmOptions);
final Map<String, JvmOption> finalJvmOptions = finalJvmOptions(userDefinedJvmOptions);
final long heapSize = extractHeapSize(finalJvmOptions);
final long maxDirectMemorySize = extractMaxDirectMemorySize(finalJvmOptions);
if (maxDirectMemorySize == 0) {
ergonomicChoices.add("-XX:MaxDirectMemorySize=" + heapSize / 2);
}

final boolean tuneG1GCForSmallHeap = tuneG1GCForSmallHeap(heapSize);
final boolean tuneG1GCHeapRegion = tuneG1GCHeapRegion(finalJvmOptions, tuneG1GCForSmallHeap);
final boolean tuneG1GCInitiatingHeapOccupancyPercent = tuneG1GCInitiatingHeapOccupancyPercent(finalJvmOptions);
final int tuneG1GCReservePercent = tuneG1GCReservePercent(finalJvmOptions, tuneG1GCForSmallHeap);

if (tuneG1GCHeapRegion) {
ergonomicChoices.add("-XX:G1HeapRegionSize=4m");
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned on another channel, I think we should make reserve pct 15 rather than the default 10 - to stay safely clear of real memory circuit breaker exceptions.

I also think we should add in the IHOP always if it is not defined, just to be safe. It should not matter for performance since G1 will adaptively adjust the value.

}
if (tuneG1GCInitiatingHeapOccupancyPercent) {
ergonomicChoices.add("-XX:InitiatingHeapOccupancyPercent=30");
}
if (tuneG1GCReservePercent != 0) {
ergonomicChoices.add("-XX:G1ReservePercent=" + tuneG1GCReservePercent);
}

return ergonomicChoices;
}

private static final Pattern OPTION = Pattern.compile(
"^\\s*\\S+\\s+(?<flag>\\S+)\\s+:?=\\s+(?<value>\\S+)?\\s+\\{[^}]+?\\}\\s+\\{[^}]+}"
"^\\s*\\S+\\s+(?<flag>\\S+)\\s+:?=\\s+(?<value>\\S+)?\\s+\\{[^}]+?\\}\\s+\\{(?<origin>[^}]+)}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Drive-by comment, feel free to ignore - should we pass Pattern.COMMENTS here so that we can break up the regex with whitespace, to make it easier to read?

);

static Map<String, Optional<String>> finalJvmOptions(final List<String> userDefinedJvmOptions) throws InterruptedException,
IOException {
private static class JvmOption {
private final String value;
private final String origin;

JvmOption(String value, String origin) {
this.value = value;
this.origin = origin;
}

public Optional<String> getValue() {
return Optional.ofNullable(value);
}

public String getMandatoryValue() {
return value;
}

public boolean isCommandLineOrigin() {
return "command line".equals(this.origin);
}
}

static Map<String, JvmOption> finalJvmOptions(final List<String> userDefinedJvmOptions) throws InterruptedException, IOException {
return flagsFinal(userDefinedJvmOptions).stream()
.map(OPTION::matcher)
.filter(Matcher::matches)
.collect(Collectors.toUnmodifiableMap(m -> m.group("flag"), m -> Optional.ofNullable(m.group("value"))));
.collect(Collectors.toUnmodifiableMap(m -> m.group("flag"), m -> new JvmOption(m.group("value"), m.group("origin"))));
}

private static List<String> flagsFinal(final List<String> userDefinedJvmOptions) throws InterruptedException, IOException {
Expand Down Expand Up @@ -116,12 +153,42 @@ private static List<String> readLinesFromInputStream(final InputStream is) throw
}

// package private for testing
static Long extractHeapSize(final Map<String, Optional<String>> finalJvmOptions) {
return Long.parseLong(finalJvmOptions.get("MaxHeapSize").get());
static Long extractHeapSize(final Map<String, JvmOption> finalJvmOptions) {
return Long.parseLong(finalJvmOptions.get("MaxHeapSize").getMandatoryValue());
}

static long extractMaxDirectMemorySize(final Map<String, JvmOption> finalJvmOptions) {
return Long.parseLong(finalJvmOptions.get("MaxDirectMemorySize").getMandatoryValue());
}

// Tune G1GC options for heaps < 8GB
static boolean tuneG1GCForSmallHeap(final long heapSize) {
return heapSize < 8L << 30;
}

static boolean tuneG1GCHeapRegion(final Map<String, JvmOption> finalJvmOptions, final boolean tuneG1GCForSmallHeap) {
JvmOption g1GCHeapRegion = finalJvmOptions.get("G1HeapRegionSize");
JvmOption g1GC = finalJvmOptions.get("UseG1GC");
return (tuneG1GCForSmallHeap && g1GC.getMandatoryValue().equals("true") && g1GCHeapRegion.isCommandLineOrigin() == false);
}

static int tuneG1GCReservePercent(final Map<String, JvmOption> finalJvmOptions, final boolean tuneG1GCForSmallHeap) {
JvmOption g1GC = finalJvmOptions.get("UseG1GC");
JvmOption g1GCReservePercent = finalJvmOptions.get("G1ReservePercent");
if (g1GC.getMandatoryValue().equals("true")) {
if (g1GCReservePercent.isCommandLineOrigin() == false && tuneG1GCForSmallHeap) {
return 15;
} else if (g1GCReservePercent.isCommandLineOrigin() == false && tuneG1GCForSmallHeap == false) {
return 25;
}
}
return 0;
}

static long extractMaxDirectMemorySize(final Map<String, Optional<String>> finalJvmOptions) {
return Long.parseLong(finalJvmOptions.get("MaxDirectMemorySize").get());
static boolean tuneG1GCInitiatingHeapOccupancyPercent(final Map<String, JvmOption> finalJvmOptions) {
JvmOption g1GC = finalJvmOptions.get("UseG1GC");
JvmOption g1GCInitiatingHeapOccupancyPercent = finalJvmOptions.get("InitiatingHeapOccupancyPercent");
return g1GCInitiatingHeapOccupancyPercent.isCommandLineOrigin() == false && g1GC.getMandatoryValue().equals("true");
}

private static final Pattern SYSTEM_PROPERTY = Pattern.compile("^-D(?<key>[\\w+].*?)=(?<value>.*)$");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -113,6 +114,45 @@ public void testExtractSystemProperties() {
assertEquals(expectedSystemProperties, parsedSystemProperties);
}

public void testG1GOptionsForSmallHeap() throws InterruptedException, IOException {
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseG1GC"));
assertThat(jvmErgonomics, hasItem("-XX:G1HeapRegionSize=4m"));
assertThat(jvmErgonomics, hasItem("-XX:InitiatingHeapOccupancyPercent=30"));
assertThat(jvmErgonomics, hasItem("-XX:G1ReservePercent=15"));
}

public void testG1GOptionsForSmallHeapWhenTuningSet() throws InterruptedException, IOException {
List<String> jvmErgonomics = JvmErgonomics.choose(
Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseG1GC", "-XX:G1HeapRegionSize=4m", "-XX:InitiatingHeapOccupancyPercent=45")
);
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1HeapRegionSize="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:InitiatingHeapOccupancyPercent="))));
assertThat(jvmErgonomics, hasItem("-XX:G1ReservePercent=15"));
}

public void testG1GOptionsForLargeHeap() throws InterruptedException, IOException {
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms8g", "-Xmx8g", "-XX:+UseG1GC"));
assertThat(jvmErgonomics, hasItem("-XX:InitiatingHeapOccupancyPercent=30"));
assertThat(jvmErgonomics, hasItem("-XX:G1ReservePercent=25"));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1HeapRegionSize="))));
}

public void testG1GOptionsForSmallHeapWhenOtherGCSet() throws InterruptedException, IOException {
List<String> jvmErgonomics = JvmErgonomics.choose(Arrays.asList("-Xms6g", "-Xmx6g", "-XX:+UseParallelGC"));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1HeapRegionSize="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:InitiatingHeapOccupancyPercent="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1ReservePercent="))));
}

public void testG1GOptionsForLargeHeapWhenTuningSet() throws InterruptedException, IOException {
List<String> jvmErgonomics = JvmErgonomics.choose(
Arrays.asList("-Xms8g", "-Xmx8g", "-XX:+UseG1GC", "-XX:InitiatingHeapOccupancyPercent=60", "-XX:G1ReservePercent=10")
);
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:InitiatingHeapOccupancyPercent="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1ReservePercent="))));
assertThat(jvmErgonomics, everyItem(not(startsWith("-XX:G1HeapRegionSize="))));
}

public void testExtractNoSystemProperties() {
Map<String, String> parsedSystemProperties = JvmErgonomics.extractSystemProperties(Arrays.asList("-Xms1024M", "-Xmx1024M"));
assertTrue(parsedSystemProperties.isEmpty());
Expand Down