You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by gj...@apache.org on 2019/09/26 16:44:34 UTC
[phoenix] branch 4.x-HBase-1.5 updated: PHOENIX-5491 Improve the
performance of InListExpression.hashCode()
This is an automated email from the ASF dual-hosted git repository.
gjacoby pushed a commit to branch 4.x-HBase-1.5
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x-HBase-1.5 by this push:
new 1899a3f PHOENIX-5491 Improve the performance of InListExpression.hashCode()
1899a3f is described below
commit 1899a3fdbee5bdb740020a898bc7183858dad82b
Author: chfeng <ch...@tencent.com>
AuthorDate: Tue Sep 24 20:35:41 2019 +0800
PHOENIX-5491 Improve the performance of InListExpression.hashCode()
Signed-off-by: Geoffrey Jacoby <gj...@apache.org>
---
.../phoenix/expression/InListExpression.java | 37 ++++++++----
.../phoenix/expression/InListExpressionTest.java | 65 ++++++++++++++++++++++
2 files changed, 91 insertions(+), 11 deletions(-)
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
index a0a2ccc..4ee08ee 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
@@ -41,10 +41,10 @@ import org.apache.phoenix.util.ByteUtil;
import org.apache.phoenix.util.ExpressionUtil;
import org.apache.phoenix.util.StringUtil;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
-
/*
* Implementation of a SQL foo IN (a,b,c) expression. Other than the first
* expression, child expressions must be constants.
@@ -59,17 +59,20 @@ public class InListExpression extends BaseSingleExpression {
private List<Expression> keyExpressions; // client side only
private boolean rowKeyOrderOptimizable; // client side only
+ // reduce hashCode() complexity
+ private int hashCode = -1;
+ private boolean hashCodeSet = false;
public static Expression create (List<Expression> children, boolean isNegate, ImmutableBytesWritable ptr, boolean rowKeyOrderOptimizable) throws SQLException {
Expression firstChild = children.get(0);
-
+
if (firstChild.isStateless() && (!firstChild.evaluate(null, ptr) || ptr.getLength() == 0)) {
return LiteralExpression.newConstant(null, PBoolean.INSTANCE, firstChild.getDeterminism());
}
if (children.size() == 2) {
return ComparisonExpression.create(isNegate ? CompareOp.NOT_EQUAL : CompareOp.EQUAL, children, ptr, rowKeyOrderOptimizable);
}
-
+
boolean addedNull = false;
SQLException sqlE = null;
List<Expression> coercedKeyExpressions = Lists.newArrayListWithExpectedSize(children.size());
@@ -93,7 +96,7 @@ public class InListExpression extends BaseSingleExpression {
return LiteralExpression.newConstant(null, PBoolean.INSTANCE, Determinism.ALWAYS);
}
Expression expression = new InListExpression(coercedKeyExpressions, rowKeyOrderOptimizable);
- if (isNegate) {
+ if (isNegate) {
expression = NotExpression.create(expression, ptr);
}
if (ExpressionUtil.isConstant(expression)) {
@@ -101,10 +104,16 @@ public class InListExpression extends BaseSingleExpression {
}
return expression;
}
-
+
public InListExpression() {
}
+ @VisibleForTesting
+ protected InListExpression(List<ImmutableBytesPtr> values) {
+ this.children = Collections.emptyList();
+ this.values = Sets.newHashSet(values);
+ }
+
public InListExpression(List<Expression> keyExpressions, boolean rowKeyOrderOptimizable) {
super(keyExpressions.get(0));
this.rowKeyOrderOptimizable = rowKeyOrderOptimizable;
@@ -132,7 +141,7 @@ public class InListExpression extends BaseSingleExpression {
} else {
isFixedLength &= fixedWidth == length;
}
-
+
valuesByteLength += ptr.getLength();
}
}
@@ -152,6 +161,7 @@ public class InListExpression extends BaseSingleExpression {
// minValue and maxValue but can infer them based on the first and last position.
this.values = new LinkedHashSet<ImmutableBytesPtr>(Arrays.asList(valuesArray));
}
+ this.hashCodeSet = false;
}
@Override
@@ -173,10 +183,14 @@ public class InListExpression extends BaseSingleExpression {
@Override
public int hashCode() {
- final int prime = 31;
- int result = 1;
- result = prime * result + children.hashCode() + values.hashCode();
- return result;
+ if (!hashCodeSet) {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + children.hashCode() + values.hashCode();
+ hashCode = result;
+ hashCodeSet = true;
+ }
+ return hashCode;
}
@Override
@@ -199,7 +213,7 @@ public class InListExpression extends BaseSingleExpression {
values.add(new ImmutableBytesPtr(valuesBytes,offset,valueLen));
return offset + valueLen;
}
-
+
@Override
public void readFields(DataInput input) throws IOException {
super.readFields(input);
@@ -210,6 +224,7 @@ public class InListExpression extends BaseSingleExpression {
int len = fixedWidth == -1 ? WritableUtils.readVInt(input) : valuesByteLength / fixedWidth;
// TODO: consider using a regular HashSet as we never serialize from the server-side
values = Sets.newLinkedHashSetWithExpectedSize(len);
+ hashCodeSet = false;
int offset = 0;
int i = 0;
if (i < len) {
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java
new file mode 100644
index 0000000..3d765ab
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java
@@ -0,0 +1,65 @@
+/*
+ * 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.phoenix.expression;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.Lists;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
+import org.junit.Test;
+
+public class InListExpressionTest {
+
+ @Test
+ public void testHashCode() throws Exception {
+ int valuesNumber = 500000;
+ List<ImmutableBytesPtr> values = new ArrayList<>(valuesNumber);
+ for (int i = 0; i < valuesNumber; i++) {
+ values.add(new ImmutableBytesPtr(Bytes.toBytes(i)));
+ }
+ InListExpression exp = new InListExpression(values);
+
+ // first time
+ long startTs = System.currentTimeMillis();
+ int firstHashCode = exp.hashCode();
+ long firstTimeCost = System.currentTimeMillis() - startTs;
+
+ // the rest access
+ int restAccessNumber = 3;
+ startTs = System.currentTimeMillis();
+ List<Integer> hashCodes = Lists.newArrayListWithExpectedSize(restAccessNumber);
+ for (int i = 0; i < restAccessNumber; i++) {
+ hashCodes.add(exp.hashCode());
+ }
+
+ // check time cost
+ long restTimeCost = System.currentTimeMillis() - startTs;
+ assertTrue("first time: " + firstTimeCost + " <= rest time: " + restTimeCost,
+ firstTimeCost > restTimeCost);
+
+ // check hash code
+ for (int hashCode : hashCodes) {
+ assertEquals("hash code not equal, firstHashCode: " + firstHashCode + ", restHashCode: "
+ + hashCode, firstHashCode, hashCode);
+ }
+ }
+}