Skip to content

Commit

Permalink
Cherry pick Gh 15187 into dremio 24.3 12 (#47)
Browse files Browse the repository at this point in the history
* apacheGH-15187: [Java] Made `reader` initialization lazy and added new `getTransferPair()` function that takes in a `Field` type (apache#34424)

This PR closes apache#15187. `FieldReader` is being allocated directly in the constructor today, and this PR changes it such that the initialization becomes lazy. Additionally, a new function `getTransferPair(Field, Allocator)` is introduced so that a new `Field` method is not constructed each time `getTransferPair` is called on the Vector.

1. Introduce a new `getTransferPair` method.
2. Make initializing `FieldReader` lazy.

Yes, some tests have been added to verify these changes.

I am not 100% sure if there are any user facing changes.

There should not be any breaking changes.
* Closes: apache#15187

Authored-by: Ramasai <ramasai.tadepalli+3108@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

* Fix import

* Fix imports

---------

Signed-off-by: David Li <li.davidm96@gmail.com>
Co-authored-by: rtadepalli <105760760+rtadepalli@users.noreply.github.com>
  • Loading branch information
lriggs and rtadepalli committed Sep 8, 2023
1 parent e4c1c2c commit c2ad17b
Show file tree
Hide file tree
Showing 48 changed files with 763 additions and 347 deletions.
Expand Up @@ -580,6 +580,14 @@ public TransferPair getTransferPair(BufferAllocator allocator) {
*/
public abstract TransferPair getTransferPair(String ref, BufferAllocator allocator);

/**
* Construct a transfer pair of this vector and another vector of same type.
* @param field Field object used by the target vector
* @param allocator allocator for the target vector
* @return TransferPair
*/
public abstract TransferPair getTransferPair(Field field, BufferAllocator allocator);

/**
* Transfer this vector'data to another vector. The memory associated
* with this vector is transferred to the allocator of target vector
Expand Down
Expand Up @@ -24,6 +24,7 @@
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.ReferenceManager;
import org.apache.arrow.util.Preconditions;
import org.apache.arrow.vector.complex.reader.FieldReader;
import org.apache.arrow.vector.util.DataSizeRoundingUtil;
import org.apache.arrow.vector.util.TransferPair;
import org.apache.arrow.vector.util.ValueVectorUtility;
Expand All @@ -50,6 +51,8 @@ public abstract class BaseValueVector implements ValueVector {

protected final BufferAllocator allocator;

protected volatile FieldReader fieldReader;

protected BaseValueVector(BufferAllocator allocator) {
this.allocator = Preconditions.checkNotNull(allocator, "allocator cannot be null");
}
Expand Down Expand Up @@ -143,6 +146,35 @@ long computeCombinedBufferSize(int valueCount, int typeWidth) {
return allocator.getRoundingPolicy().getRoundedSize(bufferSize);
}

/**
* Each vector has a different reader that implements the FieldReader interface. Overridden methods must make
* sure to return the correct concrete reader implementation.
*
* @return Returns a lambda that initializes a reader when called.
*/
protected abstract FieldReader getReaderImpl();

/**
* Default implementation to create a reader for the vector. Depends on the individual vector
* class' implementation of {@link #getReaderImpl} to initialize the reader appropriately.
*
* @return Concrete instance of FieldReader by using double-checked locking.
*/
public FieldReader getReader() {
FieldReader reader = fieldReader;

if (reader != null) {
return reader;
}
synchronized (this) {
if (fieldReader == null) {
fieldReader = getReaderImpl();
}

return fieldReader;
}
}

/**
* Container for primitive vectors (1 for the validity bit-mask and one to hold the values).
*/
Expand Down
30 changes: 20 additions & 10 deletions java/vector/src/main/java/org/apache/arrow/vector/BigIntVector.java
Expand Up @@ -37,7 +37,6 @@
*/
public final class BigIntVector extends BaseFixedWidthVector implements BaseIntVector {
public static final byte TYPE_WIDTH = 8;
private final FieldReader reader;

/**
* Instantiate a BigIntVector. This doesn't allocate any memory for
Expand Down Expand Up @@ -71,17 +70,11 @@ public BigIntVector(String name, FieldType fieldType, BufferAllocator allocator)
*/
public BigIntVector(Field field, BufferAllocator allocator) {
super(field, allocator, TYPE_WIDTH);
reader = new BigIntReaderImpl(BigIntVector.this);
}

/**
* Get a reader that supports reading values from this vector.
*
* @return Field Reader for this vector
*/
@Override
public FieldReader getReader() {
return reader;
protected FieldReader getReaderImpl() {
return new BigIntReaderImpl(BigIntVector.this);
}

/**
Expand Down Expand Up @@ -286,7 +279,7 @@ public static long get(final ArrowBuf buffer, final int index) {


/**
* Construct a TransferPair comprising of this and a target vector of
* Construct a TransferPair comprising this and a target vector of
* the same type.
*
* @param ref name of the target vector
Expand All @@ -298,6 +291,19 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
return new TransferImpl(ref, allocator);
}

/**
* Construct a TransferPair comprising of this and a target vector of
* the same type.
*
* @param field Field object used by the target vector
* @param allocator allocator for the target vector
* @return {@link TransferPair}
*/
@Override
public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
return new TransferImpl(field, allocator);
}

/**
* Construct a TransferPair with a desired target vector of the same type.
*
Expand Down Expand Up @@ -331,6 +337,10 @@ public TransferImpl(String ref, BufferAllocator allocator) {
to = new BigIntVector(ref, field.getFieldType(), allocator);
}

public TransferImpl(Field field, BufferAllocator allocator) {
to = new BigIntVector(field, allocator);
}

public TransferImpl(BigIntVector to) {
this.to = to;
}
Expand Down
31 changes: 20 additions & 11 deletions java/vector/src/main/java/org/apache/arrow/vector/BitVector.java
Expand Up @@ -46,8 +46,6 @@ public final class BitVector extends BaseFixedWidthVector {

private static final int HASH_CODE_FOR_ONE = 19;

private final FieldReader reader;

/**
* Instantiate a BitVector. This doesn't allocate any memory for
* the data in vector.
Expand Down Expand Up @@ -80,17 +78,11 @@ public BitVector(String name, FieldType fieldType, BufferAllocator allocator) {
*/
public BitVector(Field field, BufferAllocator allocator) {
super(field, allocator, 0);
reader = new BitReaderImpl(BitVector.this);
}

/**
* Get a reader that supports reading values from this vector.
*
* @return Field Reader for this vector
*/
@Override
public FieldReader getReader() {
return reader;
protected FieldReader getReaderImpl() {
return new BitReaderImpl(BitVector.this);
}

/**
Expand Down Expand Up @@ -542,7 +534,7 @@ public void setRangeToOne(int firstBitIndex, int count) {


/**
* Construct a TransferPair comprising of this and a target vector of
* Construct a TransferPair comprising this and a target vector of
* the same type.
*
* @param ref name of the target vector
Expand All @@ -554,6 +546,19 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
return new TransferImpl(ref, allocator);
}

/**
* Construct a TransferPair comprising this and a target vector of
* the same type.
*
* @param field Field object used by the target vector
* @param allocator allocator for the target vector
* @return {@link TransferPair}
*/
@Override
public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
return new TransferImpl(field, allocator);
}

/**
* Construct a TransferPair with a desired target vector of the same type.
*
Expand All @@ -572,6 +577,10 @@ public TransferImpl(String ref, BufferAllocator allocator) {
to = new BitVector(ref, field.getFieldType(), allocator);
}

public TransferImpl(Field field, BufferAllocator allocator) {
to = new BitVector(field, allocator);
}

public TransferImpl(BitVector to) {
this.to = to;
}
Expand Down
Expand Up @@ -38,7 +38,6 @@
public final class DateDayVector extends BaseFixedWidthVector {

public static final byte TYPE_WIDTH = 4;
private final FieldReader reader;

/**
* Instantiate a DateDayVector. This doesn't allocate any memory for
Expand Down Expand Up @@ -72,17 +71,11 @@ public DateDayVector(String name, FieldType fieldType, BufferAllocator allocator
*/
public DateDayVector(Field field, BufferAllocator allocator) {
super(field, allocator, TYPE_WIDTH);
reader = new DateDayReaderImpl(DateDayVector.this);
}

/**
* Get a reader that supports reading values from this vector.
*
* @return Field Reader for this vector
*/
@Override
public FieldReader getReader() {
return reader;
protected FieldReader getReaderImpl() {
return new DateDayReaderImpl(DateDayVector.this);
}

/**
Expand Down Expand Up @@ -290,7 +283,7 @@ public static int get(final ArrowBuf buffer, final int index) {


/**
* Construct a TransferPair comprising of this and a target vector of
* Construct a TransferPair comprising this and a target vector of
* the same type.
*
* @param ref name of the target vector
Expand All @@ -302,6 +295,19 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
return new TransferImpl(ref, allocator);
}

/**
* Construct a TransferPair comprising this and a target vector of
* the same type.
*
* @param field Field object used by the target vector
* @param allocator allocator for the target vector
* @return {@link TransferPair}
*/
@Override
public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
return new TransferImpl(field, allocator);
}

/**
* Construct a TransferPair with a desired target vector of the same type.
*
Expand All @@ -320,6 +326,10 @@ public TransferImpl(String ref, BufferAllocator allocator) {
to = new DateDayVector(ref, field.getFieldType(), allocator);
}

public TransferImpl(Field field, BufferAllocator allocator) {
to = new DateDayVector(field, allocator);
}

public TransferImpl(DateDayVector to) {
this.to = to;
}
Expand Down
Expand Up @@ -40,7 +40,6 @@
*/
public final class DateMilliVector extends BaseFixedWidthVector {
public static final byte TYPE_WIDTH = 8;
private final FieldReader reader;

/**
* Instantiate a DateMilliVector. This doesn't allocate any memory for
Expand Down Expand Up @@ -74,17 +73,11 @@ public DateMilliVector(String name, FieldType fieldType, BufferAllocator allocat
*/
public DateMilliVector(Field field, BufferAllocator allocator) {
super(field, allocator, TYPE_WIDTH);
reader = new DateMilliReaderImpl(DateMilliVector.this);
}

/**
* Get a reader that supports reading values from this vector.
*
* @return Field Reader for this vector
*/
@Override
public FieldReader getReader() {
return reader;
protected FieldReader getReaderImpl() {
return new DateMilliReaderImpl(DateMilliVector.this);
}

/**
Expand Down Expand Up @@ -293,7 +286,7 @@ public static long get(final ArrowBuf buffer, final int index) {


/**
* Construct a TransferPair comprising of this and a target vector of
* Construct a TransferPair comprising this and a target vector of
* the same type.
*
* @param ref name of the target vector
Expand All @@ -305,6 +298,19 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
return new TransferImpl(ref, allocator);
}

/**
* Construct a TransferPair comprising this and a target vector of
* the same type.
*
* @param field Field object used by the target vector
* @param allocator allocator for the target vector
* @return {@link TransferPair}
*/
@Override
public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
return new TransferImpl(field, allocator);
}

/**
* Construct a TransferPair with a desired target vector of the same type.
*
Expand All @@ -323,6 +329,10 @@ public TransferImpl(String ref, BufferAllocator allocator) {
to = new DateMilliVector(ref, field.getFieldType(), allocator);
}

public TransferImpl(Field field, BufferAllocator allocator) {
to = new DateMilliVector(field, allocator);
}

public TransferImpl(DateMilliVector to) {
this.to = to;
}
Expand Down

0 comments on commit c2ad17b

Please sign in to comment.