Skip to content

Commit

Permalink
[CALCITE-2902] Improve performance of AbstractRelNode#computeDigest
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@zabetak
zabetak committed Mar 11, 2019
1 parent a481357 commit f33e1b8
Show file tree
Hide file tree
Showing 5 changed files with 340 additions and 79 deletions.
117 changes: 69 additions & 48 deletions core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
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;
Expand All @@ -46,8 +45,6 @@

import org.slf4j.Logger;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -190,16 +187,14 @@ public void register(RelOptPlanner planner) {
}

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) {
Expand Down Expand Up @@ -389,42 +384,68 @@ public RelOptTable getTable() {
* @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;
}
}
}

Expand Down
12 changes: 9 additions & 3 deletions core/src/main/java/org/apache/calcite/rel/RelWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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
13 changes: 1 addition & 12 deletions core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ public SqlExplainLevel getDetailLevel() {
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;
Expand All @@ -125,13 +121,6 @@ private List<Object> getList(List<Pair<String, Object>> values, String tag) {
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);
Expand All @@ -140,7 +129,7 @@ public RelWriter done(RelNode node) {
return this;
}

public boolean nest() {
@Override public boolean nest() {
return true;
}

Expand Down
16 changes: 0 additions & 16 deletions core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,23 +131,11 @@ public SqlExplainLevel getDetailLevel() {
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 =
Expand All @@ -170,10 +158,6 @@ private boolean checkInputsPresentInExplain(RelNode node) {
return true;
}

public boolean nest() {
return false;
}

/**
* Converts the collected terms and values to a string. Does not write to
* the parent writer.
Expand Down
Loading

0 comments on commit f33e1b8

Please sign in to comment.