diff --git a/presto-main/src/main/java/com/facebook/presto/operator/GenericPageProcessor.java b/presto-main/src/main/java/com/facebook/presto/operator/GenericPageProcessor.java index 171c7e24a3a3..8e0ade78afe3 100644 --- a/presto-main/src/main/java/com/facebook/presto/operator/GenericPageProcessor.java +++ b/presto-main/src/main/java/com/facebook/presto/operator/GenericPageProcessor.java @@ -26,8 +26,6 @@ import com.facebook.presto.spi.type.Type; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import io.airlift.slice.SizeOf; -import io.airlift.slice.Slice; import java.util.HashMap; import java.util.List; @@ -38,7 +36,6 @@ import static com.facebook.presto.spi.block.DictionaryId.randomDictionaryId; import static com.google.common.base.Verify.verify; import static com.google.common.collect.Iterables.getOnlyElement; -import static io.airlift.slice.Slices.wrappedIntArray; import static java.util.Arrays.copyOf; public class GenericPageProcessor @@ -163,7 +160,7 @@ private Block projectColumnarDictionary(Page inputPage, int[] selectedPositions, dictionarySourceIds.put(dictionaryBlock.getDictionarySourceId(), sourceId); } - return new DictionaryBlock(selectedPositions.length, outputDictionary, wrappedIntArray(outputIds), false, sourceId); + return new DictionaryBlock(selectedPositions.length, outputDictionary, outputIds, false, sourceId); } private static BlockBuilder projectColumnar(int[] selectedPositions, BlockBuilder blockBuilder, Block[] inputBlocks, ProjectionFunction projection) @@ -176,11 +173,11 @@ private static BlockBuilder projectColumnar(int[] selectedPositions, BlockBuilde private static int[] filterIds(ProjectionFunction projection, Page page, int[] selectedPositions) { - Slice ids = ((DictionaryBlock) page.getBlock(getOnlyElement(projection.getInputChannels()))).getIds(); + DictionaryBlock dictionaryBlock = (DictionaryBlock) page.getBlock(getOnlyElement(projection.getInputChannels())); int[] outputIds = new int[selectedPositions.length]; for (int pos = 0; pos < selectedPositions.length; pos++) { - outputIds[pos] = ids.getInt(selectedPositions[pos] * SizeOf.SIZE_OF_INT); + outputIds[pos] = dictionaryBlock.getId(selectedPositions[pos]); } return outputIds; } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/gen/PageProcessorCompiler.java b/presto-main/src/main/java/com/facebook/presto/sql/gen/PageProcessorCompiler.java index c7fa539651c2..dc3d308f1a53 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/gen/PageProcessorCompiler.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/gen/PageProcessorCompiler.java @@ -50,8 +50,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.primitives.Primitives; -import io.airlift.slice.Slice; -import io.airlift.slice.Slices; import java.util.Arrays; import java.util.HashMap; @@ -455,7 +453,6 @@ private MethodDefinition generateProjectDictionaryMethod( Variable cardinality = scope.declareVariable("cardinality", body, selectedPositions.length()); Variable dictionary = scope.declareVariable(Block.class, "dictionary"); - Variable ids = scope.declareVariable(Slice.class, "ids"); Variable dictionaryCount = scope.declareVariable(int.class, "dictionaryCount"); Variable inputSourceId = scope.declareVariable(DictionaryId.class, "inputSourceId"); Variable outputSourceId = scope.declareVariable(DictionaryId.class, "outputSourceId"); @@ -471,7 +468,6 @@ private MethodDefinition generateProjectDictionaryMethod( BytecodeExpression castDictionaryBlock = inputBlock.cast(DictionaryBlock.class); body.comment("Extract dictionary, ids, positionCount and dictionarySourceId") .append(dictionary.set(castDictionaryBlock.invoke("getDictionary", Block.class))) - .append(ids.set(castDictionaryBlock.invoke("getIds", Slice.class))) .append(dictionaryCount.set(dictionary.invoke("getPositionCount", int.class))) .append(inputSourceId.set(castDictionaryBlock.invoke("getDictionarySourceId", DictionaryId.class))); @@ -509,8 +505,7 @@ private MethodDefinition generateProjectDictionaryMethod( .append(dictionarySourceIds.invoke("put", Object.class, inputSourceId.cast(Object.class), outputSourceId.cast(Object.class))) .pop())); - BytecodeExpression idsSlice = invokeStatic(Slices.class, "wrappedIntArray", Slice.class, outputIds); - body.append(newInstance(DictionaryBlock.class, cardinality, outputDictionary, idsSlice, constantFalse(), outputSourceId) + body.append(newInstance(DictionaryBlock.class, cardinality, outputDictionary, outputIds, constantFalse(), outputSourceId) .cast(Block.class) .ret()); return method; diff --git a/presto-main/src/test/java/com/facebook/presto/block/BlockAssertions.java b/presto-main/src/test/java/com/facebook/presto/block/BlockAssertions.java index 371458c48865..edc08bd695ca 100644 --- a/presto-main/src/test/java/com/facebook/presto/block/BlockAssertions.java +++ b/presto-main/src/test/java/com/facebook/presto/block/BlockAssertions.java @@ -42,7 +42,6 @@ import static com.facebook.presto.spi.type.VarcharType.VARCHAR; import static com.facebook.presto.testing.TestingConnectorSession.SESSION; import static com.google.common.base.Preconditions.checkArgument; -import static io.airlift.slice.Slices.wrappedIntArray; import static java.lang.Float.floatToRawIntBits; import static java.util.Objects.requireNonNull; import static org.testng.Assert.assertEquals; @@ -155,7 +154,7 @@ public static Block createStringDictionaryBlock(int start, int length) for (int i = 0; i < length; i++) { ids[i] = i % dictionarySize; } - return new DictionaryBlock(length, builder.build(), wrappedIntArray(ids)); + return new DictionaryBlock(length, builder.build(), ids); } public static Block createStringArraysBlock(Iterable> values) @@ -343,7 +342,7 @@ public static Block createLongDictionaryBlock(int start, int length) for (int i = 0; i < length; i++) { ids[i] = i % dictionarySize; } - return new DictionaryBlock(length, builder.build(), wrappedIntArray(ids)); + return new DictionaryBlock(length, builder.build(), ids); } public static Block createLongRepeatBlock(int value, int length) diff --git a/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java b/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java index 290786ec9c67..563116a2374d 100644 --- a/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java +++ b/presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java @@ -23,7 +23,6 @@ import java.util.List; import static io.airlift.slice.SizeOf.SIZE_OF_INT; -import static io.airlift.slice.Slices.wrappedIntArray; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotEquals; import static org.testng.Assert.assertTrue; @@ -85,7 +84,7 @@ public void testCopyPositionsWithCompactionsAndReorder() assertEquals(copiedBlock.getPositionCount(), positionsToCopy.size()); assertBlock(copiedBlock.getDictionary(), new Slice[] { expectedValues[0], expectedValues[5] }); - assertEquals(copiedBlock.getIds(), wrappedIntArray(0, 1, 0, 1, 0)); + assertDictionaryIds(copiedBlock, 0, 1, 0, 1, 0); } @Test @@ -102,7 +101,7 @@ public void testCopyPositionsSamePosition() assertEquals(copiedBlock.getPositionCount(), positionsToCopy.size()); assertBlock(copiedBlock.getDictionary(), new Slice[] { expectedValues[2] }); - assertEquals(copiedBlock.getIds(), wrappedIntArray(0, 0, 0)); + assertDictionaryIds(copiedBlock, 0, 0, 0); } @Test @@ -132,7 +131,7 @@ public void testCompact() assertEquals(compactBlock.getDictionary().getPositionCount(), (expectedValues.length / 2) + 1); assertBlock(compactBlock.getDictionary(), new Slice[] { expectedValues[0], expectedValues[1], expectedValues[3] }); - assertEquals(compactBlock.getIds(), wrappedIntArray(0, 1, 1, 2, 2, 0, 1, 1, 2, 2)); + assertDictionaryIds(compactBlock, 0, 1, 1, 2, 2, 0, 1, 1, 2, 2); assertEquals(compactBlock.isCompact(), true); DictionaryBlock reCompactedBlock = compactBlock.compact(); @@ -149,7 +148,10 @@ public void testCompactAllKeysReferenced() // When there is nothing to compact, we return the same block assertEquals(compactBlock.getDictionary(), dictionaryBlock.getDictionary()); - assertEquals(compactBlock.getIds(), dictionaryBlock.getIds()); + assertEquals(compactBlock.getPositionCount(), dictionaryBlock.getPositionCount()); + for (int position = 0; position < compactBlock.getPositionCount(); position++) { + assertEquals(compactBlock.getId(position), dictionaryBlock.getId(position)); + } assertEquals(compactBlock.isCompact(), true); } @@ -166,7 +168,7 @@ private static DictionaryBlock createDictionaryBlockWithUnreferencedKeys(Slice[] } ids[i] = index; } - return new DictionaryBlock(positionCount, new SliceArrayBlock(dictionarySize, expectedValues), wrappedIntArray(ids)); + return new DictionaryBlock(positionCount, new SliceArrayBlock(dictionarySize, expectedValues), ids); } private static DictionaryBlock createDictionaryBlock(Slice[] expectedValues, int positionCount) @@ -177,6 +179,14 @@ private static DictionaryBlock createDictionaryBlock(Slice[] expectedValues, int for (int i = 0; i < positionCount; i++) { ids[i] = i % dictionarySize; } - return new DictionaryBlock(positionCount, new SliceArrayBlock(dictionarySize, expectedValues), wrappedIntArray(ids)); + return new DictionaryBlock(positionCount, new SliceArrayBlock(dictionarySize, expectedValues), ids); + } + + private static void assertDictionaryIds(DictionaryBlock dictionaryBlock, int... expected) + { + assertEquals(dictionaryBlock.getPositionCount(), expected.length); + for (int position = 0; position < dictionaryBlock.getPositionCount(); position++) { + assertEquals(dictionaryBlock.getId(position), expected[position]); + } } } diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java index a8cfa215a334..b873f71184bb 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java @@ -35,7 +35,6 @@ import com.facebook.presto.type.ArrayType; import com.google.common.collect.ImmutableList; import io.airlift.slice.Slice; -import io.airlift.slice.Slices; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -220,7 +219,7 @@ private static Block createDictionaryValueBlock(int positionCount, int mapSize) for (int i = 0; i < keyIds.length; i++) { keyIds[i] = ThreadLocalRandom.current().nextInt(0, dictionarySize); } - return new DictionaryBlock(positionCount * mapSize, dictionaryBlock, Slices.wrappedIntArray(keyIds)); + return new DictionaryBlock(positionCount * mapSize, dictionaryBlock, keyIds); } private static String randomString(int length) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java index 87325a0b6f43..d6c8804c0ee0 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java @@ -35,7 +35,6 @@ import com.facebook.presto.type.MapType; import com.google.common.collect.ImmutableList; import io.airlift.slice.Slice; -import io.airlift.slice.Slices; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -195,7 +194,7 @@ private static Block createKeyBlock(int positionCount, List keys) for (int i = 0; i < keyIds.length; i++) { keyIds[i] = i % keys.size(); } - return new DictionaryBlock(positionCount * keys.size(), keyDictionaryBlock, Slices.wrappedIntArray(keyIds)); + return new DictionaryBlock(positionCount * keys.size(), keyDictionaryBlock, keyIds); } private static Block createValueBlock(int positionCount, int mapSize) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java index b6d56e40d517..53a3c46cfdc4 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java @@ -36,7 +36,6 @@ import com.facebook.presto.type.MapType; import com.google.common.collect.ImmutableList; import io.airlift.slice.Slice; -import io.airlift.slice.Slices; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -204,7 +203,7 @@ private static Block createKeyBlock(int positionCount, List keys) for (int i = 0; i < keyIds.length; i++) { keyIds[i] = i % keys.size(); } - return new DictionaryBlock(positionCount * keys.size(), keyDictionaryBlock, Slices.wrappedIntArray(keyIds)); + return new DictionaryBlock(positionCount * keys.size(), keyDictionaryBlock, keyIds); } private static Block createFixWidthValueBlock(int positionCount, int mapSize) @@ -243,7 +242,7 @@ private static Block createDictionaryValueBlock(int positionCount, int mapSize) for (int i = 0; i < keyIds.length; i++) { keyIds[i] = ThreadLocalRandom.current().nextInt(0, dictionarySize); } - return new DictionaryBlock(positionCount * mapSize, dictionaryBlock, Slices.wrappedIntArray(keyIds)); + return new DictionaryBlock(positionCount * mapSize, dictionaryBlock, keyIds); } private static String randomString(int length) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestPageProcessorCompiler.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestPageProcessorCompiler.java index 5c2e0417d090..daa1de5e3e57 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestPageProcessorCompiler.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestPageProcessorCompiler.java @@ -44,7 +44,6 @@ import static com.facebook.presto.spi.type.BooleanType.BOOLEAN; import static com.facebook.presto.spi.type.TypeSignature.parseTypeSignature; import static com.facebook.presto.spi.type.VarcharType.VARCHAR; -import static io.airlift.slice.Slices.wrappedIntArray; import static java.lang.Boolean.TRUE; import static java.util.Collections.singletonList; import static org.testng.Assert.assertEquals; @@ -190,7 +189,7 @@ private static DictionaryBlock createDictionaryBlock(Slice[] expectedValues, int for (int i = 0; i < positionCount; i++) { ids[i] = i % dictionarySize; } - return new DictionaryBlock(positionCount, new SliceArrayBlock(dictionarySize, expectedValues), wrappedIntArray(ids)); + return new DictionaryBlock(positionCount, new SliceArrayBlock(dictionarySize, expectedValues), ids); } protected static Slice[] createExpectedValues(int positionCount) diff --git a/presto-orc/src/main/java/com/facebook/presto/orc/reader/SliceDictionaryStreamReader.java b/presto-orc/src/main/java/com/facebook/presto/orc/reader/SliceDictionaryStreamReader.java index 5a32bcd3252a..80341af3dd18 100644 --- a/presto-orc/src/main/java/com/facebook/presto/orc/reader/SliceDictionaryStreamReader.java +++ b/presto-orc/src/main/java/com/facebook/presto/orc/reader/SliceDictionaryStreamReader.java @@ -185,7 +185,7 @@ else if (inDictionary[i]) { } // copy ids into a private array for this block since data vector is reused - Block block = new DictionaryBlock(nextBatchSize, dictionaryBlock, Slices.wrappedIntArray(dataVector)); + Block block = new DictionaryBlock(nextBatchSize, dictionaryBlock, dataVector); readOffset = 0; nextBatchSize = 0; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/Page.java b/presto-spi/src/main/java/com/facebook/presto/spi/Page.java index 0737b7d976fc..03267092c2b8 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/Page.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/Page.java @@ -16,7 +16,6 @@ import com.facebook.presto.spi.block.Block; import com.facebook.presto.spi.block.DictionaryBlock; import com.facebook.presto.spi.block.DictionaryId; -import io.airlift.slice.Slice; import java.util.ArrayList; import java.util.Arrays; @@ -26,8 +25,6 @@ import java.util.concurrent.atomic.AtomicLong; import static com.facebook.presto.spi.block.DictionaryId.randomDictionaryId; -import static io.airlift.slice.SizeOf.SIZE_OF_INT; -import static io.airlift.slice.Slices.wrappedIntArray; import static java.lang.Math.min; import static java.util.Objects.requireNonNull; @@ -163,7 +160,6 @@ private static List compactRelatedBlocks(List { DictionaryBlock firstDictionaryBlock = blocks.get(0); Block dictionary = firstDictionaryBlock.getDictionary(); - Slice dictionaryIds = firstDictionaryBlock.getIds(); int positionCount = firstDictionaryBlock.getPositionCount(); int dictionarySize = dictionary.getPositionCount(); @@ -175,7 +171,7 @@ private static List compactRelatedBlocks(List int newIndex = 0; for (int i = 0; i < positionCount; i++) { - int position = getIndex(dictionaryIds, i); + int position = firstDictionaryBlock.getId(i); if (remapIndex[position] == -1) { dictionaryPositionsToCopy.add(position); remapIndex[position] = newIndex; @@ -189,7 +185,7 @@ private static List compactRelatedBlocks(List } // compact the dictionaries - Slice newIdsSlice = wrappedIntArray(getNewIds(positionCount, dictionaryIds, remapIndex)); + int[] newIds = getNewIds(positionCount, firstDictionaryBlock, remapIndex); List outputDictionaryBlocks = new ArrayList<>(blocks.size()); DictionaryId newDictionaryId = randomDictionaryId(); for (DictionaryBlock dictionaryBlock : blocks) { @@ -199,21 +195,21 @@ private static List compactRelatedBlocks(List try { Block compactDictionary = dictionaryBlock.getDictionary().copyPositions(dictionaryPositionsToCopy); - outputDictionaryBlocks.add(new DictionaryBlock(positionCount, compactDictionary, newIdsSlice, true, newDictionaryId)); + outputDictionaryBlocks.add(new DictionaryBlock(positionCount, compactDictionary, newIds, true, newDictionaryId)); } catch (UnsupportedOperationException e) { // ignore if copy positions is not supported for the dictionary - outputDictionaryBlocks.add(new DictionaryBlock(positionCount, dictionaryBlock.getDictionary(), dictionaryBlock.getIds())); + outputDictionaryBlocks.add(dictionaryBlock); } } return outputDictionaryBlocks; } - private static int[] getNewIds(int positionCount, Slice ids, int[] remapIndex) + private static int[] getNewIds(int positionCount, DictionaryBlock dictionaryBlock, int[] remapIndex) { int[] newIds = new int[positionCount]; for (int i = 0; i < positionCount; i++) { - int newId = remapIndex[getIndex(ids, i)]; + int newId = remapIndex[dictionaryBlock.getId(i)]; if (newId == -1) { throw new IllegalStateException("reference to a non-existent key"); } @@ -222,11 +218,6 @@ private static int[] getNewIds(int positionCount, Slice ids, int[] remapIndex) return newIds; } - private static int getIndex(Slice ids, int i) - { - return ids.getInt(i * SIZE_OF_INT); - } - /** * Assures that all data for the block is in memory. *

diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java index 8e7b2b638b67..264459830b5d 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlock.java @@ -14,6 +14,7 @@ package com.facebook.presto.spi.block; import io.airlift.slice.Slice; +import io.airlift.slice.Slices; import org.openjdk.jol.info.ClassLayout; import java.util.ArrayList; @@ -24,10 +25,9 @@ import static com.facebook.presto.spi.block.BlockUtil.checkValidPositions; import static com.facebook.presto.spi.block.DictionaryId.randomDictionaryId; -import static io.airlift.slice.SizeOf.SIZE_OF_INT; -import static io.airlift.slice.Slices.copyOf; -import static io.airlift.slice.Slices.wrappedIntArray; +import static io.airlift.slice.SizeOf.sizeOf; import static java.lang.Math.min; +import static java.lang.Math.toIntExact; import static java.util.Objects.requireNonNull; public class DictionaryBlock @@ -37,28 +37,34 @@ public class DictionaryBlock private final int positionCount; private final Block dictionary; - private final Slice ids; + private final int idsOffset; + private final int[] ids; private final int retainedSizeInBytes; private volatile int sizeInBytes = -1; private volatile int uniqueIds = -1; private final DictionaryId dictionarySourceId; - public DictionaryBlock(int positionCount, Block dictionary, Slice ids) + public DictionaryBlock(int positionCount, Block dictionary, int[] ids) { - this(positionCount, dictionary, ids, false, randomDictionaryId()); + this(0, positionCount, dictionary, ids, false, randomDictionaryId()); } - public DictionaryBlock(int positionCount, Block dictionary, Slice ids, DictionaryId dictionaryId) + public DictionaryBlock(int positionCount, Block dictionary, int[] ids, DictionaryId dictionaryId) { - this(positionCount, dictionary, ids, false, dictionaryId); + this(0, positionCount, dictionary, ids, false, dictionaryId); } - public DictionaryBlock(int positionCount, Block dictionary, Slice ids, boolean dictionaryIsCompacted) + public DictionaryBlock(int positionCount, Block dictionary, int[] ids, boolean dictionaryIsCompacted) { - this(positionCount, dictionary, ids, dictionaryIsCompacted, randomDictionaryId()); + this(0, positionCount, dictionary, ids, dictionaryIsCompacted, randomDictionaryId()); } - public DictionaryBlock(int positionCount, Block dictionary, Slice ids, boolean dictionaryIsCompacted, DictionaryId dictionarySourceId) + public DictionaryBlock(int positionCount, Block dictionary, int[] ids, boolean dictionaryIsCompacted, DictionaryId dictionarySourceId) + { + this(0, positionCount, dictionary, ids, dictionaryIsCompacted, dictionarySourceId); + } + + private DictionaryBlock(int idsOffset, int positionCount, Block dictionary, int[] ids, boolean dictionaryIsCompacted, DictionaryId dictionarySourceId) { requireNonNull(dictionary, "dictionary is null"); requireNonNull(ids, "ids is null"); @@ -67,15 +73,16 @@ public DictionaryBlock(int positionCount, Block dictionary, Slice ids, boolean d throw new IllegalArgumentException("positionCount is negative"); } - if (ids.length() != positionCount * SIZE_OF_INT) { - throw new IllegalArgumentException("ids length does not match with positionCount"); + this.idsOffset = idsOffset; + if (ids.length - idsOffset < positionCount) { + throw new IllegalArgumentException("ids length is less than positionCount"); } this.positionCount = positionCount; this.dictionary = dictionary; this.ids = ids; this.dictionarySourceId = requireNonNull(dictionarySourceId, "dictionarySourceId is null"); - this.retainedSizeInBytes = INSTANCE_SIZE + dictionary.getRetainedSizeInBytes() + ids.getRetainedSize(); + this.retainedSizeInBytes = toIntExact(INSTANCE_SIZE + dictionary.getRetainedSizeInBytes() + sizeOf(ids)); if (dictionaryIsCompacted) { this.sizeInBytes = this.retainedSizeInBytes; @@ -86,91 +93,91 @@ public DictionaryBlock(int positionCount, Block dictionary, Slice ids, boolean d @Override public int getLength(int position) { - return dictionary.getLength(getIndex(position)); + return dictionary.getLength(getId(position)); } @Override public byte getByte(int position, int offset) { - return dictionary.getByte(getIndex(position), offset); + return dictionary.getByte(getId(position), offset); } @Override public short getShort(int position, int offset) { - return dictionary.getShort(getIndex(position), offset); + return dictionary.getShort(getId(position), offset); } @Override public int getInt(int position, int offset) { - return dictionary.getInt(getIndex(position), offset); + return dictionary.getInt(getId(position), offset); } @Override public long getLong(int position, int offset) { - return dictionary.getLong(getIndex(position), offset); + return dictionary.getLong(getId(position), offset); } @Override public Slice getSlice(int position, int offset, int length) { - return dictionary.getSlice(getIndex(position), offset, length); + return dictionary.getSlice(getId(position), offset, length); } @Override public T getObject(int position, Class clazz) { - return dictionary.getObject(getIndex(position), clazz); + return dictionary.getObject(getId(position), clazz); } @Override public boolean bytesEqual(int position, int offset, Slice otherSlice, int otherOffset, int length) { - return dictionary.bytesEqual(getIndex(position), offset, otherSlice, otherOffset, length); + return dictionary.bytesEqual(getId(position), offset, otherSlice, otherOffset, length); } @Override public int bytesCompare(int position, int offset, int length, Slice otherSlice, int otherOffset, int otherLength) { - return dictionary.bytesCompare(getIndex(position), offset, length, otherSlice, otherOffset, otherLength); + return dictionary.bytesCompare(getId(position), offset, length, otherSlice, otherOffset, otherLength); } @Override public void writeBytesTo(int position, int offset, int length, BlockBuilder blockBuilder) { - dictionary.writeBytesTo(getIndex(position), offset, length, blockBuilder); + dictionary.writeBytesTo(getId(position), offset, length, blockBuilder); } @Override public void writePositionTo(int position, BlockBuilder blockBuilder) { - dictionary.writePositionTo(getIndex(position), blockBuilder); + dictionary.writePositionTo(getId(position), blockBuilder); } @Override public boolean equals(int position, int offset, Block otherBlock, int otherPosition, int otherOffset, int length) { - return dictionary.equals(getIndex(position), offset, otherBlock, otherPosition, otherOffset, length); + return dictionary.equals(getId(position), offset, otherBlock, otherPosition, otherOffset, length); } @Override public long hash(int position, int offset, int length) { - return dictionary.hash(getIndex(position), offset, length); + return dictionary.hash(getId(position), offset, length); } @Override public int compareTo(int leftPosition, int leftOffset, int leftLength, Block rightBlock, int rightPosition, int rightOffset, int rightLength) { - return dictionary.compareTo(getIndex(leftPosition), leftOffset, leftLength, rightBlock, rightPosition, rightOffset, rightLength); + return dictionary.compareTo(getId(leftPosition), leftOffset, leftLength, rightBlock, rightPosition, rightOffset, rightLength); } @Override public Block getSingleValueBlock(int position) { - return dictionary.getSingleValueBlock(getIndex(position)); + return dictionary.getSingleValueBlock(getId(position)); } @Override @@ -195,7 +202,7 @@ private void calculateCompactSize() int uniqueIds = 0; boolean[] seen = new boolean[dictionary.getPositionCount()]; for (int i = 0; i < positionCount; i++) { - int position = getIndex(ids, i); + int position = getId(i); if (!seen[position]) { if (!dictionary.isNull(position)) { // todo this is wrong for ArrayBlock and InterleavedBlock as length means entry count @@ -205,7 +212,7 @@ private void calculateCompactSize() seen[position] = true; } } - this.sizeInBytes = sizeInBytes; + this.sizeInBytes = sizeInBytes + (positionCount * Integer.BYTES); this.uniqueIds = uniqueIds; } @@ -231,14 +238,14 @@ public Block copyPositions(List positions) int[] newIds = new int[positions.size()]; for (int i = 0; i < positions.size(); i++) { - int oldIndex = getIndex(positions.get(i)); + int oldIndex = getId(positions.get(i)); if (!oldIndexToNewIndex.containsKey(oldIndex)) { oldIndexToNewIndex.put(oldIndex, positionsToCopy.size()); positionsToCopy.add(oldIndex); } newIds[i] = oldIndexToNewIndex.get(oldIndex); } - return new DictionaryBlock(positions.size(), dictionary.copyPositions(positionsToCopy), wrappedIntArray(newIds)); + return new DictionaryBlock(positions.size(), dictionary.copyPositions(positionsToCopy), newIds); } @Override @@ -247,8 +254,7 @@ public Block getRegion(int positionOffset, int length) if (positionOffset < 0 || length < 0 || positionOffset + length > positionCount) { throw new IndexOutOfBoundsException("Invalid position " + positionOffset + " in block with " + positionCount + " positions"); } - Slice newIds = ids.slice(positionOffset * SIZE_OF_INT, length * SIZE_OF_INT); - return new DictionaryBlock(length, dictionary, newIds); + return new DictionaryBlock(idsOffset + positionOffset, length, dictionary, ids, false, randomDictionaryId()); } @Override @@ -257,7 +263,7 @@ public Block copyRegion(int position, int length) if (position < 0 || length < 0 || position + length > positionCount) { throw new IndexOutOfBoundsException("Invalid position " + position + " in block with " + positionCount + " positions"); } - Slice newIds = copyOf(ids, position * SIZE_OF_INT, length * SIZE_OF_INT); + int[] newIds = Arrays.copyOfRange(ids, idsOffset + position, idsOffset + position + length); DictionaryBlock dictionaryBlock = new DictionaryBlock(length, dictionary, newIds); return dictionaryBlock.compact(); } @@ -265,7 +271,16 @@ public Block copyRegion(int position, int length) @Override public boolean isNull(int position) { - return dictionary.isNull(getIndex(position)); + return dictionary.isNull(getId(position)); + } + + @Override + public String toString() + { + StringBuilder sb = new StringBuilder("DictionaryBlock{"); + sb.append("positionCount=").append(getPositionCount()); + sb.append('}'); + return sb.toString(); } public Block getDictionary() @@ -273,14 +288,14 @@ public Block getDictionary() return dictionary; } - public Slice getIds() + Slice getIds() { - return ids; + return Slices.wrappedIntArray(ids, idsOffset, positionCount); } public int getId(int position) { - return ids.getInt(position * SIZE_OF_INT); + return ids[position + idsOffset]; } public DictionaryId getDictionarySourceId() @@ -297,11 +312,6 @@ public boolean isCompact() return uniqueIds == dictionary.getPositionCount(); } - private int getIndex(int position) - { - return getIndex(ids, position); - } - public DictionaryBlock compact() { if (isCompact()) { @@ -316,7 +326,7 @@ public DictionaryBlock compact() int newIndex = 0; for (int i = 0; i < positionCount; i++) { - int dictionaryIndex = getIndex(i); + int dictionaryIndex = getId(i); if (remapIndex[dictionaryIndex] == -1) { dictionaryPositionsToCopy.add(dictionaryIndex); remapIndex[dictionaryIndex] = newIndex; @@ -332,7 +342,7 @@ public DictionaryBlock compact() // compact the dictionary int[] newIds = new int[positionCount]; for (int i = 0; i < positionCount; i++) { - int newId = remapIndex[getIndex(i)]; + int newId = remapIndex[getId(i)]; if (newId == -1) { throw new IllegalStateException("reference to a non-existent key"); } @@ -340,16 +350,11 @@ public DictionaryBlock compact() } try { Block compactDictionary = dictionary.copyPositions(dictionaryPositionsToCopy); - return new DictionaryBlock(positionCount, compactDictionary, wrappedIntArray(newIds), true); + return new DictionaryBlock(positionCount, compactDictionary, newIds, true); } catch (UnsupportedOperationException e) { // ignore if copy positions is not supported for the dictionary block return this; } } - - private static int getIndex(Slice ids, int i) - { - return ids.getInt(i * SIZE_OF_INT); - } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java index bb57142b67d0..099b3f1778b0 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/DictionaryBlockEncoding.java @@ -14,9 +14,9 @@ package com.facebook.presto.spi.block; import com.facebook.presto.spi.type.TypeManager; -import io.airlift.slice.Slice; import io.airlift.slice.SliceInput; import io.airlift.slice.SliceOutput; +import io.airlift.slice.Slices; import static java.util.Objects.requireNonNull; @@ -55,10 +55,7 @@ public void writeBlock(SliceOutput sliceOutput, Block block) dictionaryEncoding.writeBlock(sliceOutput, dictionary); // ids - Slice ids = dictionaryBlock.getIds(); - sliceOutput - .appendInt(ids.length()) - .writeBytes(ids); + sliceOutput.writeBytes(dictionaryBlock.getIds()); // instance id sliceOutput.appendLong(dictionaryBlock.getDictionarySourceId().getMostSignificantBits()); @@ -76,8 +73,8 @@ public Block readBlock(SliceInput sliceInput) Block dictionaryBlock = dictionaryEncoding.readBlock(sliceInput); // ids - int lengthIdsSlice = sliceInput.readInt(); - Slice ids = sliceInput.readSlice(lengthIdsSlice); + int[] ids = new int[positionCount]; + sliceInput.readBytes(Slices.wrappedIntArray(ids)); // instance id long mostSignificantBits = sliceInput.readLong(); diff --git a/presto-spi/src/test/java/com/facebook/presto/spi/TestPage.java b/presto-spi/src/test/java/com/facebook/presto/spi/TestPage.java index afd8d873fb5b..605f7076f12d 100644 --- a/presto-spi/src/test/java/com/facebook/presto/spi/TestPage.java +++ b/presto-spi/src/test/java/com/facebook/presto/spi/TestPage.java @@ -13,7 +13,6 @@ */ package com.facebook.presto.spi; -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.block.DictionaryBlock; @@ -25,9 +24,10 @@ import static com.facebook.presto.spi.block.DictionaryId.randomDictionaryId; import static com.facebook.presto.spi.type.BigintType.BIGINT; -import static io.airlift.slice.Slices.wrappedIntArray; +import static com.google.common.base.Preconditions.checkArgument; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertTrue; public class TestPage { @@ -64,24 +64,42 @@ public void testGetRegionFromNoColumnPage() public void testCompactDictionaryBlocks() throws Exception { - Slice[] expectedValues = createExpectedValues(10); - BlockBuilder blockBuilder = BIGINT.createBlockBuilder(new BlockBuilderStatus(), expectedValues.length); - for (Slice expectedValue : expectedValues) { - BIGINT.writeLong(blockBuilder, expectedValue.length()); - } - Block lengthsDictionary = blockBuilder.build(); + int positionCount = 100; // Create 2 dictionary blocks with the same source id DictionaryId commonSourceId = randomDictionaryId(); - DictionaryBlock commonSourceIdBlock1 = createDictionaryBlock(expectedValues, 100, commonSourceId); - DictionaryBlock commonSourceIdBlock2 = new DictionaryBlock(commonSourceIdBlock1.getPositionCount(), lengthsDictionary, commonSourceIdBlock1.getIds(), commonSourceId); + int commonDictionaryUsedPositions = 20; + int[] commonDictionaryIds = getDictionaryIds(positionCount, commonDictionaryUsedPositions); + + // first dictionary contains "varbinary" values + Slice[] dictionaryValues1 = createExpectedValues(50); + SliceArrayBlock dictionary1 = new SliceArrayBlock(dictionaryValues1.length, dictionaryValues1); + DictionaryBlock commonSourceIdBlock1 = new DictionaryBlock(positionCount, dictionary1, commonDictionaryIds, commonSourceId); + + // second dictionary block is "length(firstColumn)" + BlockBuilder dictionary2 = BIGINT.createBlockBuilder(new BlockBuilderStatus(), dictionary1.getPositionCount()); + for (Slice expectedValue : dictionaryValues1) { + BIGINT.writeLong(dictionary2, expectedValue.length()); + } + DictionaryBlock commonSourceIdBlock2 = new DictionaryBlock(positionCount, dictionary2.build(), commonDictionaryIds, commonSourceId); - // Create block with a different source id - DictionaryBlock randomSourceIdBlock = createDictionaryBlock(expectedValues, 100, randomDictionaryId()); + // Create block with a different source id, dictionary size, used + int otherDictionaryUsedPositions = 30; + int[] otherDictionaryIds = getDictionaryIds(positionCount, otherDictionaryUsedPositions); + SliceArrayBlock dictionary3 = new SliceArrayBlock(70, createExpectedValues(70)); + DictionaryBlock randomSourceIdBlock = new DictionaryBlock(positionCount, dictionary3, otherDictionaryIds); Page page = new Page(commonSourceIdBlock1, randomSourceIdBlock, commonSourceIdBlock2); page.compact(); + // dictionary blocks should all be compact + assertTrue(((DictionaryBlock) page.getBlock(0)).isCompact()); + assertTrue(((DictionaryBlock) page.getBlock(1)).isCompact()); + assertTrue(((DictionaryBlock) page.getBlock(2)).isCompact()); + assertEquals(((DictionaryBlock) page.getBlock(0)).getDictionary().getPositionCount(), commonDictionaryUsedPositions); + assertEquals(((DictionaryBlock) page.getBlock(1)).getDictionary().getPositionCount(), otherDictionaryUsedPositions); + assertEquals(((DictionaryBlock) page.getBlock(2)).getDictionary().getPositionCount(), commonDictionaryUsedPositions); + // Blocks that had the same source id before compacting page should have the same source id after compacting page assertNotEquals(((DictionaryBlock) page.getBlock(0)).getDictionarySourceId(), ((DictionaryBlock) page.getBlock(1)).getDictionarySourceId()); assertEquals(((DictionaryBlock) page.getBlock(0)).getDictionarySourceId(), ((DictionaryBlock) page.getBlock(2)).getDictionarySourceId()); @@ -96,7 +114,7 @@ private static Slice[] createExpectedValues(int positionCount) return expectedValues; } - protected static Slice createExpectedValue(int length) + private static Slice createExpectedValue(int length) { DynamicSliceOutput dynamicSliceOutput = new DynamicSliceOutput(16); for (int index = 0; index < length; index++) { @@ -105,14 +123,13 @@ protected static Slice createExpectedValue(int length) return dynamicSliceOutput.slice(); } - private static DictionaryBlock createDictionaryBlock(Slice[] expectedValues, int positionCount, DictionaryId dictionaryId) + private static int[] getDictionaryIds(int positionCount, int dictionarySize) { - int dictionarySize = expectedValues.length; + checkArgument(positionCount > dictionarySize); int[] ids = new int[positionCount]; - for (int i = 0; i < positionCount; i++) { ids[i] = i % dictionarySize; } - return new DictionaryBlock(positionCount, new SliceArrayBlock(dictionarySize, expectedValues), wrappedIntArray(ids), dictionaryId); + return ids; } } diff --git a/presto-spi/src/test/java/com/facebook/presto/spi/block/TestDictionaryBlockEncoding.java b/presto-spi/src/test/java/com/facebook/presto/spi/block/TestDictionaryBlockEncoding.java index 7dfba30df2e6..843350ee0adb 100644 --- a/presto-spi/src/test/java/com/facebook/presto/spi/block/TestDictionaryBlockEncoding.java +++ b/presto-spi/src/test/java/com/facebook/presto/spi/block/TestDictionaryBlockEncoding.java @@ -19,8 +19,6 @@ import com.facebook.presto.spi.type.TimeZoneKey; import com.facebook.presto.spi.type.Type; import io.airlift.slice.DynamicSliceOutput; -import io.airlift.slice.Slice; -import io.airlift.slice.Slices; import org.testng.annotations.Test; import java.util.Locale; @@ -92,10 +90,9 @@ public void testRoundTrip() for (int i = 0; i < 40; i++) { ids[i] = i % 4; } - Slice idsSlice = Slices.wrappedIntArray(ids); BlockEncoding blockEncoding = new DictionaryBlockEncoding(new VariableWidthBlockEncoding()); - DictionaryBlock dictionaryBlock = new DictionaryBlock(positionCount, dictionary, idsSlice); + DictionaryBlock dictionaryBlock = new DictionaryBlock(positionCount, dictionary, ids); DynamicSliceOutput sliceOutput = new DynamicSliceOutput(1024); blockEncoding.writeBlock(sliceOutput, dictionaryBlock); @@ -104,7 +101,9 @@ public void testRoundTrip() assertTrue(actualBlock instanceof DictionaryBlock); DictionaryBlock actualDictionaryBlock = (DictionaryBlock) actualBlock; assertBlockEquals(VARCHAR, actualDictionaryBlock.getDictionary(), dictionary); - assertEquals(actualDictionaryBlock.getIds(), idsSlice); + for (int position = 0; position < actualDictionaryBlock.getPositionCount(); position++) { + assertEquals(actualDictionaryBlock.getId(position), ids[position]); + } assertEquals(actualDictionaryBlock.getDictionarySourceId(), dictionaryBlock.getDictionarySourceId()); }