This is an automated email from the ASF dual-hosted git repository.
zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new f33e1b8 [CALCITE-2902] Improve performance of
AbstractRelNode#computeDigest
f33e1b8 is described below
commit f33e1b8f5609f11b039577694023241156951c08
Author: Stamatis Zampetakis <zabe...@gmail.com>
AuthorDate: Fri Mar 8 18:56:44 2019 +0100
[CALCITE-2902] Improve performance of AbstractRelNode#computeDigest
1. Use StringBuilder instead of PrintWriter for building the digest, which
is more efficient to initialize and append to.
2. Improve (sligthly) the performance of AbstractRelNode#getRelTypeName
(used in the digest) by performing:
a. one lastIndexOf operation instead of potentially two;
b. char instead of char[] array comparison.
3. Add micro benchmark with different implementations of
AbstractRelNode#getRelTypeName showing the benefit of the adopted
implementation.
4. Improve RelWriter interface with default methods and refactor respective
classes.
---
.../org/apache/calcite/rel/AbstractRelNode.java | 117 +++++----
.../java/org/apache/calcite/rel/RelWriter.java | 12 +-
.../calcite/rel/externalize/RelJsonWriter.java | 13 +-
.../calcite/rel/externalize/RelWriterImpl.java | 16 --
.../AbstractRelNodeGetRelTypeNameBenchmark.java | 261 +++++++++++++++++++++
5 files changed, 340 insertions(+), 79 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
index 59a9179..f4899a1 100644
--- a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
+++ b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
@@ -27,7 +27,6 @@ import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.plan.RelTrait;
import org.apache.calcite.plan.RelTraitSet;
import org.apache.calcite.rel.core.CorrelationId;
-import org.apache.calcite.rel.externalize.RelWriterImpl;
import org.apache.calcite.rel.metadata.Metadata;
import org.apache.calcite.rel.metadata.MetadataFactory;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
@@ -46,8 +45,6 @@ import com.google.common.collect.ImmutableSet;
import org.slf4j.Logger;
-import java.io.PrintWriter;
-import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -190,16 +187,14 @@ public abstract class AbstractRelNode implements RelNode {
}
public final String getRelTypeName() {
- String className = getClass().getName();
- int i = className.lastIndexOf("$");
- if (i >= 0) {
- return className.substring(i + 1);
- }
- i = className.lastIndexOf(".");
- if (i >= 0) {
- return className.substring(i + 1);
+ String cn = getClass().getName();
+ int i = cn.length();
+ while (--i >= 0) {
+ if (cn.charAt(i) == '$' || cn.charAt(i) == '.') {
+ return cn.substring(i + 1);
+ }
}
- return className;
+ return cn;
}
public boolean isValid(Litmus litmus, Context context) {
@@ -389,42 +384,68 @@ public abstract class AbstractRelNode implements RelNode {
* @return Digest
*/
protected String computeDigest() {
- StringWriter sw = new StringWriter();
- RelWriter pw =
- new RelWriterImpl(
- new PrintWriter(sw),
- SqlExplainLevel.DIGEST_ATTRIBUTES, false) {
- protected void explain_(
- RelNode rel, List<Pair<String, Object>> values) {
- pw.write(getRelTypeName());
-
- for (RelTrait trait : traitSet) {
- pw.write(".");
- pw.write(trait.toString());
- }
-
- pw.write("(");
- int j = 0;
- for (Pair<String, Object> value : values) {
- if (j++ > 0) {
- pw.write(",");
- }
- pw.write(value.left);
- pw.write("=");
- if (value.right instanceof RelNode) {
- RelNode input = (RelNode) value.right;
- pw.write(input.getRelTypeName());
- pw.write("#");
- pw.write(Integer.toString(input.getId()));
- } else {
- pw.write(String.valueOf(value.right));
- }
- }
- pw.write(")");
- }
- };
- explain(pw);
- return sw.toString();
+ RelDigestWriter rdw = new RelDigestWriter();
+ explain(rdw);
+ return rdw.digest;
+ }
+
+ /**
+ * A writer object used exclusively for computing the digest of a RelNode.
+ *
+ * <p>The writer is meant to be used only for computing a single digest and
then thrown away.
+ * After calling {@link #done(RelNode)} the writer should be used only to
obtain the computed
+ * {@link #digest}. Any other action is prohibited.</p>
+ *
+ */
+ private static final class RelDigestWriter implements RelWriter {
+
+ private final List<Pair<String, Object>> values = new ArrayList<>();
+
+ String digest = null;
+
+ @Override public void explain(final RelNode rel, final List<Pair<String,
Object>> valueList) {
+ throw new IllegalStateException("Should not be called for computing
digest");
+ }
+
+ @Override public SqlExplainLevel getDetailLevel() {
+ return SqlExplainLevel.DIGEST_ATTRIBUTES;
+ }
+
+ @Override public RelWriter item(String term, Object value) {
+ values.add(Pair.of(term, value));
+ return this;
+ }
+
+ @Override public RelWriter done(RelNode node) {
+ StringBuilder sb = new StringBuilder();
+ sb.append(node.getRelTypeName());
+
+ for (RelTrait trait : node.getTraitSet()) {
+ sb.append('.');
+ sb.append(trait.toString());
+ }
+
+ sb.append('(');
+ int j = 0;
+ for (Pair<String, Object> value : values) {
+ if (j++ > 0) {
+ sb.append(',');
+ }
+ sb.append(value.left);
+ sb.append('=');
+ if (value.right instanceof RelNode) {
+ RelNode input = (RelNode) value.right;
+ sb.append(input.getRelTypeName());
+ sb.append('#');
+ sb.append(input.getId());
+ } else {
+ sb.append(value.right);
+ }
+ }
+ sb.append(')');
+ digest = sb.toString();
+ return this;
+ }
}
}
diff --git a/core/src/main/java/org/apache/calcite/rel/RelWriter.java
b/core/src/main/java/org/apache/calcite/rel/RelWriter.java
index fa63bc2..551685c 100644
--- a/core/src/main/java/org/apache/calcite/rel/RelWriter.java
+++ b/core/src/main/java/org/apache/calcite/rel/RelWriter.java
@@ -53,7 +53,9 @@ public interface RelWriter {
* @param term Term for input, e.g. "left" or "input #1".
* @param input Input relational expression
*/
- RelWriter input(String term, RelNode input);
+ default RelWriter input(String term, RelNode input) {
+ return item(term, input);
+ }
/**
* Adds an attribute to the explanation of the current node.
@@ -67,7 +69,9 @@ public interface RelWriter {
* Adds an input to the explanation of the current node, if a condition
* holds.
*/
- RelWriter itemIf(String term, Object value, boolean condition);
+ default RelWriter itemIf(String term, Object value, boolean condition) {
+ return condition ? item(term, value) : this;
+ }
/**
* Writes the completed explanation.
@@ -78,7 +82,9 @@ public interface RelWriter {
* Returns whether the writer prefers nested values. Traditional explain
* writers prefer flattened values.
*/
- boolean nest();
+ default boolean nest() {
+ return false;
+ }
}
// End RelWriter.java
diff --git
a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java
b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java
index fb96778..c2105cb 100644
--- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java
+++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java
@@ -104,10 +104,6 @@ public class RelJsonWriter implements RelWriter {
return SqlExplainLevel.ALL_ATTRIBUTES;
}
- public RelWriter input(String term, RelNode input) {
- return this;
- }
-
public RelWriter item(String term, Object value) {
values.add(Pair.of(term, value));
return this;
@@ -125,13 +121,6 @@ public class RelJsonWriter implements RelWriter {
return list;
}
- public RelWriter itemIf(String term, Object value, boolean condition) {
- if (condition) {
- item(term, value);
- }
- return this;
- }
-
public RelWriter done(RelNode node) {
final List<Pair<String, Object>> valuesCopy =
ImmutableList.copyOf(values);
@@ -140,7 +129,7 @@ public class RelJsonWriter implements RelWriter {
return this;
}
- public boolean nest() {
+ @Override public boolean nest() {
return true;
}
diff --git
a/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java
b/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java
index af2985e..9dade7b 100644
--- a/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java
+++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java
@@ -131,23 +131,11 @@ public class RelWriterImpl implements RelWriter {
return detailLevel;
}
- public RelWriter input(String term, RelNode input) {
- values.add(Pair.of(term, (Object) input));
- return this;
- }
-
public RelWriter item(String term, Object value) {
values.add(Pair.of(term, value));
return this;
}
- public RelWriter itemIf(String term, Object value, boolean condition) {
- if (condition) {
- item(term, value);
- }
- return this;
- }
-
public RelWriter done(RelNode node) {
assert checkInputsPresentInExplain(node);
final List<Pair<String, Object>> valuesCopy =
@@ -170,10 +158,6 @@ public class RelWriterImpl implements RelWriter {
return true;
}
- public boolean nest() {
- return false;
- }
-
/**
* Converts the collected terms and values to a string. Does not write to
* the parent writer.
diff --git
a/ubenchmark/src/main/java/org/apache/calcite/benchmarks/AbstractRelNodeGetRelTypeNameBenchmark.java
b/ubenchmark/src/main/java/org/apache/calcite/benchmarks/AbstractRelNodeGetRelTypeNameBenchmark.java
new file mode 100644
index 0000000..3321881
--- /dev/null
+++
b/ubenchmark/src/main/java/org/apache/calcite/benchmarks/AbstractRelNodeGetRelTypeNameBenchmark.java
@@ -0,0 +1,261 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.benchmarks;
+
+
+import org.apache.calcite.rel.AbstractRelNode;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A benchmark of alternative implementations for {@link
AbstractRelNode#getRelTypeName()}
+ * method.
+ */
+@Fork(value = 1, jvmArgsPrepend = "-Xmx1024m")
+@Measurement(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
+@Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
+@Threads(1)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@BenchmarkMode(Mode.AverageTime)
+public class AbstractRelNodeGetRelTypeNameBenchmark {
+
+ /**
+ * A state holding the full class names of all built-in implementors of the
+ * {@link org.apache.calcite.rel.RelNode} interface.
+ */
+ @State(Scope.Thread)
+ public static class ClassNameState {
+
+ private final String[] fullNames = new String[]{
+ "org.apache.calcite.interpreter.InterpretableRel",
+ "org.apache.calcite.interpreter.BindableRel",
+ "org.apache.calcite.adapter.enumerable.EnumerableInterpretable",
+ "org.apache.calcite.adapter.enumerable.EnumerableRel",
+ "org.apache.calcite.adapter.enumerable.EnumerableLimit",
+ "org.apache.calcite.adapter.enumerable.EnumerableUnion",
+ "org.apache.calcite.adapter.enumerable.EnumerableCollect",
+ "org.apache.calcite.adapter.enumerable.EnumerableTableFunctionScan",
+ "org.apache.calcite.adapter.enumerable.EnumerableValues",
+ "org.apache.calcite.adapter.enumerable.EnumerableSemiJoin",
+ "org.apache.calcite.adapter.enumerable.EnumerableMinus",
+ "org.apache.calcite.adapter.enumerable.EnumerableIntersect",
+ "org.apache.calcite.adapter.enumerable.EnumerableUncollect",
+ "org.apache.calcite.adapter.enumerable.EnumerableMergeJoin",
+ "org.apache.calcite.adapter.enumerable.EnumerableProject",
+ "org.apache.calcite.adapter.enumerable.EnumerableFilter",
+ "org.apache.calcite.adapter.jdbc.JdbcToEnumerableConverter",
+ "org.apache.calcite.adapter.enumerable.EnumerableThetaJoin",
+ "org.apache.calcite.adapter.enumerable.EnumerableTableScan",
+ "org.apache.calcite.adapter.enumerable.EnumerableJoin",
+ "org.apache.calcite.adapter.enumerable.EnumerableTableModify",
+ "org.apache.calcite.adapter.enumerable.EnumerableAggregate",
+ "org.apache.calcite.adapter.enumerable.EnumerableCorrelate",
+ "org.apache.calcite.adapter.enumerable.EnumerableSort",
+ "org.apache.calcite.adapter.enumerable.EnumerableWindow",
+ "org.apache.calcite.plan.volcano.VolcanoPlannerTraitTest$FooRel",
+ "org.apache.calcite.adapter.enumerable.EnumerableCalc",
+ "org.apache.calcite.adapter.enumerable.EnumerableInterpreter",
+ "org.apache.calcite.adapter.geode.rel.GeodeToEnumerableConverter",
+ "org.apache.calcite.adapter.pig.PigToEnumerableConverter",
+ "org.apache.calcite.adapter.mongodb.MongoToEnumerableConverter",
+ "org.apache.calcite.adapter.csv.CsvTableScan",
+ "org.apache.calcite.adapter.spark.SparkToEnumerableConverter",
+
"org.apache.calcite.adapter.elasticsearch.ElasticsearchToEnumerableConverter",
+ "org.apache.calcite.adapter.file.FileTableScan",
+ "org.apache.calcite.adapter.cassandra.CassandraToEnumerableConverter",
+ "org.apache.calcite.adapter.splunk.SplunkTableScan",
+ "org.apache.calcite.adapter.jdbc.JdbcRel",
+ "org.apache.calcite.adapter.jdbc.JdbcTableScan",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcJoin",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcCalc",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcProject",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcFilter",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcAggregate",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcSort",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcUnion",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcIntersect",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcMinus",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcTableModify",
+ "org.apache.calcite.adapter.jdbc.JdbcRules$JdbcValues",
+ "org.apache.calcite.tools.PlannerTest$MockJdbcTableScan",
+ "org.apache.calcite.rel.AbstractRelNode",
+ "org.apache.calcite.rel.rules.MultiJoin",
+ "org.apache.calcite.rel.core.TableFunctionScan",
+ "org.apache.calcite.rel.BiRel",
+ "org.apache.calcite.rel.SingleRel",
+ "org.apache.calcite.rel.core.Values",
+ "org.apache.calcite.rel.core.TableScan",
+ "org.apache.calcite.plan.hep.HepRelVertex",
+ "org.apache.calcite.plan.RelOptPlanReaderTest$MyRel",
+ "org.apache.calcite.plan.volcano.TraitPropagationTest$PhysTable",
+ "org.apache.calcite.plan.volcano.PlannerTests$TestLeafRel",
+ "org.apache.calcite.plan.volcano.RelSubset",
+ "org.apache.calcite.rel.core.SetOp",
+ "org.apache.calcite.plan.volcano.VolcanoPlannerTraitTest$TestLeafRel",
+ "org.apache.calcite.adapter.druid.DruidQuery",
+
"org.apache.calcite.sql2rel.RelStructuredTypeFlattener$SelfFlatteningRel",
+ "org.apache.calcite.rel.convert.Converter",
+ "org.apache.calcite.rel.convert.ConverterImpl",
+ "org.apache.calcite.plan.volcano.TraitPropagationTest$Phys",
+ "org.apache.calcite.plan.volcano.TraitPropagationTest$PhysTable",
+ "org.apache.calcite.plan.volcano.TraitPropagationTest$PhysSort",
+ "org.apache.calcite.plan.volcano.TraitPropagationTest$PhysAgg",
+ "org.apache.calcite.plan.volcano.TraitPropagationTest$PhysProj",
+ "org.apache.calcite.interpreter.BindableRel",
+ "org.apache.calcite.adapter.enumerable.EnumerableBindable",
+ "org.apache.calcite.interpreter.Bindables$BindableTableScan",
+ "org.apache.calcite.interpreter.Bindables$BindableFilter",
+ "org.apache.calcite.interpreter.Bindables$BindableProject",
+ "org.apache.calcite.interpreter.Bindables$BindableSort",
+ "org.apache.calcite.interpreter.Bindables$BindableJoin",
+ "org.apache.calcite.interpreter.Bindables$BindableUnion",
+ "org.apache.calcite.interpreter.Bindables$BindableValues",
+ "org.apache.calcite.interpreter.Bindables$BindableAggregate",
+ "org.apache.calcite.interpreter.Bindables$BindableWindow",
+ "org.apache.calcite.adapter.druid.DruidQuery",
+ "org.apache.calcite.adapter.cassandra.CassandraRel",
+ "org.apache.calcite.adapter.cassandra.CassandraFilter",
+ "org.apache.calcite.adapter.cassandra.CassandraProject",
+ "org.apache.calcite.adapter.cassandra.CassandraLimit",
+ "org.apache.calcite.adapter.cassandra.CassandraSort",
+ "org.apache.calcite.adapter.cassandra.CassandraTableScan",
+ "org.apache.calcite.adapter.mongodb.MongoRel",
+ "org.apache.calcite.adapter.mongodb.MongoTableScan",
+ "org.apache.calcite.adapter.mongodb.MongoProject",
+ "org.apache.calcite.adapter.mongodb.MongoFilter",
+ "org.apache.calcite.adapter.mongodb.MongoAggregate",
+ "org.apache.calcite.adapter.mongodb.MongoSort",
+ "org.apache.calcite.adapter.spark.SparkRel",
+ "org.apache.calcite.adapter.spark.JdbcToSparkConverter",
+ "org.apache.calcite.adapter.spark.SparkRules$SparkValues",
+ "org.apache.calcite.adapter.spark.EnumerableToSparkConverter",
+ "org.apache.calcite.adapter.spark.SparkRules$SparkCalc",
+ "org.apache.calcite.adapter.elasticsearch.ElasticsearchRel",
+ "org.apache.calcite.adapter.elasticsearch.ElasticsearchFilter",
+ "org.apache.calcite.adapter.elasticsearch.ElasticsearchProject",
+ "org.apache.calcite.adapter.elasticsearch.ElasticsearchAggregate",
+ "org.apache.calcite.adapter.elasticsearch.ElasticsearchTableScan",
+ "org.apache.calcite.adapter.elasticsearch.ElasticsearchSort",
+ "org.apache.calcite.adapter.geode.rel.GeodeRel",
+ "org.apache.calcite.adapter.geode.rel.GeodeSort",
+ "org.apache.calcite.adapter.geode.rel.GeodeTableScan",
+ "org.apache.calcite.adapter.geode.rel.GeodeProject",
+ "org.apache.calcite.adapter.geode.rel.GeodeFilter",
+ "org.apache.calcite.adapter.geode.rel.GeodeAggregate",
+ "org.apache.calcite.adapter.pig.PigRel",
+ "org.apache.calcite.adapter.pig.PigTableScan",
+ "org.apache.calcite.adapter.pig.PigAggregate",
+ "org.apache.calcite.adapter.pig.PigJoin",
+ "org.apache.calcite.adapter.pig.PigFilter",
+ "org.apache.calcite.adapter.pig.PigProject"
+ };
+
+ @Param({"11", "31", "63"})
+ private long seed;
+
+ private Random r = null;
+
+ /**
+ * Setups the random number generator at the beginning of each iteration.
+ *
+ * To have relatively comparable results the generator should always use
the same seed for the
+ * whole duration of the benchmark.
+ */
+ @Setup(Level.Iteration)
+ public void setupRandom() {
+ r = new Random(seed);
+ }
+
+ /**
+ * Returns a pseudo random class name which corresponds to an implementor
of the RelNode
+ * interface.
+ */
+ public String nextName() {
+ return fullNames[r.nextInt(fullNames.length)];
+ }
+ }
+
+ @Benchmark
+ public String useStringLastIndexOfTwoTimesV1(ClassNameState state) {
+ String cn = state.nextName();
+ int i = cn.lastIndexOf("$");
+ if (i >= 0) {
+ return cn.substring(i + 1);
+ }
+ i = cn.lastIndexOf(".");
+ if (i >= 0) {
+ return cn.substring(i + 1);
+ }
+ return cn;
+ }
+
+ @Benchmark
+ public String useStringLastIndexOfTwoTimeV2(ClassNameState state) {
+ String cn = state.nextName();
+ int i = cn.lastIndexOf('$');
+ if (i >= 0) {
+ return cn.substring(i + 1);
+ }
+ i = cn.lastIndexOf('.');
+ if (i >= 0) {
+ return cn.substring(i + 1);
+ }
+ return cn;
+ }
+
+ @Benchmark
+ public String useCustomLastIndexOf(ClassNameState state) {
+ String cn = state.nextName();
+ int i = cn.length();
+ while (--i >= 0) {
+ if (cn.charAt(i) == '$' || cn.charAt(i) == '.') {
+ return cn.substring(i + 1);
+ }
+ }
+ return cn;
+ }
+
+ public static void main(String[] args) throws RunnerException {
+ Options opt = new OptionsBuilder()
+ .include(AbstractRelNodeGetRelTypeNameBenchmark.class.getName())
+ .detectJvmArgs()
+ .build();
+
+ new Runner(opt).run();
+ }
+}
+
+// End AbstractRelNodeGetRelTypeNameBenchmark.java
