From d80cf92fdaade895acc2edd3d128fbee4035a013 Mon Sep 17 00:00:00 2001 From: Ying Su Date: Tue, 1 Jan 2019 16:47:45 -0800 Subject: [PATCH] Fix lazy map hashtable regression In 23de11f7d3 we lazily build the hashtables for map. It introduced a regression for the case where the MapBlock is created through AbstractMapBlock.getRegion(), in which the hashtables built on the MapBlock region was not updated in the original MapBlock, thus causing repeated hashtables build on the same base MapBlock. The change is to encapsulate the int[] hashtables in AbstractMapBlock$HashTables object and make it a member of MapBlock/MapBlockBuilder, so that when the sliced MapBlock builds the hashtables, the base MapBlock would also gets updated. --- .../presto/array/ReferenceCountMap.java | 4 ++ .../presto/block/AbstractTestBlock.java | 4 ++ .../facebook/presto/block/TestMapBlock.java | 13 ++++ .../presto/spi/block/AbstractMapBlock.java | 72 +++++++++++++++---- .../facebook/presto/spi/block/MapBlock.java | 35 ++++----- .../presto/spi/block/MapBlockBuilder.java | 37 +++++----- .../presto/spi/block/MapBlockEncoding.java | 5 +- .../presto/spi/block/SingleMapBlock.java | 20 +++--- .../spi/block/SingleMapBlockEncoding.java | 3 +- .../com/facebook/presto/spi/type/MapType.java | 3 +- 10 files changed, 137 insertions(+), 59 deletions(-) diff --git a/presto-array/src/main/java/com/facebook/presto/array/ReferenceCountMap.java b/presto-array/src/main/java/com/facebook/presto/array/ReferenceCountMap.java index 84702b8106a15..64bf876c4c881 100644 --- a/presto-array/src/main/java/com/facebook/presto/array/ReferenceCountMap.java +++ b/presto-array/src/main/java/com/facebook/presto/array/ReferenceCountMap.java @@ -13,6 +13,7 @@ */ package com.facebook.presto.array; +import com.facebook.presto.spi.block.AbstractMapBlock; import com.facebook.presto.spi.block.Block; import io.airlift.slice.SizeOf; import io.airlift.slice.Slice; @@ -90,6 +91,9 @@ else if (key instanceof Slice) { else if (key.getClass().isArray()) { extraIdentity = getLength(key); } + else if (key instanceof AbstractMapBlock.HashTables) { + extraIdentity = (int) ((AbstractMapBlock.HashTables) key).getRetainedSizeInBytes(); + } else { throw new IllegalArgumentException(format("Unsupported type for %s", key)); } diff --git a/presto-main/src/test/java/com/facebook/presto/block/AbstractTestBlock.java b/presto-main/src/test/java/com/facebook/presto/block/AbstractTestBlock.java index 63978bd55e7e6..1e5ca386915ba 100644 --- a/presto-main/src/test/java/com/facebook/presto/block/AbstractTestBlock.java +++ b/presto-main/src/test/java/com/facebook/presto/block/AbstractTestBlock.java @@ -14,6 +14,7 @@ package com.facebook.presto.block; import com.facebook.presto.metadata.FunctionRegistry; +import com.facebook.presto.spi.block.AbstractMapBlock.HashTables; import com.facebook.presto.spi.block.Block; import com.facebook.presto.spi.block.BlockBuilder; import com.facebook.presto.spi.block.BlockBuilderStatus; @@ -154,6 +155,9 @@ else if (type == short[].class) { else if (type == DictionaryId.class) { retainedSize += ClassLayout.parseClass(DictionaryId.class).instanceSize(); } + else if (type == HashTables.class) { + retainedSize += ((HashTables) field.get(block)).getRetainedSizeInBytes(); + } else if (type == MethodHandle.class) { // MethodHandles are only used in MapBlock/MapBlockBuilder, // and they are shared among blocks created by the same MapType. 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..367bb209857ff 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 @@ -18,6 +18,7 @@ import com.facebook.presto.spi.block.Block; import com.facebook.presto.spi.block.BlockBuilder; import com.facebook.presto.spi.block.ByteArrayBlock; +import com.facebook.presto.spi.block.MapBlock; import com.facebook.presto.spi.block.MapBlockBuilder; import com.facebook.presto.spi.block.SingleMapBlock; import com.facebook.presto.spi.type.MapType; @@ -93,10 +94,16 @@ public void testLazyHashTableBuildOverBlockRegion() int offset = block.getPositionCount() / 2; Block blockRegion = block.getRegion(offset, block.getPositionCount() - offset); + assertFalse(((MapBlock) blockRegion).isHashTablesCreated()); + assertFalse(((MapBlock) block).isHashTablesCreated()); + // 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); + assertTrue(((MapBlock) blockRegion).isHashTablesCreated()); + assertTrue(((MapBlock) block).isHashTablesCreated()); + Map[] valuesWithNull = alternatingNullValues(values); Block blockWithNull = createBlockWithValuesFromKeyValueBlock(valuesWithNull); @@ -104,9 +111,15 @@ public void testLazyHashTableBuildOverBlockRegion() offset = blockWithNull.getPositionCount() / 2; Block blockRegionWithNull = blockWithNull.getRegion(offset, blockWithNull.getPositionCount() - offset); + assertFalse(((MapBlock) blockRegionWithNull).isHashTablesCreated()); + assertFalse(((MapBlock) blockWithNull).isHashTablesCreated()); + // 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); + + assertTrue(((MapBlock) blockRegionWithNull).isHashTablesCreated()); + assertTrue(((MapBlock) blockWithNull).isHashTablesCreated()); } private Map[] createTestMap(int... entryCounts) 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..5166ff276719a 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 @@ -15,6 +15,7 @@ package com.facebook.presto.spi.block; import com.facebook.presto.spi.type.Type; +import org.openjdk.jol.info.ClassLayout; import javax.annotation.Nullable; @@ -28,6 +29,7 @@ import static com.facebook.presto.spi.block.BlockUtil.compactArray; import static com.facebook.presto.spi.block.BlockUtil.compactOffsets; import static com.facebook.presto.spi.block.MapBlock.createMapBlockInternal; +import static io.airlift.slice.SizeOf.sizeOf; import static java.util.Objects.requireNonNull; public abstract class AbstractMapBlock @@ -55,8 +57,7 @@ public AbstractMapBlock(Type keyType, MethodHandle keyNativeHashCode, MethodHand protected abstract Block getRawValueBlock(); - @Nullable - protected abstract int[] getHashTables(); + protected abstract HashTables getHashTables(); /** * offset is entry-based, not position-based. In other words, @@ -116,7 +117,7 @@ public Block copyPositions(int[] positions, int offset, int length) newPosition++; } - int[] hashTable = getHashTables(); + int[] hashTable = getHashTables().getRawHashTables(); int[] newHashTable = null; if (hashTable != null) { newHashTable = new int[newOffsets[newOffsets.length - 1] * HASH_MULTIPLIER]; @@ -141,7 +142,7 @@ public Block copyPositions(int[] positions, int offset, int length) newOffsets, newKeys, newValues, - Optional.ofNullable(newHashTable), + new HashTables(newHashTable), keyType, keyBlockNativeEquals, keyNativeHashCode, @@ -161,7 +162,7 @@ public Block getRegion(int position, int length) getOffsets(), getRawKeyBlock(), getRawValueBlock(), - Optional.ofNullable(getHashTables()), + getHashTables(), keyType, keyBlockNativeEquals, keyNativeHashCode, @@ -181,7 +182,8 @@ public long getRegionSizeInBytes(int position, int length) return getRawKeyBlock().getRegionSizeInBytes(entriesStart, entryCount) + getRawValueBlock().getRegionSizeInBytes(entriesStart, entryCount) + (Integer.BYTES + Byte.BYTES) * (long) length + - Integer.BYTES * HASH_MULTIPLIER * (long) entryCount; + Integer.BYTES * HASH_MULTIPLIER * (long) entryCount + + getHashTables().getInstanceSizeInBytes(); } @Override @@ -211,7 +213,8 @@ public long getPositionsSizeInBytes(boolean[] positions) return getRawKeyBlock().getPositionsSizeInBytes(entryPositions) + getRawValueBlock().getPositionsSizeInBytes(entryPositions) + (Integer.BYTES + Byte.BYTES) * (long) usedPositionCount + - Integer.BYTES * HASH_MULTIPLIER * (long) usedEntryCount; + Integer.BYTES * HASH_MULTIPLIER * (long) usedEntryCount + + getHashTables().getInstanceSizeInBytes(); } @Override public Block copyRegion(int position, int length) @@ -228,13 +231,13 @@ public Block copyRegion(int position, int length) boolean[] mapIsNull = getMapIsNull(); boolean[] newMapIsNull = mapIsNull == null ? null : compactArray(mapIsNull, position + getOffsetBase(), length); - int[] hashTables = getHashTables(); + int[] hashTables = getHashTables().getRawHashTables(); int[] newHashTable = null; if (hashTables != null) { newHashTable = compactArray(hashTables, startValueOffset * HASH_MULTIPLIER, (endValueOffset - startValueOffset) * HASH_MULTIPLIER); } - if (newKeys == getRawKeyBlock() && newValues == getRawValueBlock() && newOffsets == getOffsets() && newMapIsNull == mapIsNull && newHashTable == getHashTables()) { + if (newKeys == getRawKeyBlock() && newValues == getRawValueBlock() && newOffsets == getOffsets() && newMapIsNull == mapIsNull && newHashTable == hashTables) { return this; } return createMapBlockInternal( @@ -244,7 +247,7 @@ public Block copyRegion(int position, int length) newOffsets, newKeys, newValues, - Optional.ofNullable(newHashTable), + new HashTables(newHashTable), keyType, keyBlockNativeEquals, keyNativeHashCode, @@ -285,7 +288,7 @@ public Block getSingleValueBlock(int position) Block newKeys = getRawKeyBlock().copyRegion(startValueOffset, valueLength); Block newValues = getRawValueBlock().copyRegion(startValueOffset, valueLength); - int[] hashTables = getHashTables(); + int[] hashTables = getHashTables().getRawHashTables(); int[] newHashTable = null; if (hashTables != null) { newHashTable = Arrays.copyOfRange(hashTables, startValueOffset * HASH_MULTIPLIER, endValueOffset * HASH_MULTIPLIER); @@ -298,7 +301,7 @@ public Block getSingleValueBlock(int position) new int[] {0, valueLength}, newKeys, newValues, - Optional.ofNullable(newHashTable), + new HashTables(newHashTable), keyType, keyBlockNativeEquals, keyNativeHashCode, @@ -335,10 +338,55 @@ public boolean isNull(int position) return mapIsNull != null && mapIsNull[position + getOffsetBase()]; } + // Only for testing + public boolean isHashTablesCreated() + { + return getHashTables().getRawHashTables() != null; + } + private void checkReadablePosition(int position) { if (position < 0 || position >= getPositionCount()) { throw new IllegalArgumentException("position is not valid"); } } + + public static class HashTables + { + private static final int INSTANCE_SIZE = ClassLayout.parseClass(MapBlock.class).instanceSize(); + + // Hash to location in map. Writes to the field is protected by "this" monitor. + @Nullable + private volatile int[] hashTables; + + HashTables() + { + this.hashTables = null; + } + + HashTables(int[] hashTables) + { + this.hashTables = hashTables; + } + + int[] getRawHashTables() + { + return hashTables; + } + + void setRawHashTables(int[] hashTables) + { + this.hashTables = hashTables; + } + + public long getInstanceSizeInBytes() + { + return INSTANCE_SIZE; + } + + public long getRetainedSizeInBytes() + { + return INSTANCE_SIZE + sizeOf(hashTables); + } + } } 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..58dc4465ab967 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 @@ -43,7 +43,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 HashTables hashTables; private volatile long sizeInBytes; private final long retainedSizeInBytes; @@ -77,7 +77,7 @@ public static MapBlock fromKeyValueBlock( offsets, keyBlock, valueBlock, - Optional.empty(), + new HashTables(), mapType.getKeyType(), keyBlockNativeEquals, keyNativeHashCode, @@ -100,13 +100,14 @@ public static MapBlock createMapBlockInternal( int[] offsets, Block keyBlock, Block valueBlock, - Optional hashTables, + HashTables hashTables, Type keyType, MethodHandle keyBlockNativeEquals, MethodHandle keyNativeHashCode, MethodHandle keyBlockHashCode) { validateConstructorArguments(startOffset, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, keyType, keyBlockNativeEquals, keyNativeHashCode); + requireNonNull(hashTables, "hashTables is null"); return new MapBlock( startOffset, positionCount, @@ -114,7 +115,7 @@ public static MapBlock createMapBlockInternal( offsets, keyBlock, valueBlock, - hashTables.orElse(null), + hashTables, keyType, keyBlockNativeEquals, keyNativeHashCode, @@ -171,7 +172,7 @@ private MapBlock( int[] offsets, Block keyBlock, Block valueBlock, - @Nullable int[] hashTables, + HashTables hashTables, Type keyType, MethodHandle keyBlockNativeEquals, MethodHandle keyNativeHashCode, @@ -179,8 +180,9 @@ private MapBlock( { super(keyType, keyNativeHashCode, keyBlockNativeEquals, keyBlockHashCode); - if (hashTables != null && 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)); + int[] rawHashTables = hashTables.getRawHashTables(); + 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)); } this.startOffset = startOffset; @@ -190,7 +192,6 @@ private MapBlock( this.keyBlock = keyBlock; this.valueBlock = valueBlock; 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 @@ -201,7 +202,8 @@ private MapBlock( + valueBlock.getRetainedSizeInBytes() + sizeOf(offsets) + sizeOf(mapIsNull) - + sizeOfIntArray(keyBlock.getPositionCount() * HASH_MULTIPLIER); // hashtable size if it was built + + sizeOfIntArray(keyBlock.getPositionCount() * HASH_MULTIPLIER) // Raw int[] hashTables size if it was built + + hashTables.getInstanceSizeInBytes(); // The instance size for HashTables object because it always is not null. } @Override @@ -217,7 +219,7 @@ protected Block getRawValueBlock() } @Override - protected int[] getHashTables() + protected HashTables getHashTables() { return hashTables; } @@ -264,7 +266,8 @@ private void calculateSize() sizeInBytes = keyBlock.getRegionSizeInBytes(entriesStart, entryCount) + valueBlock.getRegionSizeInBytes(entriesStart, entryCount) + (Integer.BYTES + Byte.BYTES) * (long) this.positionCount + - Integer.BYTES * HASH_MULTIPLIER * (long) entryCount; + Integer.BYTES * HASH_MULTIPLIER * (long) entryCount + + hashTables.getInstanceSizeInBytes(); } @Override @@ -280,7 +283,7 @@ public void retainedBytesForEachPart(BiConsumer consumer) consumer.accept(valueBlock, valueBlock.getRetainedSizeInBytes()); consumer.accept(offsets, sizeOf(offsets)); consumer.accept(mapIsNull, sizeOf(mapIsNull)); - consumer.accept(hashTables, sizeOf(hashTables)); + consumer.accept(hashTables, hashTables.getRetainedSizeInBytes()); consumer.accept(this, (long) INSTANCE_SIZE); } @@ -312,7 +315,7 @@ public Block getLoadedBlock() offsets, keyBlock, loadedValueBlock, - Optional.ofNullable(hashTables), + hashTables, keyType, keyBlockNativeEquals, keyNativeHashCode, @@ -322,13 +325,13 @@ public Block getLoadedBlock() @Override protected void ensureHashTableLoaded() { - if (this.hashTables != null) { + if (this.hashTables.getRawHashTables() != null) { return; } // This can only happen for MapBlock, not MapBlockBuilder because the latter always has non-null hashtables synchronized (this) { - if (this.hashTables != null) { + if (this.hashTables.getRawHashTables() != null) { return; } @@ -352,7 +355,7 @@ protected void ensureHashTableLoaded() keyOffset * HASH_MULTIPLIER, keyCount * HASH_MULTIPLIER); } - this.hashTables = hashTables; + this.hashTables.setRawHashTables(hashTables); } } } 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..527bed721744a 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 @@ -48,7 +48,7 @@ public class MapBlockBuilder private boolean[] mapIsNull; private final BlockBuilder keyBlockBuilder; private final BlockBuilder valueBlockBuilder; - private int[] hashTables; + private HashTables hashTables; private boolean currentEntryOpened; @@ -87,7 +87,7 @@ private MapBlockBuilder( BlockBuilder valueBlockBuilder, int[] offsets, boolean[] mapIsNull, - int[] hashTables) + int[] rawHashTables) { super(keyType, keyNativeHashCode, keyBlockNativeEquals, keyBlockHashCode); @@ -99,7 +99,7 @@ private MapBlockBuilder( this.mapIsNull = requireNonNull(mapIsNull, "mapIsNull is null"); this.keyBlockBuilder = requireNonNull(keyBlockBuilder, "keyBlockBuilder is null"); this.valueBlockBuilder = requireNonNull(valueBlockBuilder, "valueBlockBuilder is null"); - this.hashTables = requireNonNull(hashTables, "hashTables is null"); + this.hashTables = new HashTables(requireNonNull(rawHashTables, "hashTables is null")); } @Override @@ -115,7 +115,7 @@ protected Block getRawValueBlock() } @Override - protected int[] getHashTables() + protected HashTables getHashTables() { return hashTables; } @@ -149,7 +149,8 @@ public long getSizeInBytes() { return keyBlockBuilder.getSizeInBytes() + valueBlockBuilder.getSizeInBytes() + (Integer.BYTES + Byte.BYTES) * (long) positionCount + - Integer.BYTES * HASH_MULTIPLIER * (long) keyBlockBuilder.getPositionCount(); + Integer.BYTES * HASH_MULTIPLIER * (long) keyBlockBuilder.getPositionCount() + + hashTables.getInstanceSizeInBytes(); } @Override @@ -160,7 +161,7 @@ public long getRetainedSizeInBytes() + valueBlockBuilder.getRetainedSizeInBytes() + sizeOf(offsets) + sizeOf(mapIsNull) - + sizeOf(hashTables); + + hashTables.getRetainedSizeInBytes(); if (blockBuilderStatus != null) { size += BlockBuilderStatus.INSTANCE_SIZE; } @@ -174,7 +175,7 @@ public void retainedBytesForEachPart(BiConsumer consumer) consumer.accept(valueBlockBuilder, valueBlockBuilder.getRetainedSizeInBytes()); consumer.accept(offsets, sizeOf(offsets)); consumer.accept(mapIsNull, sizeOf(mapIsNull)); - consumer.accept(hashTables, sizeOf(hashTables)); + consumer.accept(hashTables, hashTables.getRetainedSizeInBytes()); consumer.accept(this, (long) INSTANCE_SIZE); } @@ -207,7 +208,7 @@ public BlockBuilder closeEntry() previousAggregatedEntryCount, entryCount, keyBlockHashCode, - hashTables, + hashTables.getRawHashTables(), previousAggregatedEntryCount * HASH_MULTIPLIER, entryCount * HASH_MULTIPLIER); return this; @@ -240,7 +241,7 @@ public BlockBuilder closeEntryStrict() entryCount, keyBlockEquals, keyBlockHashCode, - hashTables, + hashTables.getRawHashTables(), previousAggregatedEntryCount * HASH_MULTIPLIER, entryCount * HASH_MULTIPLIER); return this; @@ -264,7 +265,7 @@ private BlockBuilder closeEntry(@Nullable int[] providedHashTable, int providedH int hashTableOffset = previousAggregatedEntryCount * HASH_MULTIPLIER; int hashTableSize = (aggregatedEntryCount - previousAggregatedEntryCount) * HASH_MULTIPLIER; for (int i = 0; i < hashTableSize; i++) { - hashTables[hashTableOffset + i] = providedHashTable[providedHashTableOffset + i]; + hashTables.getRawHashTables()[hashTableOffset + i] = providedHashTable[providedHashTableOffset + i]; } } else { @@ -275,7 +276,7 @@ private BlockBuilder closeEntry(@Nullable int[] providedHashTable, int providedH previousAggregatedEntryCount, entryCount, keyBlockHashCode, - hashTables, + hashTables.getRawHashTables(), previousAggregatedEntryCount * HASH_MULTIPLIER, entryCount * HASH_MULTIPLIER); } @@ -315,11 +316,12 @@ private void entryAdded(boolean isNull) private void ensureHashTableSize() { - if (hashTables.length < offsets[positionCount] * HASH_MULTIPLIER) { + int[] rawHashTables = hashTables.getRawHashTables(); + if (rawHashTables.length < offsets[positionCount] * HASH_MULTIPLIER) { int newSize = BlockUtil.calculateNewArraySize(offsets[positionCount] * HASH_MULTIPLIER); - int oldSize = hashTables.length; - hashTables = Arrays.copyOf(hashTables, newSize); - Arrays.fill(hashTables, oldSize, hashTables.length, -1); + int[] newRawHashTables = Arrays.copyOf(rawHashTables, newSize); + Arrays.fill(newRawHashTables, rawHashTables.length, newSize, -1); + hashTables.setRawHashTables(newRawHashTables); } } @@ -329,6 +331,7 @@ public Block build() if (currentEntryOpened) { throw new IllegalStateException("Current entry must be closed before the block can be built"); } + return createMapBlockInternal( 0, positionCount, @@ -336,7 +339,7 @@ public Block build() offsets, keyBlockBuilder.build(), valueBlockBuilder.build(), - Optional.of(Arrays.copyOf(hashTables, offsets[positionCount] * HASH_MULTIPLIER)), + new HashTables(Arrays.copyOf(hashTables.getRawHashTables(), offsets[positionCount] * HASH_MULTIPLIER)), keyType, keyBlockNativeEquals, keyNativeHashCode, @@ -415,7 +418,7 @@ public BlockBuilder appendStructureInternal(Block block, int position) } } - closeEntry(mapBlock.getHashTables(), startValueOffset * HASH_MULTIPLIER); + closeEntry(mapBlock.getHashTables().getRawHashTables(), startValueOffset * HASH_MULTIPLIER); return this; } 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..1abfcd45f0275 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 @@ -14,6 +14,7 @@ package com.facebook.presto.spi.block; +import com.facebook.presto.spi.block.AbstractMapBlock.HashTables; import com.facebook.presto.spi.type.MapType; import com.facebook.presto.spi.type.Type; import com.facebook.presto.spi.type.TypeManager; @@ -54,7 +55,7 @@ public void writeBlock(BlockEncodingSerde blockEncodingSerde, SliceOutput sliceO int offsetBase = mapBlock.getOffsetBase(); int[] offsets = mapBlock.getOffsets(); - int[] hashTable = mapBlock.getHashTables(); + int[] hashTable = mapBlock.getHashTables().getRawHashTables(); int entriesStartOffset = offsets[offsetBase]; int entriesEndOffset = offsets[offsetBase + positionCount]; @@ -110,6 +111,6 @@ public Block readBlock(BlockEncodingSerde blockEncodingSerde, SliceInput sliceIn 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, new HashTables(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..4ea491d26ef75 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 @@ -25,7 +25,6 @@ import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.spi.block.AbstractMapBlock.HASH_MULTIPLIER; import static com.facebook.presto.spi.block.MapBlockBuilder.computePosition; -import static io.airlift.slice.SizeOf.sizeOf; import static io.airlift.slice.SizeOf.sizeOfIntArray; import static java.lang.String.format; @@ -62,7 +61,8 @@ public long getSizeInBytes() @Override public long getRetainedSizeInBytes() { - return INSTANCE_SIZE + mapBlock.getRawKeyBlock().getRetainedSizeInBytes() + mapBlock.getRawValueBlock().getRetainedSizeInBytes() + sizeOf(mapBlock.getHashTables()); + return INSTANCE_SIZE + mapBlock.getRawKeyBlock().getRetainedSizeInBytes() + mapBlock.getRawValueBlock().getRetainedSizeInBytes() + + mapBlock.getHashTables().getRetainedSizeInBytes(); } @Override @@ -70,7 +70,7 @@ 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(mapBlock.getHashTables(), mapBlock.getHashTables().getRetainedSizeInBytes()); consumer.accept(this, (long) INSTANCE_SIZE); } @@ -124,7 +124,7 @@ public Block getLoadedBlock() int[] getHashTable() { - return mapBlock.getHashTables(); + return mapBlock.getHashTables().getRawHashTables(); } Type getKeyType() @@ -142,7 +142,7 @@ public int seekKey(Object nativeValue) } mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); + int[] hashTable = mapBlock.getHashTables().getRawHashTables(); long hashCode; try { @@ -189,7 +189,7 @@ public int seekKeyExact(long nativeValue) } mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); + int[] hashTable = mapBlock.getHashTables().getRawHashTables(); long hashCode; try { @@ -233,7 +233,7 @@ public int seekKeyExact(boolean nativeValue) } mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); + int[] hashTable = mapBlock.getHashTables().getRawHashTables(); long hashCode; try { @@ -277,7 +277,7 @@ public int seekKeyExact(double nativeValue) } mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); + int[] hashTable = mapBlock.getHashTables().getRawHashTables(); long hashCode; try { @@ -321,7 +321,7 @@ public int seekKeyExact(Slice nativeValue) } mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); + int[] hashTable = mapBlock.getHashTables().getRawHashTables(); long hashCode; try { @@ -365,7 +365,7 @@ public int seekKeyExact(Block nativeValue) } mapBlock.ensureHashTableLoaded(); - int[] hashTable = mapBlock.getHashTables(); + int[] hashTable = mapBlock.getHashTables().getRawHashTables(); long hashCode; try { 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..74420fa21c00e 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 @@ -14,6 +14,7 @@ package com.facebook.presto.spi.block; +import com.facebook.presto.spi.block.AbstractMapBlock.HashTables; import com.facebook.presto.spi.function.OperatorType; import com.facebook.presto.spi.type.Type; import com.facebook.presto.spi.type.TypeManager; @@ -110,7 +111,7 @@ public Block readBlock(BlockEncodingSerde blockEncodingSerde, SliceInput sliceIn new int[] {0, keyBlock.getPositionCount()}, keyBlock, valueBlock, - Optional.ofNullable(hashTable), + new HashTables(hashTable), keyType, keyBlockNativeEquals, keyNativeHashCode, 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..1aa76106d7f05 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 @@ -14,6 +14,7 @@ package com.facebook.presto.spi.type; import com.facebook.presto.spi.ConnectorSession; +import com.facebook.presto.spi.block.AbstractMapBlock.HashTables; import com.facebook.presto.spi.block.Block; import com.facebook.presto.spi.block.BlockBuilder; import com.facebook.presto.spi.block.BlockBuilderStatus; @@ -278,7 +279,7 @@ public static Block createMapBlockInternal( int[] offsets, Block keyBlock, Block valueBlock, - Optional hashTables) + HashTables 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.