Skip to content

Commit

Permalink
Remove BlockEncodingManager's dependency on TypeManager
Browse files Browse the repository at this point in the history 
BlockEncodingManager as a low level class manager block serde should not depend
on type systems. The dependency was introduced to get method handles to build
hash tables for map block. However map block hashtables are lazily built so they
are not really needed to encode / decode the map block. Changing the code to
require functionalities needing the hashtable to pass in method handles instead
so we can remove BlockEncodingManager's dependency on TypeManager.
  • Loading branch information
@rongrong
rongrong authored and Rongrong Zhong committed Sep 29, 2020
1 parent ef5843a commit 2dfb7cd
Show file tree
Hide file tree
Showing 101 changed files with 291 additions and 538 deletions.
2 changes: 1 addition & 1 deletion presto-accumulo/src/test/java/com/facebook/presto/accumulo/model/TestField.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public void testMap()
{
TypeManager typeManager = new TypeRegistry();
// associate typeManager with a function manager
new FunctionManager(typeManager, new BlockEncodingManager(typeManager), new FeaturesConfig());
new FunctionManager(typeManager, new BlockEncodingManager(), new FeaturesConfig());

Type type = typeManager.getParameterizedType(StandardTypes.MAP, ImmutableList.of(
TypeSignatureParameter.of(VARCHAR.getTypeSignature()),
Expand Down
2 changes: 1 addition & 1 deletion ...test/java/com/facebook/presto/accumulo/serializers/AbstractTestAccumuloRowSerializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public void testMap()
{
TypeManager typeManager = new TypeRegistry();
// associate typeManager with a function manager
new FunctionManager(typeManager, new BlockEncodingManager(typeManager), new FeaturesConfig());
new FunctionManager(typeManager, new BlockEncodingManager(), new FeaturesConfig());

AccumuloRowSerializer serializer = serializerClass.getConstructor().newInstance();
Type type = typeManager.getParameterizedType(StandardTypes.MAP, ImmutableList.of(
Expand Down
48 changes: 6 additions & 42 deletions presto-common/src/main/java/com/facebook/presto/common/block/AbstractMapBlock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.facebook.presto.common.block;

import com.facebook.presto.common.type.Type;
import io.airlift.slice.SliceOutput;
import org.openjdk.jol.info.ClassLayout;

Expand Down Expand Up @@ -42,23 +41,8 @@ public abstract class AbstractMapBlock
// inverse of hash fill ratio, must be integer
static final int HASH_MULTIPLIER = 2;

protected final Type keyType;
protected final MethodHandle keyNativeHashCode;
protected final MethodHandle keyBlockNativeEquals;
protected final MethodHandle keyBlockHashCode;

protected volatile long logicalSizeInBytes;

public AbstractMapBlock(Type keyType, MethodHandle keyNativeHashCode, MethodHandle keyBlockNativeEquals, MethodHandle keyBlockHashCode)
{
this.keyType = requireNonNull(keyType, "keyType is null");
// keyNativeHashCode can only be null due to map block kill switch. deprecated.new-map-block
this.keyNativeHashCode = keyNativeHashCode;
// keyBlockNativeEquals can only be null due to map block kill switch. deprecated.new-map-block
this.keyBlockNativeEquals = keyBlockNativeEquals;
this.keyBlockHashCode = requireNonNull(keyBlockHashCode, "keyBlockHashCode is null");
}

protected abstract Block getRawKeyBlock();

protected abstract Block getRawValueBlock();
Expand All @@ -80,7 +64,7 @@ public AbstractMapBlock(Type keyType, MethodHandle keyNativeHashCode, MethodHand
@Nullable
protected abstract boolean[] getMapIsNull();

protected abstract void ensureHashTableLoaded();
protected abstract void ensureHashTableLoaded(MethodHandle keyBlockHashCode);

int getOffset(int position)
{
Expand Down Expand Up @@ -149,11 +133,7 @@ public Block copyPositions(int[] positions, int offset, int length)
newOffsets,
newKeys,
newValues,
new HashTables(Optional.ofNullable(newRawHashTables), length, newHashTableEntries),
keyType,
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
new HashTables(Optional.ofNullable(newRawHashTables), length, newHashTableEntries));
}

@Override
Expand All @@ -169,11 +149,7 @@ public Block getRegion(int position, int length)
getOffsets(),
getRawKeyBlock(),
getRawValueBlock(),
getHashTables(),
keyType,
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
getHashTables());
}

@Override
Expand Down Expand Up @@ -281,11 +257,7 @@ public Block copyRegion(int position, int length)
newOffsets,
newKeys,
newValues,
new HashTables(Optional.ofNullable(newRawHashTables), length, expectedNewHashTableEntries),
keyType,
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
new HashTables(Optional.ofNullable(newRawHashTables), length, expectedNewHashTableEntries));
}

@Override
Expand Down Expand Up @@ -355,11 +327,7 @@ public Block getSingleValueBlock(int position)
new int[] {0, valueLength},
newKeys,
newValues,
new HashTables(Optional.ofNullable(newRawHashTables), 1, expectedNewHashTableEntries),
keyType,
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
new HashTables(Optional.ofNullable(newRawHashTables), 1, expectedNewHashTableEntries));
}

