Optimize array hash code

Avoid extra object allocations
7mming7 · Dec 14, 2015 · cc82199 · cc82199
1 parent f91b6ed
commit cc82199
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 10 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/operator/scalar/ArrayHashCodeOperator.java b/presto-main/src/main/java/com/facebook/presto/operator/scalar/ArrayHashCodeOperator.java
@@ -16,8 +16,6 @@
 import com.facebook.presto.metadata.FunctionRegistry;
 import com.facebook.presto.metadata.SqlOperator;
 import com.facebook.presto.spi.block.Block;
-import com.facebook.presto.spi.block.BlockBuilder;
-import com.facebook.presto.spi.block.BlockBuilderStatus;
 import com.facebook.presto.spi.type.StandardTypes;
 import com.facebook.presto.spi.type.Type;
 import com.facebook.presto.spi.type.TypeManager;
@@ -28,6 +26,8 @@
 
 import static com.facebook.presto.metadata.OperatorType.HASH_CODE;
 import static com.facebook.presto.metadata.Signature.comparableTypeParameter;
+import static com.facebook.presto.type.ArrayType.ARRAY_NULL_ELEMENT_MSG;
+import static com.facebook.presto.type.TypeUtils.checkElementNotNull;
 import static com.facebook.presto.util.Reflection.methodHandle;
 
 public class ArrayHashCodeOperator
@@ -45,14 +45,17 @@ private ArrayHashCodeOperator()
     public ScalarFunctionImplementation specialize(Map<String, Type> types, int arity, TypeManager typeManager, FunctionRegistry functionRegistry)
     {
         Type type = types.get("T");
-        type = typeManager.getParameterizedType(StandardTypes.ARRAY, ImmutableList.of(type.getTypeSignature()), ImmutableList.of());
         return new ScalarFunctionImplementation(false, ImmutableList.of(false), METHOD_HANDLE.bindTo(type), isDeterministic());
     }
 
     public static long hash(Type type, Block block)
     {
-        BlockBuilder blockBuilder = type.createBlockBuilder(new BlockBuilderStatus(), 1);
-        blockBuilder.writeObject(block).closeEntry();
-        return type.hash(blockBuilder.build(), 0);
+        int hash = 0;
+        for (int i = 0; i < block.getPositionCount(); i++) {
+            checkElementNotNull(block.isNull(i), ARRAY_NULL_ELEMENT_MSG);
+            // TODO: This should be migrated away from Type.hash and invoke a MethodHandle for the hash code operator instead
+            hash = (int) CombineHashFunction.getHash(hash, type.hash(block, i));
+        }
+        return hash;
     }
 }
diff --git a/presto-main/src/main/java/com/facebook/presto/type/ArrayType.java b/presto-main/src/main/java/com/facebook/presto/type/ArrayType.java
@@ -13,6 +13,7 @@
  */
 package com.facebook.presto.type;
 
+import com.facebook.presto.operator.scalar.CombineHashFunction;
 import com.facebook.presto.spi.ConnectorSession;
 import com.facebook.presto.spi.block.ArrayBlockBuilder;
 import com.facebook.presto.spi.block.Block;
@@ -26,7 +27,6 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.Objects;
 
 import static com.facebook.presto.type.TypeUtils.checkElementNotNull;
 import static com.facebook.presto.type.TypeUtils.parameterizedTypeName;
@@ -86,12 +86,12 @@ public boolean equalTo(Block leftBlock, int leftPosition, Block rightBlock, int
     public int hash(Block block, int position)
     {
         Block array = getObject(block, position);
-        List<Integer> hashArray = new ArrayList<>(array.getPositionCount());
+        int hash = 0;
         for (int i = 0; i < array.getPositionCount(); i++) {
             checkElementNotNull(array.isNull(i), ARRAY_NULL_ELEMENT_MSG);
-            hashArray.add(elementType.hash(array, i));
+            hash = (int) CombineHashFunction.getHash(hash, elementType.hash(array, i));
         }
-        return Objects.hash(hashArray);
+        return hash;
     }
 
     @Override