Skip to content

Commit c09dc8e

Browse files
authored
Optimize parsing of compound format in MergePolicyConfig (#135643)
By checking if the setting value ends with a `b`, we can skip the ratio/double parsing. The default value is `1gb`, so this will skip the ratio parsing by default. I was doing some benchmarking on component templates and noticed the `MergePolicyConfig` constructor sticking out slightly in flamegraphs due to the failing double parsing.
1 parent 28aa123 commit c09dc8e

File tree

2 files changed

+31
-18
lines changed

2 files changed

+31
-18
lines changed

server/src/main/java/org/elasticsearch/index/MergePolicyConfig.java

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.apache.lucene.index.MergePolicy;
1515
import org.apache.lucene.index.NoMergePolicy;
1616
import org.apache.lucene.index.TieredMergePolicy;
17+
import org.elasticsearch.ElasticsearchParseException;
1718
import org.elasticsearch.common.settings.Setting;
1819
import org.elasticsearch.common.settings.Setting.Property;
1920
import org.elasticsearch.common.unit.ByteSizeUnit;
@@ -398,26 +399,32 @@ private static CompoundFileThreshold parseCompoundFormat(String noCFSRatio) {
398399
return new CompoundFileThreshold(1.0d);
399400
} else if (noCFSRatio.equalsIgnoreCase("false")) {
400401
return new CompoundFileThreshold(0.0d);
401-
} else {
402+
}
403+
NumberFormatException suppressedNfe = null;
404+
if (noCFSRatio.endsWith("b") == false) {
405+
// If the value ends with a `b`, it implies it is probably a byte size value, so do not try to parse as a ratio at all.
406+
// The main motivation is to make parsing faster. Using exception throwing and catching when trying to parse
407+
// as a ratio as a means of identifying that a string is not a ratio can be quite slow.
402408
try {
403-
try {
404-
return new CompoundFileThreshold(Double.parseDouble(noCFSRatio));
405-
} catch (NumberFormatException ex) {
406-
throw new IllegalArgumentException(
407-
"index.compound_format must be a boolean, a non-negative byte size or a ratio in the interval [0..1] but was: ["
408-
+ noCFSRatio
409-
+ "]",
410-
ex
411-
);
412-
}
413-
} catch (IllegalArgumentException e) {
414-
try {
415-
return new CompoundFileThreshold(ByteSizeValue.parseBytesSizeValue(noCFSRatio, INDEX_COMPOUND_FORMAT_SETTING_KEY));
416-
} catch (RuntimeException e2) {
417-
e.addSuppressed(e2);
418-
}
419-
throw e;
409+
return new CompoundFileThreshold(Double.parseDouble(noCFSRatio));
410+
} catch (NumberFormatException e) {
411+
// ignore for now, see if it parses as bytes
412+
suppressedNfe = e;
413+
}
414+
}
415+
try {
416+
return new CompoundFileThreshold(ByteSizeValue.parseBytesSizeValue(noCFSRatio, INDEX_COMPOUND_FORMAT_SETTING_KEY));
417+
} catch (ElasticsearchParseException ex) {
418+
final var illegalArgumentException = new IllegalArgumentException(
419+
"index.compound_format must be a boolean, a non-negative byte size or a ratio in the interval [0..1] but was: ["
420+
+ noCFSRatio
421+
+ "]",
422+
ex
423+
);
424+
if (suppressedNfe != null) {
425+
illegalArgumentException.addSuppressed(suppressedNfe);
420426
}
427+
throw illegalArgumentException;
421428
}
422429
}
423430

server/src/test/java/org/elasticsearch/index/MergePolicyConfigTests.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ public void testCompoundFileSettings() throws IOException {
5050
assertCompoundThreshold(build(0.0), 0.0, ByteSizeValue.ofBytes(Long.MAX_VALUE));
5151
assertCompoundThreshold(build(0.0), 0.0, ByteSizeValue.ofBytes(Long.MAX_VALUE));
5252
assertCompoundThreshold(build("100MB"), 1.0, ByteSizeValue.ofMb(100));
53+
assertCompoundThreshold(build(" 1gb"), 1.0, ByteSizeValue.ofGb(1));
54+
assertCompoundThreshold(build(" 1gb "), 1.0, ByteSizeValue.ofGb(1));
55+
assertCompoundThreshold(build("1k"), 1.0, ByteSizeValue.ofKb(1));
56+
assertCompoundThreshold(build("1t"), 1.0, ByteSizeValue.ofTb(1));
57+
assertCompoundThreshold(build(" 0"), 0, ByteSizeValue.ofBytes(Long.MAX_VALUE));
58+
assertCompoundThreshold(build(" 0 "), 0, ByteSizeValue.ofBytes(Long.MAX_VALUE));
5359
assertCompoundThreshold(build("0MB"), 1.0, ByteSizeValue.ofBytes(0));
5460
assertCompoundThreshold(build("0B"), 1.0, ByteSizeValue.ofBytes(0));
5561
}

0 commit comments

Comments
 (0)