@Override
Expand Down Expand Up @@ -499,11 +467,7 @@ public Block appendNull()
offsets,
getRawKeyBlock(),
getRawValueBlock(),
getHashTables(),
keyType,
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
getHashTables());
}

private void calculateLogicalSize()
Expand Down
15 changes: 3 additions & 12 deletions presto-common/src/main/java/com/facebook/presto/common/block/ColumnarMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
*/
package com.facebook.presto.common.block;

import com.facebook.presto.common.type.Type;
import org.openjdk.jol.info.ClassLayout;

import javax.annotation.Nullable;
Expand All @@ -30,7 +29,6 @@ public class ColumnarMap
private final int[] offsets;
private final Block keysBlock;
private final Block valuesBlock;
private final Type keyType;
private final int[] hashTables;
private final long retainedSizeInBytes;
private final long estimatedSerializedSizeInBytes;
Expand Down Expand Up @@ -68,7 +66,8 @@ public static ColumnarMap toColumnarMap(Block block)
offsets,
keysBlock,
valuesBlock,
mapBlock.keyType, hashTables, INSTANCE_SIZE + block.getRetainedSizeInBytes(),
hashTables,
INSTANCE_SIZE + block.getRetainedSizeInBytes(),
block.getSizeInBytes());
}

Expand Down Expand Up @@ -106,7 +105,6 @@ private static ColumnarMap toColumnarMap(DictionaryBlock dictionaryBlock)
offsets,
new DictionaryBlock(dictionaryIds.length, keysBlock, dictionaryIds),
new DictionaryBlock(dictionaryIds.length, valuesBlock, dictionaryIds),
columnarMap.keyType,
columnarMap.getHashTables(),
INSTANCE_SIZE + dictionaryBlock.getRetainedSizeInBytes() + sizeOf(offsets) + sizeOf(dictionaryIds),
// The estimated serialized size is the sum of the following
Expand Down Expand Up @@ -148,7 +146,6 @@ private static ColumnarMap toColumnarMap(RunLengthEncodedBlock rleBlock)
offsets,
new DictionaryBlock(dictionaryIds.length, keysBlock, dictionaryIds),
new DictionaryBlock(dictionaryIds.length, valuesBlock, dictionaryIds),
columnarMap.keyType,
columnarMap.getHashTables(),
INSTANCE_SIZE + rleBlock.getRetainedSizeInBytes() + sizeOf(offsets) + sizeOf(dictionaryIds),
// The estimated serialized size is the sum of the following
Expand All @@ -159,14 +156,13 @@ private static ColumnarMap toColumnarMap(RunLengthEncodedBlock rleBlock)
(Integer.BYTES + Byte.BYTES) * positionCount + (long) ((keysBlock.getSizeInBytes() / (double) keysBlock.getPositionCount() + valuesBlock.getSizeInBytes() / (double) valuesBlock.getPositionCount()) * offsets[positionCount]));
}

private ColumnarMap(Block nullCheckBlock, int offsetsOffset, int[] offsets, Block keysBlock, Block valuesBlock, Type keyType, @Nullable int[] hashTables, long retainedSizeInBytes, long estimatedSerializedSizeInBytes)
private ColumnarMap(Block nullCheckBlock, int offsetsOffset, int[] offsets, Block keysBlock, Block valuesBlock, @Nullable int[] hashTables, long retainedSizeInBytes, long estimatedSerializedSizeInBytes)
{
this.nullCheckBlock = nullCheckBlock;
this.offsetsOffset = offsetsOffset;
this.offsets = offsets;
this.keysBlock = keysBlock;
this.valuesBlock = valuesBlock;
this.keyType = keyType;
this.hashTables = hashTables;
this.retainedSizeInBytes = retainedSizeInBytes;
this.estimatedSerializedSizeInBytes = estimatedSerializedSizeInBytes;
Expand Down Expand Up @@ -212,11 +208,6 @@ public Block getNullCheckBlock()
return nullCheckBlock;
}

public Type getKeyType()
{
return keyType;
}

