Skip to content

Commit

Permalink
Change DictionaryBlock to use an array internally
Browse files Browse the repository at this point in the history
  • Loading branch information
@dain
dain committed Dec 15, 2016
1 parent 3e48a1b commit 7737d80
Show file tree
Hide file tree
Showing 14 changed files with 135 additions and 129 deletions.
9 changes: 3 additions & 6 deletions presto-main/src/main/java/com/facebook/presto/operator/GenericPageProcessor.java
View file
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand Down
7 changes: 1 addition & 6 deletions presto-main/src/main/java/com/facebook/presto/sql/gen/PageProcessorCompiler.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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)));

Expand Down Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions presto-main/src/test/java/com/facebook/presto/block/BlockAssertions.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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<? extends Iterable<String>> values)
Expand Down Expand Up @@ -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)
Expand Down
24 changes: 17 additions & 7 deletions presto-main/src/test/java/com/facebook/presto/block/TestDictionaryBlock.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}

Expand All @@ -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)
Expand All @@ -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]);
}
}
}
3 changes: 1 addition & 2 deletions presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -195,7 +194,7 @@ private static Block createKeyBlock(int positionCount, List<String> 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)
Expand Down
5 changes: 2 additions & 3 deletions presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -204,7 +203,7 @@ private static Block createKeyBlock(int positionCount, List<String> 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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions presto-main/src/test/java/com/facebook/presto/operator/scalar/TestPageProcessorCompiler.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion presto-orc/src/main/java/com/facebook/presto/orc/reader/SliceDictionaryStreamReader.java
View file
Expand Up @@ -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;
Expand Down
21 changes: 6 additions & 15 deletions presto-spi/src/main/java/com/facebook/presto/spi/Page.java
View file
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -163,7 +160,6 @@ private static List<DictionaryBlock> compactRelatedBlocks(List<DictionaryBlock>
{
DictionaryBlock firstDictionaryBlock = blocks.get(0);
Block dictionary = firstDictionaryBlock.getDictionary();
Slice dictionaryIds = firstDictionaryBlock.getIds();

int positionCount = firstDictionaryBlock.getPositionCount();
int dictionarySize = dictionary.getPositionCount();
Expand All @@ -175,7 +171,7 @@ private static List<DictionaryBlock> compactRelatedBlocks(List<DictionaryBlock>

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;
Expand All @@ -189,7 +185,7 @@ private static List<DictionaryBlock> compactRelatedBlocks(List<DictionaryBlock>
}

// compact the dictionaries
Slice newIdsSlice = wrappedIntArray(getNewIds(positionCount, dictionaryIds, remapIndex));
int[] newIds = getNewIds(positionCount, firstDictionaryBlock, remapIndex);
List<DictionaryBlock> outputDictionaryBlocks = new ArrayList<>(blocks.size());
DictionaryId newDictionaryId = randomDictionaryId();
for (DictionaryBlock dictionaryBlock : blocks) {
Expand All @@ -199,21 +195,21 @@ private static List<DictionaryBlock> compactRelatedBlocks(List<DictionaryBlock>

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");
}
Expand All @@ -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.
* <p>
Expand Down

0 comments on commit 7737d80

Please sign in to comment.