Skip to content

Dont use row block field block offsets if not needed #10825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sopel39 merged 2 commits into from
Feb 23, 2022
Merged

Dont use row block field block offsets if not needed #10825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
660b887
Add support for RowType to BenchmarkDataGenerator and benchmarks
Feb 22, 2022
f1459b2
Do not use/serialize/deserialize fieldBlockOffset when not needed
Feb 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 73 additions & 29 deletions core/trino-main/src/test/java/io/trino/execution/buffer/BenchmarkBlockSerde.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import io.trino.spi.type.DecimalType;
Comment thread
radek-kondziolka marked this conversation as resolved.
Outdated
import io.trino.spi.type.Decimals;
import io.trino.spi.type.Int128;
import io.trino.spi.type.RowType;
import io.trino.spi.type.SqlDecimal;
import io.trino.spi.type.Type;
import io.trino.spi.type.VarcharType;
Expand All @@ -42,7 +43,9 @@
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import org.testng.annotations.Test;
import oshi.util.tuples.Pair;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Random;
Expand Down Expand Up @@ -176,6 +179,18 @@ public Object deserializeLineitem(LineitemBenchmarkData data)
return ImmutableList.copyOf(readPages(data.getPagesSerde(), new BasicSliceInput(data.getDataSource())));
}

@Benchmark
public Object serializeRow(RowTypeBenchmarkData data)
{
return serializePages(data);
}

@Benchmark
public Object deserializeRow(RowTypeBenchmarkData data)
{
return ImmutableList.copyOf(readPages(data.getPagesSerde(), new BasicSliceInput(data.getDataSource())));
}

private static List<Slice> serializePages(BenchmarkData data)
{
PagesSerdeContext context = new PagesSerdeContext();
Expand All @@ -199,35 +214,7 @@ public void setup(Type type, Function<Random, ?> valueGenerator)

Iterator<?> values = createValues(ROWS, valueGenerator, nullChance);
while (values.hasNext()) {
Object value = values.next();
if (value == null) {
blockBuilder.appendNull();
}
else if (BIGINT.equals(type)) {
BIGINT.writeLong(blockBuilder, ((Number) value).longValue());
}
else if (Decimals.isLongDecimal(type)) {
type.writeObject(blockBuilder, Int128.valueOf(((SqlDecimal) value).toBigDecimal().unscaledValue()));
}
else if (type instanceof VarcharType) {
Slice slice = truncateToLength(utf8Slice((String) value), type);
type.writeSlice(blockBuilder, slice);
}
else if (TIMESTAMP_PICOS.equals(type)) {
TIMESTAMP_PICOS.writeObject(blockBuilder, value);
}
else if (INTEGER.equals(type)) {
blockBuilder.writeInt((int) value);
}
else if (SMALLINT.equals(type)) {
blockBuilder.writeShort((short) value);
}
else if (TINYINT.equals(type)) {
blockBuilder.writeByte((byte) value);
}
else {
throw new IllegalArgumentException("Unsupported type " + type);
}
writeValue(type, values.next(), blockBuilder);
pageBuilder.declarePosition();
if (pageBuilder.isFull()) {
pagesBuilder.add(pageBuilder.build());
Expand All @@ -245,6 +232,51 @@ else if (TINYINT.equals(type)) {

setup(sliceOutput.slice(), pagesSerde, pages);
}

private void writeValue(Type type, Object value, BlockBuilder blockBuilder)
Comment thread
radek-kondziolka marked this conversation as resolved.
Outdated
{
if (value == null) {
blockBuilder.appendNull();
}
else if (BIGINT.equals(type)) {
BIGINT.writeLong(blockBuilder, ((Number) value).longValue());
}
else if (Decimals.isLongDecimal(type)) {
type.writeObject(blockBuilder, Int128.valueOf(((SqlDecimal) value).toBigDecimal().unscaledValue()));
}
else if (type instanceof VarcharType) {
Slice slice = truncateToLength(utf8Slice((String) value), type);
type.writeSlice(blockBuilder, slice);
}
else if (TIMESTAMP_PICOS.equals(type)) {
TIMESTAMP_PICOS.writeObject(blockBuilder, value);
}
else if (INTEGER.equals(type)) {
blockBuilder.writeInt((int) value);
}
else if (SMALLINT.equals(type)) {
blockBuilder.writeShort((short) value);
}
else if (TINYINT.equals(type)) {
blockBuilder.writeByte((byte) value);
}
else if (type instanceof RowType) {
BlockBuilder row = blockBuilder.beginBlockEntry();
List<?> values = (List<?>) value;
if (values.size() != type.getTypeParameters().size()) {
throw new IllegalArgumentException("Size of types and values must have the same size");
}
List<Pair<Type, Object>> pairs = new ArrayList<>();
for (int i = 0; i < type.getTypeParameters().size(); i++) {
pairs.add(new Pair<Type, Object>(type.getTypeParameters().get(i), ((List<?>) value).get(i)));
}
pairs.forEach(p -> writeValue(p.getA(), p.getB(), row));
blockBuilder.closeEntry();
}
else {
throw new IllegalArgumentException("Unsupported type " + type);
}
}
}

public abstract static class BenchmarkData
Expand Down Expand Up @@ -369,6 +401,18 @@ public void setup()
}
}