@Nullable
public int[] getHashTables()
{
Expand Down
61 changes: 10 additions & 51 deletions presto-common/src/main/java/com/facebook/presto/common/block/MapBlock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

package com.facebook.presto.common.block;

import com.facebook.presto.common.type.MapType;
import com.facebook.presto.common.type.Type;
import org.openjdk.jol.info.ClassLayout;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -53,22 +51,15 @@ public class MapBlock
* Create a map block directly from columnar nulls, keys, values, and offsets into the keys and values.
* A null map must have no entries.
*
* @param mapType key type K
* @param keyBlockNativeEquals equality between key stack type and a block+position; signature is (K, Block, int)boolean
* @param keyNativeHashCode hash of a key stack type; signature is (K)long
*/
public static MapBlock fromKeyValueBlock(
int positionCount,
Optional<boolean[]> mapIsNull,
int[] offsets,
Block keyBlock,
Block valueBlock,
MapType mapType,
MethodHandle keyBlockNativeEquals,
MethodHandle keyNativeHashCode,
MethodHandle keyBlockHashCode)
Block valueBlock)
{
validateConstructorArguments(0, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, mapType.getKeyType(), keyBlockNativeEquals, keyNativeHashCode);
validateConstructorArguments(0, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock);

return createMapBlockInternal(
0,
Expand All @@ -77,21 +68,14 @@ public static MapBlock fromKeyValueBlock(
offsets,
keyBlock,
valueBlock,
new HashTables(Optional.empty(), positionCount, keyBlock.getPositionCount() * HASH_MULTIPLIER),
mapType.getKeyType(),
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
new HashTables(Optional.empty(), positionCount, keyBlock.getPositionCount() * HASH_MULTIPLIER));
}

/**
* Create a map block directly without per element validations.
* <p>
* Internal use by this package and com.facebook.presto.spi.Type only.
*
* @param keyType key type K
* @param keyBlockNativeEquals equality between key stack type and a block+position; signature is (K, Block, int)boolean
* @param keyNativeHashCode hash of a key stack type; signature is (K)long
*/
public static MapBlock createMapBlockInternal(
int startOffset,
Expand All @@ -100,13 +84,9 @@ public static MapBlock createMapBlockInternal(
int[] offsets,
Block keyBlock,
Block valueBlock,
HashTables hashTables,
Type keyType,
MethodHandle keyBlockNativeEquals,
MethodHandle keyNativeHashCode,
MethodHandle keyBlockHashCode)
HashTables hashTables)
{
validateConstructorArguments(startOffset, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, keyType, keyBlockNativeEquals, keyNativeHashCode);
validateConstructorArguments(startOffset, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock);
requireNonNull(hashTables, "hashTables is null");
return new MapBlock(
startOffset,
Expand All @@ -115,11 +95,7 @@ public static MapBlock createMapBlockInternal(
offsets,
keyBlock,
valueBlock,
hashTables,
keyType,
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
hashTables);
}

private static void validateConstructorArguments(
Expand All @@ -128,10 +104,7 @@ private static void validateConstructorArguments(
@Nullable boolean[] mapIsNull,
int[] offsets,
Block keyBlock,
Block valueBlock,
Type keyType,
MethodHandle keyBlockNativeEquals,
MethodHandle keyNativeHashCode)
Block valueBlock)
{
if (startOffset < 0) {
throw new IllegalArgumentException("startOffset is negative");
Expand All @@ -155,10 +128,6 @@ private static void validateConstructorArguments(
if (keyBlock.getPositionCount() != valueBlock.getPositionCount()) {
throw new IllegalArgumentException(format("keyBlock and valueBlock has different size: %s %s", keyBlock.getPositionCount(), valueBlock.getPositionCount()));
}

requireNonNull(keyType, "keyType is null");
requireNonNull(keyBlockNativeEquals, "keyBlockNativeEquals is null");
requireNonNull(keyNativeHashCode, "keyNativeHashCode is null");
}

/**
Expand All @@ -172,14 +141,8 @@ private MapBlock(
int[] offsets,
Block keyBlock,
Block valueBlock,
HashTables hashTables,
Type keyType,
MethodHandle keyBlockNativeEquals,
MethodHandle keyNativeHashCode,
MethodHandle keyBlockHashCode)
HashTables hashTables)
{
super(keyType, keyNativeHashCode, keyBlockNativeEquals, keyBlockHashCode);

int[] rawHashTables = hashTables.get();
if (rawHashTables != null && rawHashTables.length < keyBlock.getPositionCount() * HASH_MULTIPLIER) {
throw new IllegalArgumentException(format("keyBlock/valueBlock size does not match hash table size: %s %s", keyBlock.getPositionCount(), rawHashTables.length));
Expand Down Expand Up @@ -315,15 +278,11 @@ public Block getLoadedBlock()
offsets,
keyBlock,
loadedValueBlock,
hashTables,
keyType,
keyBlockNativeEquals,
keyNativeHashCode,
keyBlockHashCode);
hashTables);
}

@Override
protected void ensureHashTableLoaded()
protected void ensureHashTableLoaded(MethodHandle keyBlockHashCode)
{
if (isHashTablesPresent()) {
return;
Expand Down
Loading

0 comments on commit 2dfb7cd

Please sign in to comment.