-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Speed up COALESCE significantly #120139
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
Speed up COALESCE significantly #120139
Changes from all commits
edb6c03
8bd0989
759d052
f28f6af
8dfed18
40794d0
c7e1648
2615926
fbac0fe
8d81a3e
c93198d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,7 @@ | |
import org.elasticsearch.xpack.esql.expression.function.scalar.date.DateTrunc; | ||
import org.elasticsearch.xpack.esql.expression.function.scalar.math.Abs; | ||
import org.elasticsearch.xpack.esql.expression.function.scalar.multivalue.MvMin; | ||
import org.elasticsearch.xpack.esql.expression.function.scalar.nulls.Coalesce; | ||
import org.elasticsearch.xpack.esql.expression.function.scalar.string.RLike; | ||
import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Add; | ||
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Equals; | ||
|
@@ -96,6 +97,9 @@ public class EvalBenchmark { | |
"add_double", | ||
"case_1_eager", | ||
"case_1_lazy", | ||
"coalesce_2_noop", | ||
"coalesce_2_eager", | ||
"coalesce_2_lazy", | ||
"date_trunc", | ||
"equal_to_const", | ||
"long_equal_to_long", | ||
|
@@ -142,8 +146,34 @@ private static EvalOperator.ExpressionEvaluator evaluator(String operation) { | |
lhs = new Add(Source.EMPTY, lhs, new Literal(Source.EMPTY, 1L, DataType.LONG)); | ||
rhs = new Add(Source.EMPTY, rhs, new Literal(Source.EMPTY, 1L, DataType.LONG)); | ||
} | ||
yield EvalMapper.toEvaluator(FOLD_CONTEXT, new Case(Source.EMPTY, condition, List.of(lhs, rhs)), layout(f1, f2)) | ||
.get(driverContext); | ||
EvalOperator.ExpressionEvaluator evaluator = EvalMapper.toEvaluator( | ||
FOLD_CONTEXT, | ||
new Case(Source.EMPTY, condition, List.of(lhs, rhs)), | ||
layout(f1, f2) | ||
).get(driverContext); | ||
String desc = operation.endsWith("lazy") ? "CaseLazyEvaluator" : "CaseEagerEvaluator"; | ||
if (evaluator.toString().contains(desc) == false) { | ||
throw new IllegalArgumentException("Evaluator was [" + evaluator + "] but expected one containing [" + desc + "]"); | ||
} | ||
yield evaluator; | ||
} | ||
case "coalesce_2_noop", "coalesce_2_eager", "coalesce_2_lazy" -> { | ||
FieldAttribute f1 = longField(); | ||
FieldAttribute f2 = longField(); | ||
Expression lhs = f1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using ternary expression here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that makes this one harder to read. The |
||
if (operation.endsWith("lazy")) { | ||
lhs = new Add(Source.EMPTY, lhs, new Literal(Source.EMPTY, 1L, DataType.LONG)); | ||
} | ||
EvalOperator.ExpressionEvaluator evaluator = EvalMapper.toEvaluator( | ||
FOLD_CONTEXT, | ||
new Coalesce(Source.EMPTY, lhs, List.of(f2)), | ||
layout(f1, f2) | ||
).get(driverContext); | ||
String desc = operation.endsWith("lazy") ? "CoalesceLazyEvaluator" : "CoalesceEagerEvaluator"; | ||
if (evaluator.toString().contains(desc) == false) { | ||
throw new IllegalArgumentException("Evaluator was [" + evaluator + "] but expected one containing [" + desc + "]"); | ||
} | ||
yield evaluator; | ||
} | ||
case "date_trunc" -> { | ||
FieldAttribute timestamp = new FieldAttribute( | ||
|
@@ -260,6 +290,38 @@ private static void checkExpected(String operation, Page actual) { | |
} | ||
} | ||
} | ||
case "coalesce_2_noop" -> { | ||
LongVector f1 = actual.<LongBlock>getBlock(0).asVector(); | ||
LongVector result = actual.<LongBlock>getBlock(2).asVector(); | ||
for (int i = 0; i < BLOCK_LENGTH; i++) { | ||
long expected = f1.getLong(i); | ||
if (result.getLong(i) != expected) { | ||
throw new AssertionError("[" + operation + "] expected [" + expected + "] but was [" + result.getLong(i) + "]"); | ||
} | ||
} | ||
} | ||
case "coalesce_2_eager" -> { | ||
LongBlock f1 = actual.<LongBlock>getBlock(0); | ||
LongVector f2 = actual.<LongBlock>getBlock(1).asVector(); | ||
LongVector result = actual.<LongBlock>getBlock(2).asVector(); | ||
for (int i = 0; i < BLOCK_LENGTH; i++) { | ||
long expected = i % 5 == 0 ? f2.getLong(i) : f1.getLong(f1.getFirstValueIndex(i)); | ||
if (result.getLong(i) != expected) { | ||
throw new AssertionError("[" + operation + "] expected [" + expected + "] but was [" + result.getLong(i) + "]"); | ||
} | ||
} | ||
} | ||
case "coalesce_2_lazy" -> { | ||
LongBlock f1 = actual.<LongBlock>getBlock(0); | ||
LongVector f2 = actual.<LongBlock>getBlock(1).asVector(); | ||
LongVector result = actual.<LongBlock>getBlock(2).asVector(); | ||
for (int i = 0; i < BLOCK_LENGTH; i++) { | ||
long expected = i % 5 == 0 ? f2.getLong(i) : f1.getLong(f1.getFirstValueIndex(i)) + 1; | ||
if (result.getLong(i) != expected) { | ||
throw new AssertionError("[" + operation + "] expected [" + expected + "] but was [" + result.getLong(i) + "]"); | ||
} | ||
} | ||
} | ||
case "date_trunc" -> { | ||
LongVector v = actual.<LongBlock>getBlock(1).asVector(); | ||
long oneDay = TimeValue.timeValueHours(24).millis(); | ||
|
@@ -304,7 +366,7 @@ private static void checkExpected(String operation, Page actual) { | |
} | ||
} | ||
} | ||
default -> throw new UnsupportedOperationException(); | ||
default -> throw new UnsupportedOperationException(operation); | ||
} | ||
} | ||
|
||
|
@@ -324,7 +386,7 @@ private static Page page(String operation) { | |
} | ||
yield new Page(builder.build()); | ||
} | ||
case "case_1_eager", "case_1_lazy" -> { | ||
case "case_1_eager", "case_1_lazy", "coalesce_2_noop" -> { | ||
var f1 = blockFactory.newLongBlockBuilder(BLOCK_LENGTH); | ||
var f2 = blockFactory.newLongBlockBuilder(BLOCK_LENGTH); | ||
for (int i = 0; i < BLOCK_LENGTH; i++) { | ||
|
@@ -333,6 +395,19 @@ private static Page page(String operation) { | |
} | ||
yield new Page(f1.build(), f2.build()); | ||
} | ||
case "coalesce_2_eager", "coalesce_2_lazy" -> { | ||
var f1 = blockFactory.newLongBlockBuilder(BLOCK_LENGTH); | ||
var f2 = blockFactory.newLongBlockBuilder(BLOCK_LENGTH); | ||
for (int i = 0; i < BLOCK_LENGTH; i++) { | ||
if (i % 5 == 0) { | ||
f1.appendNull(); | ||
} else { | ||
f1.appendLong(i); | ||
} | ||
f2.appendLong(-i); | ||
} | ||
yield new Page(f1.build(), f2.build()); | ||
} | ||
case "long_equal_to_long" -> { | ||
var lhs = blockFactory.newLongBlockBuilder(BLOCK_LENGTH); | ||
var rhs = blockFactory.newLongBlockBuilder(BLOCK_LENGTH); | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using
Strings.format
for these.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find that harder to read these days.