diff --git a/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java b/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java index da97247b12345..87239562767ca 100644 --- a/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java +++ b/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java @@ -27,7 +27,6 @@ import org.testng.annotations.Test; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -81,34 +80,6 @@ public void testCompactBlock() testIncompactBlock(mapType(TINYINT, TINYINT).createBlockFromKeyValue(Optional.of(mapIsNull), offsets, inCompactKeyBlock, inCompactValueBlock)); } - // TODO: remove this test when we have a more unified testWith() using assertBlock() - @Test - public void testLazyHashTableBuildOverBlockRegion() - { - Map[] values = createTestMap(9, 3, 4, 0, 8, 0, 6, 5); - Block block = createBlockWithValuesFromKeyValueBlock(values); - BlockBuilder blockBuilder = createBlockBuilderWithValues(values); - - // Create a MapBlock that is a region of another MapBlock. It doesn't have hashtables built at the time of creation. - int offset = block.getPositionCount() / 2; - Block blockRegion = block.getRegion(offset, block.getPositionCount() - offset); - - // Lazily build the hashtables for the block region and use them to do position/value check. - Map[] expectedValues = Arrays.copyOfRange(values, values.length / 2, values.length); - assertBlock(blockRegion, () -> blockBuilder.newBlockBuilderLike(null), expectedValues); - - Map[] valuesWithNull = alternatingNullValues(values); - Block blockWithNull = createBlockWithValuesFromKeyValueBlock(valuesWithNull); - - // Create a MapBlock that is a region of another MapBlock with null values. It doesn't have hashtables built at the time of creation. - offset = blockWithNull.getPositionCount() / 2; - Block blockRegionWithNull = blockWithNull.getRegion(offset, blockWithNull.getPositionCount() - offset); - - // Lazily build the hashtables for the block region and use them to do position/value check. - Map[] expectedValuesWithNull = Arrays.copyOfRange(valuesWithNull, valuesWithNull.length / 2, valuesWithNull.length); - assertBlock(blockRegionWithNull, () -> blockBuilder.newBlockBuilderLike(null), expectedValuesWithNull); - } - private Map[] createTestMap(int... entryCounts) { Map[] result = new Map[entryCounts.length]; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java index 714c038d50ee9..8b86570047f0b 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java @@ -39,23 +39,20 @@ public abstract class AbstractMapBlock protected final Type keyType; protected final MethodHandle keyNativeHashCode; protected final MethodHandle keyBlockNativeEquals; - protected final MethodHandle keyBlockHashCode; - public AbstractMapBlock(Type keyType, MethodHandle keyNativeHashCode, MethodHandle keyBlockNativeEquals, MethodHandle keyBlockHashCode) + public AbstractMapBlock(Type keyType, MethodHandle keyNativeHashCode, MethodHandle keyBlockNativeEquals) { 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(); - @Nullable protected abstract int[] getHashTables(); /** @@ -73,8 +70,6 @@ public AbstractMapBlock(Type keyType, MethodHandle keyNativeHashCode, MethodHand @Nullable protected abstract boolean[] getMapIsNull(); - protected abstract void ensureHashTableLoaded(); - int getOffset(int position) { return getOffsets()[position + getOffsetBase()]; @@ -117,35 +112,21 @@ public Block copyPositions(int[] positions, int offset, int length) } int[] hashTable = getHashTables(); - int[] newHashTable = null; - if (hashTable != null) { - newHashTable = new int[newOffsets[newOffsets.length - 1] * HASH_MULTIPLIER]; - int newHashIndex = 0; - for (int i = offset; i < offset + length; ++i) { - int position = positions[i]; - int entriesStartOffset = getOffset(position); - int entriesEndOffset = getOffset(position + 1); - for (int hashIndex = entriesStartOffset * HASH_MULTIPLIER; hashIndex < entriesEndOffset * HASH_MULTIPLIER; hashIndex++) { - newHashTable[newHashIndex] = hashTable[hashIndex]; - newHashIndex++; - } + int[] newHashTable = new int[newOffsets[newOffsets.length - 1] * HASH_MULTIPLIER]; + int newHashIndex = 0; + for (int i = offset; i < offset + length; ++i) { + int position = positions[i]; + int entriesStartOffset = getOffset(position); + int entriesEndOffset = getOffset(position + 1); + for (int hashIndex = entriesStartOffset * HASH_MULTIPLIER; hashIndex < entriesEndOffset * HASH_MULTIPLIER; hashIndex++) { + newHashTable[newHashIndex] = hashTable[hashIndex]; + newHashIndex++; } } Block newKeys = getRawKeyBlock().copyPositions(entriesPositions.elements(), 0, entriesPositions.size()); Block newValues = getRawValueBlock().copyPositions(entriesPositions.elements(), 0, entriesPositions.size()); - return createMapBlockInternal( - 0, - length, - Optional.of(newMapIsNull), - newOffsets, - newKeys, - newValues, - Optional.ofNullable(newHashTable), - keyType, - keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); + return createMapBlockInternal(0, length, Optional.of(newMapIsNull), newOffsets, newKeys, newValues, newHashTable, keyType, keyBlockNativeEquals, keyNativeHashCode); } @Override @@ -161,11 +142,10 @@ public Block getRegion(int position, int length) getOffsets(), getRawKeyBlock(), getRawValueBlock(), - Optional.ofNullable(getHashTables()), + getHashTables(), keyType, keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); + keyNativeHashCode); } @Override @@ -227,12 +207,7 @@ public Block copyRegion(int position, int length) int[] newOffsets = compactOffsets(getOffsets(), position + getOffsetBase(), length); boolean[] mapIsNull = getMapIsNull(); boolean[] newMapIsNull = mapIsNull == null ? null : compactArray(mapIsNull, position + getOffsetBase(), length); - - int[] hashTables = getHashTables(); - int[] newHashTable = null; - if (hashTables != null) { - newHashTable = compactArray(hashTables, startValueOffset * HASH_MULTIPLIER, (endValueOffset - startValueOffset) * HASH_MULTIPLIER); - } + int[] newHashTable = compactArray(getHashTables(), startValueOffset * HASH_MULTIPLIER, (endValueOffset - startValueOffset) * HASH_MULTIPLIER); if (newKeys == getRawKeyBlock() && newValues == getRawValueBlock() && newOffsets == getOffsets() && newMapIsNull == mapIsNull && newHashTable == getHashTables()) { return this; @@ -244,11 +219,10 @@ public Block copyRegion(int position, int length) newOffsets, newKeys, newValues, - Optional.ofNullable(newHashTable), + newHashTable, keyType, keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); + keyNativeHashCode); } @Override @@ -264,7 +238,12 @@ public T getObject(int position, Class clazz) return clazz.cast(new SingleMapBlock( startEntryOffset * 2, (endEntryOffset - startEntryOffset) * 2, - this)); + getRawKeyBlock(), + getRawValueBlock(), + getHashTables(), + keyType, + keyNativeHashCode, + keyBlockNativeEquals)); } @Override @@ -284,12 +263,7 @@ public Block getSingleValueBlock(int position) int valueLength = endValueOffset - startValueOffset; Block newKeys = getRawKeyBlock().copyRegion(startValueOffset, valueLength); Block newValues = getRawValueBlock().copyRegion(startValueOffset, valueLength); - - int[] hashTables = getHashTables(); - int[] newHashTable = null; - if (hashTables != null) { - newHashTable = Arrays.copyOfRange(hashTables, startValueOffset * HASH_MULTIPLIER, endValueOffset * HASH_MULTIPLIER); - } + int[] newHashTable = Arrays.copyOfRange(getHashTables(), startValueOffset * HASH_MULTIPLIER, endValueOffset * HASH_MULTIPLIER); return createMapBlockInternal( 0, @@ -298,11 +272,10 @@ public Block getSingleValueBlock(int position) new int[] {0, valueLength}, newKeys, newValues, - Optional.ofNullable(newHashTable), + newHashTable, keyType, keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); + keyNativeHashCode); } @Override diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java index f8981f6b603b8..0a51354f1b46c 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java @@ -27,7 +27,6 @@ import static com.facebook.presto.spi.block.MapBlockBuilder.buildHashTable; import static io.airlift.slice.SizeOf.sizeOf; -import static io.airlift.slice.SizeOf.sizeOfIntArray; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -43,7 +42,7 @@ public class MapBlock private final int[] offsets; private final Block keyBlock; private final Block valueBlock; - private volatile int[] hashTables; // hash to location in map. Writes to the field is protected by "this" monitor. + private final int[] hashTables; // hash to location in map; private volatile long sizeInBytes; private final long retainedSizeInBytes; @@ -69,6 +68,20 @@ public static MapBlock fromKeyValueBlock( validateConstructorArguments(0, offsets.length - 1, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, mapType.getKeyType(), keyBlockNativeEquals, keyNativeHashCode); int mapCount = offsets.length - 1; + int elementCount = keyBlock.getPositionCount(); + int[] hashTables = new int[elementCount * HASH_MULTIPLIER]; + Arrays.fill(hashTables, -1); + for (int i = 0; i < mapCount; i++) { + int keyOffset = offsets[i]; + int keyCount = offsets[i + 1] - keyOffset; + if (keyCount < 0) { + throw new IllegalArgumentException(format("Offset is not monotonically ascending. offsets[%s]=%s, offsets[%s]=%s", i, offsets[i], i + 1, offsets[i + 1])); + } + if (mapIsNull.isPresent() && mapIsNull.get()[i] && keyCount != 0) { + throw new IllegalArgumentException("A null map must have zero entries"); + } + buildHashTable(keyBlock, keyOffset, keyCount, keyBlockHashCode, hashTables, keyOffset * HASH_MULTIPLIER, keyCount * HASH_MULTIPLIER); + } return createMapBlockInternal( 0, @@ -77,11 +90,10 @@ public static MapBlock fromKeyValueBlock( offsets, keyBlock, valueBlock, - Optional.empty(), + hashTables, mapType.getKeyType(), keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); + keyNativeHashCode); } /** @@ -100,25 +112,13 @@ public static MapBlock createMapBlockInternal( int[] offsets, Block keyBlock, Block valueBlock, - Optional hashTables, + int[] hashTables, Type keyType, MethodHandle keyBlockNativeEquals, - MethodHandle keyNativeHashCode, - MethodHandle keyBlockHashCode) + MethodHandle keyNativeHashCode) { validateConstructorArguments(startOffset, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, keyType, keyBlockNativeEquals, keyNativeHashCode); - return new MapBlock( - startOffset, - positionCount, - mapIsNull.orElse(null), - offsets, - keyBlock, - valueBlock, - hashTables.orElse(null), - keyType, - keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); + return new MapBlock(startOffset, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, hashTables, keyType, keyBlockNativeEquals, keyNativeHashCode); } private static void validateConstructorArguments( @@ -171,15 +171,15 @@ private MapBlock( int[] offsets, Block keyBlock, Block valueBlock, - @Nullable int[] hashTables, + int[] hashTables, Type keyType, MethodHandle keyBlockNativeEquals, - MethodHandle keyNativeHashCode, - MethodHandle keyBlockHashCode) + MethodHandle keyNativeHashCode) { - super(keyType, keyNativeHashCode, keyBlockNativeEquals, keyBlockHashCode); + super(keyType, keyNativeHashCode, keyBlockNativeEquals); - if (hashTables != null && hashTables.length < keyBlock.getPositionCount() * HASH_MULTIPLIER) { + requireNonNull(hashTables, "hashTables is null"); + if (hashTables.length < keyBlock.getPositionCount() * HASH_MULTIPLIER) { throw new IllegalArgumentException(format("keyBlock/valueBlock size does not match hash table size: %s %s", keyBlock.getPositionCount(), hashTables.length)); } @@ -192,16 +192,7 @@ private MapBlock( this.hashTables = hashTables; this.sizeInBytes = -1; - - // We will add the hashtable size to the retained size even if it's not built yet. This could be overestimating - // but is necessary to avoid reliability issues. Currently the memory counting framework only pull the retained - // size once for each operator so updating in the middle of the processing would not work. - this.retainedSizeInBytes = INSTANCE_SIZE - + keyBlock.getRetainedSizeInBytes() - + valueBlock.getRetainedSizeInBytes() - + sizeOf(offsets) - + sizeOf(mapIsNull) - + sizeOfIntArray(keyBlock.getPositionCount() * HASH_MULTIPLIER); // hashtable size if it was built + this.retainedSizeInBytes = INSTANCE_SIZE + keyBlock.getRetainedSizeInBytes() + valueBlock.getRetainedSizeInBytes() + sizeOf(offsets) + sizeOf(mapIsNull) + sizeOf(hashTables); } @Override @@ -312,47 +303,9 @@ public Block getLoadedBlock() offsets, keyBlock, loadedValueBlock, - Optional.ofNullable(hashTables), + hashTables, keyType, keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); - } - - @Override - protected void ensureHashTableLoaded() - { - if (this.hashTables != null) { - return; - } - - // This can only happen for MapBlock, not MapBlockBuilder because the latter always has non-null hashtables - synchronized (this) { - if (this.hashTables != null) { - return; - } - - int[] hashTables = new int[getRawKeyBlock().getPositionCount() * HASH_MULTIPLIER]; - Arrays.fill(hashTables, -1); - for (int i = 0; i < offsets.length - 1; i++) { - int keyOffset = offsets[i]; - int keyCount = offsets[i + 1] - keyOffset; - if (keyCount < 0) { - throw new IllegalArgumentException(format("Offset is not monotonically ascending. offsets[%s]=%s, offsets[%s]=%s", i, offsets[i], i + 1, offsets[i + 1])); - } - if (mapIsNull != null && mapIsNull[i] && keyCount != 0) { - throw new IllegalArgumentException("A null map must have zero entries"); - } - buildHashTable( - getRawKeyBlock(), - keyOffset, - keyCount, - keyBlockHashCode, - hashTables, - keyOffset * HASH_MULTIPLIER, - keyCount * HASH_MULTIPLIER); - } - this.hashTables = hashTables; - } + keyNativeHashCode); } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java index d7f3e6e979c3a..d4b41cea4d6e9 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java @@ -39,6 +39,7 @@ public class MapBlockBuilder private static final int INSTANCE_SIZE = ClassLayout.parseClass(MapBlockBuilder.class).instanceSize(); private final MethodHandle keyBlockEquals; + private final MethodHandle keyBlockHashCode; @Nullable private final BlockBuilderStatus blockBuilderStatus; @@ -89,9 +90,10 @@ private MapBlockBuilder( boolean[] mapIsNull, int[] hashTables) { - super(keyType, keyNativeHashCode, keyBlockNativeEquals, keyBlockHashCode); + super(keyType, keyNativeHashCode, keyBlockNativeEquals); this.keyBlockEquals = requireNonNull(keyBlockEquals, "keyBlockEquals is null"); + this.keyBlockHashCode = requireNonNull(keyBlockHashCode, "keyBlockHashCode is null"); this.blockBuilderStatus = blockBuilderStatus; this.positionCount = 0; @@ -155,12 +157,7 @@ public long getSizeInBytes() @Override public long getRetainedSizeInBytes() { - long size = INSTANCE_SIZE - + keyBlockBuilder.getRetainedSizeInBytes() - + valueBlockBuilder.getRetainedSizeInBytes() - + sizeOf(offsets) - + sizeOf(mapIsNull) - + sizeOf(hashTables); + long size = INSTANCE_SIZE + keyBlockBuilder.getRetainedSizeInBytes() + valueBlockBuilder.getRetainedSizeInBytes() + sizeOf(offsets) + sizeOf(mapIsNull) + sizeOf(hashTables); if (blockBuilderStatus != null) { size += BlockBuilderStatus.INSTANCE_SIZE; } @@ -202,14 +199,7 @@ public BlockBuilder closeEntry() int previousAggregatedEntryCount = offsets[positionCount - 1]; int aggregatedEntryCount = offsets[positionCount]; int entryCount = aggregatedEntryCount - previousAggregatedEntryCount; - buildHashTable( - keyBlockBuilder, - previousAggregatedEntryCount, - entryCount, - keyBlockHashCode, - hashTables, - previousAggregatedEntryCount * HASH_MULTIPLIER, - entryCount * HASH_MULTIPLIER); + buildHashTable(keyBlockBuilder, previousAggregatedEntryCount, entryCount, keyBlockHashCode, hashTables, previousAggregatedEntryCount * HASH_MULTIPLIER, entryCount * HASH_MULTIPLIER); return this; } @@ -246,7 +236,7 @@ public BlockBuilder closeEntryStrict() return this; } - private BlockBuilder closeEntry(@Nullable int[] providedHashTable, int providedHashTableOffset) + private BlockBuilder closeEntry(int[] providedHashTable, int providedHashTableOffset) { if (!currentEntryOpened) { throw new IllegalStateException("Expected entry to be opened but was closed"); @@ -256,29 +246,14 @@ private BlockBuilder closeEntry(@Nullable int[] providedHashTable, int providedH currentEntryOpened = false; ensureHashTableSize(); - int previousAggregatedEntryCount = offsets[positionCount - 1]; - int aggregatedEntryCount = offsets[positionCount]; - if (providedHashTable != null) { - // Directly copy instead of building hashtable if providedHashTable is not null - int hashTableOffset = previousAggregatedEntryCount * HASH_MULTIPLIER; - int hashTableSize = (aggregatedEntryCount - previousAggregatedEntryCount) * HASH_MULTIPLIER; - for (int i = 0; i < hashTableSize; i++) { - hashTables[hashTableOffset + i] = providedHashTable[providedHashTableOffset + i]; - } - } - else { - // Build hash table for this map entry. - int entryCount = aggregatedEntryCount - previousAggregatedEntryCount; - buildHashTable( - keyBlockBuilder, - previousAggregatedEntryCount, - entryCount, - keyBlockHashCode, - hashTables, - previousAggregatedEntryCount * HASH_MULTIPLIER, - entryCount * HASH_MULTIPLIER); + // Directly copy instead of building hashtable + int hashTableOffset = offsets[positionCount - 1] * HASH_MULTIPLIER; + int hashTableSize = (offsets[positionCount] - offsets[positionCount - 1]) * HASH_MULTIPLIER; + for (int i = 0; i < hashTableSize; i++) { + hashTables[hashTableOffset + i] = providedHashTable[providedHashTableOffset + i]; } + return this; } @@ -336,11 +311,9 @@ public Block build() offsets, keyBlockBuilder.build(), valueBlockBuilder.build(), - Optional.of(Arrays.copyOf(hashTables, offsets[positionCount] * HASH_MULTIPLIER)), + Arrays.copyOf(hashTables, offsets[positionCount] * HASH_MULTIPLIER), keyType, - keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); + keyBlockNativeEquals, keyNativeHashCode); } @Override @@ -437,9 +410,6 @@ public BlockBuilder newBlockBuilderLike(BlockBuilderStatus blockBuilderStatus) newNegativeOneFilledArray(newSize * HASH_MULTIPLIER)); } - @Override - protected void ensureHashTableLoaded() {} - private static int[] newNegativeOneFilledArray(int size) { int[] hashTable = new int[size]; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java index d03904f5af3ba..cc0b43537b653 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java @@ -64,15 +64,8 @@ public void writeBlock(BlockEncodingSerde blockEncodingSerde, SliceOutput sliceO blockEncodingSerde.writeBlock(sliceOutput, mapBlock.getRawKeyBlock().getRegion(entriesStartOffset, entriesEndOffset - entriesStartOffset)); blockEncodingSerde.writeBlock(sliceOutput, mapBlock.getRawValueBlock().getRegion(entriesStartOffset, entriesEndOffset - entriesStartOffset)); - if (hashTable != null) { - int hashTableLength = (entriesEndOffset - entriesStartOffset) * HASH_MULTIPLIER; - sliceOutput.appendInt(hashTableLength); // hashtable length - sliceOutput.writeBytes(wrappedIntArray(hashTable, entriesStartOffset * HASH_MULTIPLIER, hashTableLength)); - } - else { - // if the hashTable is null, we write the length -1 - sliceOutput.appendInt(-1); // hashtable length - } + sliceOutput.appendInt((entriesEndOffset - entriesStartOffset) * HASH_MULTIPLIER); + sliceOutput.writeBytes(wrappedIntArray(hashTable, entriesStartOffset * HASH_MULTIPLIER, (entriesEndOffset - entriesStartOffset) * HASH_MULTIPLIER)); sliceOutput.appendInt(positionCount); for (int position = 0; position < positionCount + 1; position++) { @@ -89,27 +82,18 @@ public Block readBlock(BlockEncodingSerde blockEncodingSerde, SliceInput sliceIn Block keyBlock = blockEncodingSerde.readBlock(sliceInput); Block valueBlock = blockEncodingSerde.readBlock(sliceInput); - int hashTableLength = sliceInput.readInt(); - int[] hashTable = null; - if (hashTableLength >= 0) { - hashTable = new int[hashTableLength]; - sliceInput.readBytes(wrappedIntArray(hashTable)); - } - - if (keyBlock.getPositionCount() != valueBlock.getPositionCount()) { - throw new IllegalArgumentException( - format("Deserialized MapBlock violates invariants: key %d, value %d", keyBlock.getPositionCount(), valueBlock.getPositionCount())); - } + int[] hashTable = new int[sliceInput.readInt()]; + sliceInput.readBytes(wrappedIntArray(hashTable)); - if (hashTable != null && keyBlock.getPositionCount() * HASH_MULTIPLIER != hashTable.length) { + if (keyBlock.getPositionCount() != valueBlock.getPositionCount() || keyBlock.getPositionCount() * HASH_MULTIPLIER != hashTable.length) { throw new IllegalArgumentException( - format("Deserialized MapBlock violates invariants: expected hashtable size %d, actual hashtable size %d", keyBlock.getPositionCount() * HASH_MULTIPLIER, hashTable.length)); + format("Deserialized MapBlock violates invariants: key %d, value %d, hash %d", keyBlock.getPositionCount(), valueBlock.getPositionCount(), hashTable.length)); } int positionCount = sliceInput.readInt(); int[] offsets = new int[positionCount + 1]; sliceInput.readBytes(wrappedIntArray(offsets)); Optional mapIsNull = EncoderUtil.decodeNullBits(sliceInput, positionCount); - return MapType.createMapBlockInternal(typeManager, keyType, 0, positionCount, mapIsNull, offsets, keyBlock, valueBlock, Optional.ofNullable(hashTable)); + return MapType.createMapBlockInternal(typeManager, keyType, 0, positionCount, mapIsNull, offsets, keyBlock, valueBlock, hashTable); } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlock.java index 9b7a2666d75f9..6abae88e01329 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlock.java @@ -19,6 +19,7 @@ import io.airlift.slice.Slice; import org.openjdk.jol.info.ClassLayout; +import java.lang.invoke.MethodHandle; import java.util.function.BiConsumer; import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR; @@ -35,14 +36,24 @@ public class SingleMapBlock private static final int INSTANCE_SIZE = ClassLayout.parseClass(SingleMapBlock.class).instanceSize(); private final int offset; - private final int positionCount; // The number of keys in this single map * 2 - private final AbstractMapBlock mapBlock; - - SingleMapBlock(int offset, int positionCount, AbstractMapBlock mapBlock) + private final int positionCount; + private final Block keyBlock; + private final Block valueBlock; + private final int[] hashTable; + final Type keyType; + private final MethodHandle keyNativeHashCode; + private final MethodHandle keyBlockNativeEquals; + + SingleMapBlock(int offset, int positionCount, Block keyBlock, Block valueBlock, int[] hashTable, Type keyType, MethodHandle keyNativeHashCode, MethodHandle keyBlockNativeEquals) { this.offset = offset; this.positionCount = positionCount; - this.mapBlock = mapBlock; + this.keyBlock = keyBlock; + this.valueBlock = valueBlock; + this.hashTable = hashTable; + this.keyType = keyType; + this.keyNativeHashCode = keyNativeHashCode; + this.keyBlockNativeEquals = keyBlockNativeEquals; } @Override @@ -54,23 +65,23 @@ public int getPositionCount() @Override public long getSizeInBytes() { - return mapBlock.getRawKeyBlock().getRegionSizeInBytes(offset / 2, positionCount / 2) + - mapBlock.getRawValueBlock().getRegionSizeInBytes(offset / 2, positionCount / 2) + + return keyBlock.getRegionSizeInBytes(offset / 2, positionCount / 2) + + valueBlock.getRegionSizeInBytes(offset / 2, positionCount / 2) + sizeOfIntArray(positionCount / 2 * HASH_MULTIPLIER); } @Override public long getRetainedSizeInBytes() { - return INSTANCE_SIZE + mapBlock.getRawKeyBlock().getRetainedSizeInBytes() + mapBlock.getRawValueBlock().getRetainedSizeInBytes() + sizeOf(mapBlock.getHashTables()); + return INSTANCE_SIZE + keyBlock.getRetainedSizeInBytes() + valueBlock.getRetainedSizeInBytes() + sizeOf(hashTable); } @Override public void retainedBytesForEachPart(BiConsumer consumer) { - consumer.accept(mapBlock.getRawKeyBlock(), mapBlock.getRawKeyBlock().getRetainedSizeInBytes()); - consumer.accept(mapBlock.getRawValueBlock(), mapBlock.getRawValueBlock().getRetainedSizeInBytes()); - consumer.accept(mapBlock.getHashTables(), sizeOf(mapBlock.getHashTables())); + consumer.accept(keyBlock, keyBlock.getRetainedSizeInBytes()); + consumer.accept(valueBlock, valueBlock.getRetainedSizeInBytes()); + consumer.accept(hashTable, sizeOf(hashTable)); consumer.accept(this, (long) INSTANCE_SIZE); } @@ -89,13 +100,13 @@ public int getOffset() @Override Block getRawKeyBlock() { - return mapBlock.getRawKeyBlock(); + return keyBlock; } @Override Block getRawValueBlock() { - return mapBlock.getRawValueBlock(); + return valueBlock; } @Override @@ -107,29 +118,29 @@ public String toString() @Override public Block getLoadedBlock() { - if (mapBlock.getRawKeyBlock() != mapBlock.getRawKeyBlock().getLoadedBlock()) { + if (keyBlock != keyBlock.getLoadedBlock()) { // keyBlock has to be loaded since MapBlock constructs hash table eagerly. throw new IllegalStateException(); } - Block loadedValueBlock = mapBlock.getRawValueBlock().getLoadedBlock(); - if (loadedValueBlock == mapBlock.getRawValueBlock()) { + Block loadedValueBlock = valueBlock.getLoadedBlock(); + if (loadedValueBlock == valueBlock) { return this; } return new SingleMapBlock( offset, positionCount, - mapBlock); + keyBlock, + loadedValueBlock, + hashTable, + keyType, + keyNativeHashCode, + keyBlockNativeEquals); } int[] getHashTable() { - return mapBlock.getHashTables(); - } - - Type getKeyType() - { - return mapBlock.keyType; + return hashTable; } /** @@ -141,12 +152,9 @@ public int seekKey(Object nativeValue) return -1; } - mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); - long hashCode; try { - hashCode = (long) mapBlock.keyNativeHashCode.invoke(nativeValue); + hashCode = (long) keyNativeHashCode.invoke(nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -163,7 +171,7 @@ public int seekKey(Object nativeValue) Boolean match; try { // assuming maps with indeterminate keys are not supported - match = (Boolean) mapBlock.keyBlockNativeEquals.invoke(mapBlock.getRawKeyBlock(), offset / 2 + keyPosition, nativeValue); + match = (Boolean) keyBlockNativeEquals.invoke(keyBlock, offset / 2 + keyPosition, nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -188,12 +196,9 @@ public int seekKeyExact(long nativeValue) return -1; } - mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); - long hashCode; try { - hashCode = (long) mapBlock.keyNativeHashCode.invokeExact(nativeValue); + hashCode = (long) keyNativeHashCode.invokeExact(nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -210,7 +215,7 @@ public int seekKeyExact(long nativeValue) Boolean match; try { // assuming maps with indeterminate keys are not supported - match = (Boolean) mapBlock.keyBlockNativeEquals.invokeExact(mapBlock.getRawKeyBlock(), offset / 2 + keyPosition, nativeValue); + match = (Boolean) keyBlockNativeEquals.invokeExact(keyBlock, offset / 2 + keyPosition, nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -232,12 +237,9 @@ public int seekKeyExact(boolean nativeValue) return -1; } - mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); - long hashCode; try { - hashCode = (long) mapBlock.keyNativeHashCode.invokeExact(nativeValue); + hashCode = (long) keyNativeHashCode.invokeExact(nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -254,7 +256,7 @@ public int seekKeyExact(boolean nativeValue) Boolean match; try { // assuming maps with indeterminate keys are not supported - match = (Boolean) mapBlock.keyBlockNativeEquals.invokeExact(mapBlock.getRawKeyBlock(), offset / 2 + keyPosition, nativeValue); + match = (Boolean) keyBlockNativeEquals.invokeExact(keyBlock, offset / 2 + keyPosition, nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -276,12 +278,9 @@ public int seekKeyExact(double nativeValue) return -1; } - mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); - long hashCode; try { - hashCode = (long) mapBlock.keyNativeHashCode.invokeExact(nativeValue); + hashCode = (long) keyNativeHashCode.invokeExact(nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -298,7 +297,7 @@ public int seekKeyExact(double nativeValue) Boolean match; try { // assuming maps with indeterminate keys are not supported - match = (Boolean) mapBlock.keyBlockNativeEquals.invokeExact(mapBlock.getRawKeyBlock(), offset / 2 + keyPosition, nativeValue); + match = (Boolean) keyBlockNativeEquals.invokeExact(keyBlock, offset / 2 + keyPosition, nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -320,12 +319,9 @@ public int seekKeyExact(Slice nativeValue) return -1; } - mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); - long hashCode; try { - hashCode = (long) mapBlock.keyNativeHashCode.invokeExact(nativeValue); + hashCode = (long) keyNativeHashCode.invokeExact(nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -342,7 +338,7 @@ public int seekKeyExact(Slice nativeValue) Boolean match; try { // assuming maps with indeterminate keys are not supported - match = (Boolean) mapBlock.keyBlockNativeEquals.invokeExact(mapBlock.getRawKeyBlock(), offset / 2 + keyPosition, nativeValue); + match = (Boolean) keyBlockNativeEquals.invokeExact(keyBlock, offset / 2 + keyPosition, nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -364,12 +360,9 @@ public int seekKeyExact(Block nativeValue) return -1; } - mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); - long hashCode; try { - hashCode = (long) mapBlock.keyNativeHashCode.invokeExact(nativeValue); + hashCode = (long) keyNativeHashCode.invokeExact(nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); @@ -386,7 +379,7 @@ public int seekKeyExact(Block nativeValue) Boolean match; try { // assuming maps with indeterminate keys are not supported - match = (Boolean) mapBlock.keyBlockNativeEquals.invokeExact(mapBlock.getRawKeyBlock(), offset / 2 + keyPosition, nativeValue); + match = (Boolean) keyBlockNativeEquals.invokeExact(keyBlock, offset / 2 + keyPosition, nativeValue); } catch (Throwable throwable) { throw handleThrowable(throwable); diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlockEncoding.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlockEncoding.java index 6eee30661e0fb..0b28dd4b728f3 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlockEncoding.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/SingleMapBlockEncoding.java @@ -22,7 +22,6 @@ import io.airlift.slice.SliceOutput; import java.lang.invoke.MethodHandle; -import java.util.Optional; import static com.facebook.presto.spi.block.AbstractMapBlock.HASH_MULTIPLIER; import static com.facebook.presto.spi.block.MethodHandleUtil.compose; @@ -55,23 +54,15 @@ public String getName() public void writeBlock(BlockEncodingSerde blockEncodingSerde, SliceOutput sliceOutput, Block block) { SingleMapBlock singleMapBlock = (SingleMapBlock) block; - TypeSerde.writeType(sliceOutput, singleMapBlock.getKeyType()); + TypeSerde.writeType(sliceOutput, singleMapBlock.keyType); int offset = singleMapBlock.getOffset(); int positionCount = singleMapBlock.getPositionCount(); blockEncodingSerde.writeBlock(sliceOutput, singleMapBlock.getRawKeyBlock().getRegion(offset / 2, positionCount / 2)); blockEncodingSerde.writeBlock(sliceOutput, singleMapBlock.getRawValueBlock().getRegion(offset / 2, positionCount / 2)); int[] hashTable = singleMapBlock.getHashTable(); - - if (hashTable != null) { - int hashTableLength = positionCount / 2 * HASH_MULTIPLIER; - sliceOutput.appendInt(hashTableLength); // hashtable length - sliceOutput.writeBytes(wrappedIntArray(hashTable, offset / 2 * HASH_MULTIPLIER, hashTableLength)); - } - else { - // if the hashTable is null, we write the length -1 - sliceOutput.appendInt(-1); - } + sliceOutput.appendInt(positionCount / 2 * HASH_MULTIPLIER); + sliceOutput.writeBytes(wrappedIntArray(hashTable, offset / 2 * HASH_MULTIPLIER, positionCount / 2 * HASH_MULTIPLIER)); } @Override @@ -81,41 +72,18 @@ public Block readBlock(BlockEncodingSerde blockEncodingSerde, SliceInput sliceIn MethodHandle keyNativeEquals = typeManager.resolveOperator(OperatorType.EQUAL, asList(keyType, keyType)); MethodHandle keyBlockNativeEquals = compose(keyNativeEquals, nativeValueGetter(keyType)); MethodHandle keyNativeHashCode = typeManager.resolveOperator(OperatorType.HASH_CODE, singletonList(keyType)); - MethodHandle keyBlockHashCode = compose(keyNativeHashCode, nativeValueGetter(keyType)); Block keyBlock = blockEncodingSerde.readBlock(sliceInput); Block valueBlock = blockEncodingSerde.readBlock(sliceInput); - int hashTableLength = sliceInput.readInt(); - int[] hashTable = null; - if (hashTableLength >= 0) { - hashTable = new int[hashTableLength]; - sliceInput.readBytes(wrappedIntArray(hashTable)); - } - - if (keyBlock.getPositionCount() != valueBlock.getPositionCount()) { - throw new IllegalArgumentException( - format("Deserialized SingleMapBlock violates invariants: key %d, value %d", keyBlock.getPositionCount(), valueBlock.getPositionCount())); - } + int[] hashTable = new int[sliceInput.readInt()]; + sliceInput.readBytes(wrappedIntArray(hashTable)); - if (hashTable != null && keyBlock.getPositionCount() * HASH_MULTIPLIER != hashTable.length) { + if (keyBlock.getPositionCount() != valueBlock.getPositionCount() || keyBlock.getPositionCount() * HASH_MULTIPLIER != hashTable.length) { throw new IllegalArgumentException( - format("Deserialized SingleMapBlock violates invariants: expected hashtable size %d, actual hashtable size %d", keyBlock.getPositionCount() * HASH_MULTIPLIER, hashTable.length)); + format("Deserialized SingleMapBlock violates invariants: key %d, value %d, hash %d", keyBlock.getPositionCount(), valueBlock.getPositionCount(), hashTable.length)); } - MapBlock mapBlock = MapBlock.createMapBlockInternal( - 0, - 1, - Optional.empty(), - new int[] {0, keyBlock.getPositionCount()}, - keyBlock, - valueBlock, - Optional.ofNullable(hashTable), - keyType, - keyBlockNativeEquals, - keyNativeHashCode, - keyBlockHashCode); - - return new SingleMapBlock(0, keyBlock.getPositionCount() * 2, mapBlock); + return new SingleMapBlock(0, keyBlock.getPositionCount() * 2, keyBlock, valueBlock, hashTable, keyType, keyNativeHashCode, keyBlockNativeEquals); } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java b/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java index 2da884b04aae6..8a6e4b940f687 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java @@ -278,11 +278,11 @@ public static Block createMapBlockInternal( int[] offsets, Block keyBlock, Block valueBlock, - Optional hashTables) + int[] hashTables) { // TypeManager caches types. Therefore, it is important that we go through it instead of coming up with the MethodHandles directly. // BIGINT is chosen arbitrarily here. Any type will do. MapType mapType = (MapType) typeManager.getType(new TypeSignature(StandardTypes.MAP, TypeSignatureParameter.of(keyType.getTypeSignature()), TypeSignatureParameter.of(BIGINT.getTypeSignature()))); - return MapBlock.createMapBlockInternal(startOffset, positionCount, mapIsNull, offsets, keyBlock, valueBlock, hashTables, keyType, mapType.keyBlockNativeEquals, mapType.keyNativeHashCode, mapType.keyBlockHashCode); + return MapBlock.createMapBlockInternal(startOffset, positionCount, mapIsNull, offsets, keyBlock, valueBlock, hashTables, keyType, mapType.keyBlockNativeEquals, mapType.keyNativeHashCode); } }