This is an automated email from the ASF dual-hosted git repository.
abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 7552dc49fb7 Reduce amount of expression objects created during
evaluations (#15552)
7552dc49fb7 is described below
commit 7552dc49fb7937b4de2ef26e9857b8d26003e4cc
Author: Zoltan Haindrich <[email protected]>
AuthorDate: Fri Dec 15 11:41:59 2023 +0100
Reduce amount of expression objects created during evaluations (#15552)
I was looking into a query which was performing a bit poorly because the
case_searched was touching more than 1 columns (if there is only 1 column there
is a cache based evaluator).
While I was doing that I've noticed that there are a few simple things
which could help a bit:
use a static TRUE/FALSE instead of creating a new object every time
create the ExprEval early for ConstantExpr -s (except the one for
BigInteger which seem to have some odd contract)
return early from type autodetection
these changes mostly reduce the amount of garbage the query creates during
case_searched evaluation; although ExpressionSelectorBenchmark shows some
improvements ~15% - but my manual trials on the taxi dataset with 60M rows
showed more improvements - probably due to the fact that these changes mostly
only reduce gc pressure.
---
.../druid/benchmark/ExpressionFilterBenchmark.java | 2 +
.../benchmark/ExpressionSelectorBenchmark.java | 156 ++++++++++++++++++++-
.../org/apache/druid/math/expr/ConstantExpr.java | 20 ++-
.../java/org/apache/druid/math/expr/ExprEval.java | 6 +-
.../druid/math/expr/ExpressionTypeConversion.java | 3 +
.../java/org/apache/druid/math/expr/ExprTest.java | 7 +-
.../org/apache/druid/math/expr/OutputTypeTest.java | 11 ++
7 files changed, 193 insertions(+), 12 deletions(-)
diff --git
a/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java
b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java
index eea1804292a..e7d8ad00bc5 100644
---
a/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java
+++
b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java
@@ -25,6 +25,7 @@ import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.java.util.common.guava.Sequence;
import org.apache.druid.java.util.common.io.Closer;
+import org.apache.druid.math.expr.ExpressionProcessing;
import org.apache.druid.query.expression.TestExprMacroTable;
import org.apache.druid.query.filter.AndDimFilter;
import org.apache.druid.query.filter.DimFilter;
@@ -70,6 +71,7 @@ public class ExpressionFilterBenchmark
{
static {
NullHandling.initializeForTests();
+ ExpressionProcessing.initializeForTests();
}
@Param({"1000000"})
diff --git
a/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionSelectorBenchmark.java
b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionSelectorBenchmark.java
index fcc4cf24794..d8e1dfc7646 100644
---
a/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionSelectorBenchmark.java
+++
b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionSelectorBenchmark.java
@@ -25,8 +25,10 @@ import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.java.util.common.guava.Sequence;
import org.apache.druid.java.util.common.io.Closer;
+import org.apache.druid.math.expr.ExpressionProcessing;
import org.apache.druid.query.dimension.DefaultDimensionSpec;
import org.apache.druid.query.dimension.ExtractionDimensionSpec;
+import org.apache.druid.query.expression.LookupEnabledTestExprMacroTable;
import org.apache.druid.query.expression.TestExprMacroTable;
import org.apache.druid.query.extraction.StrlenExtractionFn;
import org.apache.druid.query.extraction.TimeFormatExtractionFn;
@@ -59,21 +61,21 @@ import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;
-
import java.util.BitSet;
import java.util.List;
import java.util.concurrent.TimeUnit;
@State(Scope.Benchmark)
@Fork(value = 1)
-@Warmup(iterations = 15)
-@Measurement(iterations = 30)
+@Warmup(iterations = 3, time = 3)
+@Measurement(iterations = 10, time = 3)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
public class ExpressionSelectorBenchmark
{
static {
NullHandling.initializeForTests();
+ ExpressionProcessing.initializeForTests();
}
@Param({"1000000"})
@@ -451,6 +453,154 @@ public class ExpressionSelectorBenchmark
blackhole.consume(results);
}
+ @Benchmark
+ public void caseSearched1(Blackhole blackhole)
+ {
+ final Sequence<Cursor> cursors = new
QueryableIndexStorageAdapter(index).makeCursors(
+ null,
+ index.getDataInterval(),
+ VirtualColumns.create(
+ ImmutableList.of(
+ new ExpressionVirtualColumn(
+ "v",
+ "case_searched(s == 'asd' || isnull(s) || s == 'xxx', 1, s
== 'foo' || s == 'bar', 2, 3)",
+ ColumnType.LONG,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ ),
+ Granularities.ALL,
+ false,
+ null
+ );
+
+ final List<?> results = cursors
+ .map(cursor -> {
+ final ColumnValueSelector selector =
cursor.getColumnSelectorFactory().makeColumnValueSelector("v");
+ consumeLong(cursor, selector, blackhole);
+ return null;
+ })
+ .toList();
+
+ blackhole.consume(results);
+ }
+
+ @Benchmark
+ public void caseSearched2(Blackhole blackhole)
+ {
+ final Sequence<Cursor> cursors = new
QueryableIndexStorageAdapter(index).makeCursors(
+ null,
+ index.getDataInterval(),
+ VirtualColumns.create(
+ ImmutableList.of(
+ new ExpressionVirtualColumn(
+ "v",
+ "case_searched(s == 'asd' || isnull(s) || n == 1, 1, n ==
2, 2, 3)",
+ ColumnType.LONG,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ ),
+ Granularities.ALL,
+ false,
+ null
+ );
+
+ final List<?> results = cursors
+ .map(cursor -> {
+ final ColumnValueSelector selector =
cursor.getColumnSelectorFactory().makeColumnValueSelector("v");
+ consumeLong(cursor, selector, blackhole);
+ return null;
+ })
+ .toList();
+
+ blackhole.consume(results);
+ }
+
+ @Benchmark
+ public void caseSearchedWithLookup(Blackhole blackhole)
+ {
+ final Sequence<Cursor> cursors = new
QueryableIndexStorageAdapter(index).makeCursors(
+ null,
+ index.getDataInterval(),
+ VirtualColumns.create(
+ ImmutableList.of(
+ new ExpressionVirtualColumn(
+ "v",
+ "case_searched(n == 1001, -1, "
+ + "lookup(s, 'lookyloo') == 'asd1', 1, "
+ + "lookup(s, 'lookyloo') == 'asd2', 2, "
+ + "lookup(s, 'lookyloo') == 'asd3', 3, "
+ + "lookup(s, 'lookyloo') == 'asd4', 4, "
+ + "lookup(s, 'lookyloo') == 'asd5', 5, "
+ + "-2)",
+ ColumnType.LONG,
+ LookupEnabledTestExprMacroTable.INSTANCE
+ )
+ )
+ ),
+ Granularities.ALL,
+ false,
+ null
+ );
+
+ final List<?> results = cursors
+ .map(cursor -> {
+ final ColumnValueSelector selector =
cursor.getColumnSelectorFactory().makeColumnValueSelector("v");
+ consumeLong(cursor, selector, blackhole);
+ return null;
+ })
+ .toList();
+
+ blackhole.consume(results);
+ }
+
+ @Benchmark
+ public void caseSearchedWithLookup2(Blackhole blackhole)
+ {
+ final Sequence<Cursor> cursors = new
QueryableIndexStorageAdapter(index).makeCursors(
+ null,
+ index.getDataInterval(),
+ VirtualColumns.create(
+ ImmutableList.of(
+ new ExpressionVirtualColumn(
+ "ll",
+ "lookup(s, 'lookyloo')",
+ ColumnType.STRING,
+ LookupEnabledTestExprMacroTable.INSTANCE
+ ),
+ new ExpressionVirtualColumn(
+ "v",
+ "case_searched(n == 1001, -1, "
+ + "ll == 'asd1', 1, "
+ + "ll == 'asd2', 2, "
+ + "ll == 'asd3', 3, "
+ + "ll == 'asd4', 4, "
+ + "ll == 'asd5', 5, "
+ + "-2)",
+ ColumnType.LONG,
+ LookupEnabledTestExprMacroTable.INSTANCE
+ )
+ )
+ ),
+ Granularities.ALL,
+ false,
+ null
+ );
+
+ final List<?> results = cursors
+ .map(cursor -> {
+ final ColumnValueSelector selector =
cursor.getColumnSelectorFactory().makeColumnValueSelector("v");
+ consumeLong(cursor, selector, blackhole);
+ return null;
+ })
+ .toList();
+
+ blackhole.consume(results);
+ }
+
+
+
private void consumeDimension(final Cursor cursor, final DimensionSelector
selector, final Blackhole blackhole)
{
if (selector.getValueCardinality() >= 0) {
diff --git
a/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java
b/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java
index fdf6f080ee9..8790b2ed614 100644
--- a/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java
+++ b/processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java
@@ -157,9 +157,12 @@ class BigIntegerExpr extends ConstantExpr<BigInteger>
class LongExpr extends ConstantExpr<Long>
{
+ private final ExprEval expr;
+
LongExpr(Long value)
{
super(ExpressionType.LONG, Preconditions.checkNotNull(value, "value"));
+ expr = ExprEval.ofLong(value);
}
@Override
@@ -171,7 +174,7 @@ class LongExpr extends ConstantExpr<Long>
@Override
public ExprEval eval(ObjectBinding bindings)
{
- return ExprEval.ofLong(value);
+ return expr;
}
@Override
@@ -240,9 +243,12 @@ class NullLongExpr extends ConstantExpr<Long>
class DoubleExpr extends ConstantExpr<Double>
{
+ private final ExprEval expr;
+
DoubleExpr(Double value)
{
super(ExpressionType.DOUBLE, Preconditions.checkNotNull(value, "value"));
+ expr = ExprEval.ofDouble(value);
}
@Override
@@ -254,7 +260,7 @@ class DoubleExpr extends ConstantExpr<Double>
@Override
public ExprEval eval(ObjectBinding bindings)
{
- return ExprEval.ofDouble(value);
+ return expr;
}
@Override
@@ -323,9 +329,12 @@ class NullDoubleExpr extends ConstantExpr<Double>
class StringExpr extends ConstantExpr<String>
{
+ private final ExprEval expr;
+
StringExpr(@Nullable String value)
{
super(ExpressionType.STRING, NullHandling.emptyToNullIfNeeded(value));
+ expr = ExprEval.of(value);
}
@Override
@@ -337,7 +346,7 @@ class StringExpr extends ConstantExpr<String>
@Override
public ExprEval eval(ObjectBinding bindings)
{
- return ExprEval.of(value);
+ return expr;
}
@Override
@@ -464,15 +473,18 @@ class ArrayExpr extends ConstantExpr<Object[]>
class ComplexExpr extends ConstantExpr<Object>
{
+ private final ExprEval expr;
+
protected ComplexExpr(ExpressionType outputType, @Nullable Object value)
{
super(outputType, value);
+ expr = ExprEval.ofComplex(outputType, value);
}
@Override
public ExprEval eval(ObjectBinding bindings)
{
- return ExprEval.ofComplex(outputType, value);
+ return expr;
}
@Override
diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java
b/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java
index 35f59e5ebe7..e62021623bd 100644
--- a/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java
+++ b/processing/src/main/java/org/apache/druid/math/expr/ExprEval.java
@@ -363,7 +363,7 @@ public abstract class ExprEval<T>
case DOUBLE:
return ExprEval.of(Evals.asDouble(value));
case LONG:
- return ExprEval.of(Evals.asLong(value));
+ return ofLongBoolean(value);
case STRING:
return ExprEval.of(String.valueOf(value));
default:
@@ -376,7 +376,7 @@ public abstract class ExprEval<T>
*/
public static ExprEval ofLongBoolean(boolean value)
{
- return ExprEval.of(Evals.asLong(value));
+ return value ? LongExprEval.TRUE : LongExprEval.FALSE;
}
public static ExprEval ofComplex(ExpressionType outputType, @Nullable Object
value)
@@ -922,6 +922,8 @@ public abstract class ExprEval<T>
private static class LongExprEval extends NumericExprEval
{
+ private static final LongExprEval TRUE = new
LongExprEval(Evals.asLong(true));
+ private static final LongExprEval FALSE = new
LongExprEval(Evals.asLong(false));
private static final LongExprEval OF_NULL = new LongExprEval(null);
private LongExprEval(@Nullable Number value)
diff --git
a/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
b/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
index ba576afa0c1..efe0328a837 100644
---
a/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
+++
b/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
@@ -55,6 +55,9 @@ public class ExpressionTypeConversion
{
ExpressionType type = eval.type();
ExpressionType otherType = otherEval.type();
+ if (type == otherType && type.getType().isPrimitive()) {
+ return type;
+ }
if (Types.is(type, ExprType.STRING) && Types.is(otherType,
ExprType.STRING)) {
return ExpressionType.STRING;
}
diff --git a/processing/src/test/java/org/apache/druid/math/expr/ExprTest.java
b/processing/src/test/java/org/apache/druid/math/expr/ExprTest.java
index 3c089ac5b6a..f8d222e0838 100644
--- a/processing/src/test/java/org/apache/druid/math/expr/ExprTest.java
+++ b/processing/src/test/java/org/apache/druid/math/expr/ExprTest.java
@@ -139,7 +139,7 @@ public class ExprTest
public void testEqualsContractForStringExpr()
{
EqualsVerifier.forClass(StringExpr.class)
- .withIgnoredFields("outputType")
+ .withIgnoredFields("outputType", "expr")
.withPrefabValues(ExpressionType.class,
ExpressionType.STRING, ExpressionType.DOUBLE)
.usingGetClass()
.verify();
@@ -149,7 +149,7 @@ public class ExprTest
public void testEqualsContractForDoubleExpr()
{
EqualsVerifier.forClass(DoubleExpr.class)
- .withIgnoredFields("outputType")
+ .withIgnoredFields("outputType", "expr")
.withPrefabValues(ExpressionType.class,
ExpressionType.DOUBLE, ExpressionType.LONG)
.usingGetClass()
.verify();
@@ -159,7 +159,7 @@ public class ExprTest
public void testEqualsContractForLongExpr()
{
EqualsVerifier.forClass(LongExpr.class)
- .withIgnoredFields("outputType")
+ .withIgnoredFields("outputType", "expr")
.withPrefabValues(ExpressionType.class, ExpressionType.LONG,
ExpressionType.STRING)
.usingGetClass()
.verify();
@@ -187,6 +187,7 @@ public class ExprTest
ExpressionTypeFactory.getInstance().ofComplex("bar")
)
.withNonnullFields("outputType")
+ .withIgnoredFields("expr")
.usingGetClass()
.verify();
}
diff --git
a/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
b/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
index f8d663abc70..d313749fef8 100644
--- a/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
+++ b/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
@@ -453,6 +453,8 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
final ExprEval<?> longEval = ExprEval.of(1L);
final ExprEval<?> doubleEval = ExprEval.of(1.0);
final ExprEval<?> arrayEval = ExprEval.ofLongArray(new Long[]{1L, 2L, 3L});
+ final ExprEval<?> complexEval =
ExprEval.ofComplex(ExpressionType.UNKNOWN_COMPLEX, new Object());
+ final ExprEval<?> complexEval2 = ExprEval.ofComplex(new
ExpressionType(ExprType.COMPLEX, null, null), new Object());
// only long stays long
Assert.assertEquals(ExpressionType.LONG,
ExpressionTypeConversion.autoDetect(longEval, longEval));
@@ -479,6 +481,15 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
Assert.assertEquals(ExpressionType.DOUBLE,
ExpressionTypeConversion.autoDetect(nullStringEval, arrayEval));
Assert.assertEquals(ExpressionType.DOUBLE,
ExpressionTypeConversion.autoDetect(doubleEval, arrayEval));
Assert.assertEquals(ExpressionType.DOUBLE,
ExpressionTypeConversion.autoDetect(longEval, arrayEval));
+
+ Assert.assertEquals(ExpressionType.DOUBLE,
ExpressionTypeConversion.autoDetect(longEval, complexEval));
+ Assert.assertEquals(ExpressionType.DOUBLE,
ExpressionTypeConversion.autoDetect(doubleEval, complexEval));
+ Assert.assertEquals(ExpressionType.DOUBLE,
ExpressionTypeConversion.autoDetect(arrayEval, complexEval));
+ Assert.assertEquals(ExpressionType.DOUBLE,
ExpressionTypeConversion.autoDetect(complexEval, complexEval));
+ Assert.assertEquals(
+ ExpressionTypeConversion.autoDetect(complexEval, complexEval),
+ ExpressionTypeConversion.autoDetect(complexEval2, complexEval)
+ );
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