@State(Thread)
public static class RowTypeBenchmarkData
extends TypeBenchmarkData
{
@Setup
public void setup()
{
RowType type = RowType.anonymous(ImmutableList.of(BIGINT));
super.setup(type, (random -> BenchmarkDataGenerator.randomRow(type.getTypeParameters(), random)));
}
}

@Test
public void test()
{
Expand Down
24 changes: 24 additions & 0 deletions core/trino-main/src/test/java/io/trino/execution/buffer/BenchmarkDataGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
*/
package io.trino.execution.buffer;

import io.trino.spi.type.DecimalType;
import io.trino.spi.type.LongTimestamp;
import io.trino.spi.type.SqlDecimal;
import io.trino.spi.type.Type;

import java.math.BigInteger;
import java.util.ArrayList;
Expand All @@ -23,7 +25,9 @@
import java.util.Random;
import java.util.function.Function;

import static io.trino.spi.type.BigintType.BIGINT;
import static io.trino.spi.type.Timestamps.PICOSECONDS_PER_MICROSECOND;
import static io.trino.spi.type.VarcharType.VARCHAR;

public class BenchmarkDataGenerator
{
Expand Down Expand Up @@ -93,4 +97,24 @@ public static byte randomByte(Random random)
{
return (byte) random.nextInt();
}

public static List<Object> randomRow(List<Type> fieldTypes, Random random)
Comment thread
radek-kondziolka marked this conversation as resolved.
Outdated
Comment thread
radek-kondziolka marked this conversation as resolved.
{
List<Object> row = new ArrayList<>(fieldTypes.size());
for (Type type : fieldTypes) {
if (type == VARCHAR) {
row.add(randomAsciiString(random));
}
else if (type == BIGINT) {
row.add(random.nextLong());
}
else if (type instanceof DecimalType) {
row.add(randomLongDecimal(random));
}
else {
throw new UnsupportedOperationException(String.format("The %s is not supported", type));
}
}
return row;
}
}
21 changes: 11 additions & 10 deletions core/trino-spi/src/main/java/io/trino/spi/block/AbstractRowBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public final List<Block> getChildren()

Comment thread
radek-kondziolka marked this conversation as resolved.
Outdated
Comment thread
radek-kondziolka marked this conversation as resolved.
Outdated
protected abstract Block[] getRawFieldBlocks();
Comment thread
radek-kondziolka marked this conversation as resolved.
Outdated

@Nullable
protected abstract int[] getFieldBlockOffsets();

protected abstract int getOffsetBase();
Expand All @@ -51,7 +52,8 @@ public final List<Block> getChildren()
// the offset in each field block, it can also be viewed as the "entry-based" offset in the RowBlock
protected final int getFieldBlockOffset(int position)
{
return getFieldBlockOffsets()[position + getOffsetBase()];
int[] offsets = getFieldBlockOffsets();
return offsets != null ? offsets[position + getOffsetBase()] : position + getOffsetBase();
}

protected AbstractRowBlock(int numFields)
Expand All @@ -73,7 +75,7 @@ public final Block copyPositions(int[] positions, int offset, int length)
{
checkArrayRange(positions, offset, length);

int[] newOffsets = new int[length + 1];
int[] newOffsets = null;

int[] fieldBlockPositions = new int[length];
int fieldBlockPositionCount;
Expand All @@ -82,17 +84,15 @@ public final Block copyPositions(int[] positions, int offset, int length)
// No nulls are present
newRowIsNull = null;
for (int i = 0; i < fieldBlockPositions.length; i++) {
newOffsets[i] = i; // No nulls, all offsets are just their index mapping
int position = positions[offset + i];
checkReadablePosition(position);
fieldBlockPositions[i] = getFieldBlockOffset(position);
}
// Record last offset position
newOffsets[fieldBlockPositions.length] = fieldBlockPositions.length;
fieldBlockPositionCount = fieldBlockPositions.length;
}
else {
newRowIsNull = new boolean[length];
newOffsets = new int[length + 1];
fieldBlockPositionCount = 0;
for (int i = 0; i < length; i++) {
newOffsets[i] = fieldBlockPositionCount;
Expand All @@ -107,8 +107,9 @@ public final Block copyPositions(int[] positions, int offset, int length)
// Record last offset position
newOffsets[length] = fieldBlockPositionCount;
if (fieldBlockPositionCount == length) {
// No nulls encountered, discard the null mask
// No nulls encountered, discard the null mask and offsets
newRowIsNull = null;
newOffsets = null;
}
}

Expand Down Expand Up @@ -183,12 +184,12 @@ public Block copyRegion(int position, int length)
for (int i = 0; i < numFields; i++) {
newBlocks[i] = getRawFieldBlocks()[i].copyRegion(startFieldBlockOffset, fieldBlockLength);
}

int[] newOffsets = compactOffsets(getFieldBlockOffsets(), position + getOffsetBase(), length);
int[] fieldBlockOffsets = getFieldBlockOffsets();
int[] newOffsets = fieldBlockOffsets == null ? null : compactOffsets(fieldBlockOffsets, position + getOffsetBase(), length);
boolean[] rowIsNull = getRowIsNull();
boolean[] newRowIsNull = rowIsNull == null ? null : compactArray(rowIsNull, position + getOffsetBase(), length);

if (arraySame(newBlocks, getRawFieldBlocks()) && newOffsets == getFieldBlockOffsets() && newRowIsNull == rowIsNull) {
if (arraySame(newBlocks, getRawFieldBlocks()) && newOffsets == fieldBlockOffsets && newRowIsNull == rowIsNull) {
return this;
}
return createRowBlockInternal(0, length, newRowIsNull, newOffsets, newBlocks);
Expand Down Expand Up @@ -218,7 +219,7 @@ public Block getSingleValueBlock(int position)
newBlocks[i] = getRawFieldBlocks()[i].copyRegion(startFieldBlockOffset, fieldBlockLength);
}
boolean[] newRowIsNull = isNull(position) ? new boolean[] {true} : null;
int[] newOffsets = new int[] {0, fieldBlockLength};
int[] newOffsets = isNull(position) ? new int[] {0, fieldBlockLength} : null;

return createRowBlockInternal(0, 1, newRowIsNull, newOffsets, newBlocks);
}
Expand Down
30 changes: 15 additions & 15 deletions core/trino-spi/src/main/java/io/trino/spi/block/RowBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,10 @@ public class RowBlock
public static Block fromFieldBlocks(int positionCount, Optional<boolean[]> rowIsNullOptional, Block[] fieldBlocks)
{
boolean[] rowIsNull = rowIsNullOptional.orElse(null);
int[] fieldBlockOffsets = new int[positionCount + 1];
if (rowIsNull == null) {
// Fast-path create identity field block offsets from position only
for (int position = 0; position < fieldBlockOffsets.length; position++) {
fieldBlockOffsets[position] = position;
}
}
else {
int[] fieldBlockOffsets = null;
if (rowIsNull != null) {
// Check for nulls when computing field block offsets
fieldBlockOffsets = new int[positionCount + 1];
fieldBlockOffsets[0] = 0;
for (int position = 0; position < positionCount; position++) {
fieldBlockOffsets[position + 1] = fieldBlockOffsets[position] + (rowIsNull[position] ? 0 : 1);
Expand All @@ -63,6 +58,7 @@ public static Block fromFieldBlocks(int positionCount, Optional<boolean[]> rowIs
if (fieldBlockOffsets[positionCount] == positionCount) {
// No nulls encountered, discard the null mask
rowIsNull = null;
fieldBlockOffsets = null;
}
}

Expand All @@ -73,13 +69,13 @@ public static Block fromFieldBlocks(int positionCount, Optional<boolean[]> rowIs
/**
* Create a row block directly without per element validations.
*/
static RowBlock createRowBlockInternal(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks)
static RowBlock createRowBlockInternal(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, @Nullable int[] fieldBlockOffsets, Block[] fieldBlocks)
{
validateConstructorArguments(startOffset, positionCount, rowIsNull, fieldBlockOffsets, fieldBlocks);
return new RowBlock(startOffset, positionCount, rowIsNull, fieldBlockOffsets, fieldBlocks);
}

private static void validateConstructorArguments(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks)
private static void validateConstructorArguments(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, @Nullable int[] fieldBlockOffsets, Block[] fieldBlocks)
{
if (startOffset < 0) {
throw new IllegalArgumentException("arrayOffset is negative");
Expand All @@ -93,8 +89,11 @@ private static void validateConstructorArguments(int startOffset, int positionCo
throw new IllegalArgumentException("rowIsNull length is less than positionCount");
}

requireNonNull(fieldBlockOffsets, "fieldBlockOffsets is null");
Comment thread
radek-kondziolka marked this conversation as resolved.
Outdated
if (fieldBlockOffsets.length - startOffset < positionCount + 1) {
if ((rowIsNull == null) != (fieldBlockOffsets == null)) {
throw new IllegalArgumentException("When rowIsNull is (non) null then fieldBlockOffsets should be (non) null as well");
}

if (fieldBlockOffsets != null && fieldBlockOffsets.length - startOffset < positionCount + 1) {
throw new IllegalArgumentException("fieldBlockOffsets length is less than positionCount");
}

Expand All @@ -116,7 +115,7 @@ private static void validateConstructorArguments(int startOffset, int positionCo
* Use createRowBlockInternal or fromFieldBlocks instead of this method. The caller of this method is assumed to have
* validated the arguments with validateConstructorArguments.
*/
private RowBlock(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks)
private RowBlock(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, @Nullable int[] fieldBlockOffsets, Block[] fieldBlocks)
{
super(fieldBlocks.length);

Expand All @@ -136,6 +135,7 @@ protected Block[] getRawFieldBlocks()
}

@Override
@Nullable
protected int[] getFieldBlockOffsets()
{
return fieldBlockOffsets;
Expand Down Expand Up @@ -176,8 +176,8 @@ public long getSizeInBytes()
long sizeInBytes = getBaseSizeInBytes();
boolean hasUnloadedBlocks = false;

int startFieldBlockOffset = fieldBlockOffsets[startOffset];
int endFieldBlockOffset = fieldBlockOffsets[startOffset + positionCount];
int startFieldBlockOffset = fieldBlockOffsets != null ? fieldBlockOffsets[startOffset] : startOffset;
int endFieldBlockOffset = fieldBlockOffsets != null ? fieldBlockOffsets[startOffset + positionCount] : startOffset + positionCount;
int fieldBlockLength = endFieldBlockOffset - startFieldBlockOffset;

for (Block fieldBlock : fieldBlocks) {
Expand Down
5 changes: 3 additions & 2 deletions core/trino-spi/src/main/java/io/trino/spi/block/RowBlockBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,10 @@ protected Block[] getRawFieldBlocks()
}

@Override
@Nullable
protected int[] getFieldBlockOffsets()
{
return fieldBlockOffsets;
return hasNullRow ? fieldBlockOffsets : null;
}

@Override
Expand Down Expand Up @@ -223,7 +224,7 @@ public Block build()
for (int i = 0; i < numFields; i++) {
fieldBlocks[i] = fieldBlockBuilders[i].build();
}
return createRowBlockInternal(0, positionCount, hasNullRow ? rowIsNull : null, fieldBlockOffsets, fieldBlocks);
return createRowBlockInternal(0, positionCount, hasNullRow ? rowIsNull : null, hasNullRow ? fieldBlockOffsets : null, fieldBlocks);
}

@Override
Expand Down
Loading