Skip to content

GH-15187: [Java] Made reader initialization lazy and added new getTransferPair() function that takes in a Field type #34424

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
merged 13 commits into from
Jun 18, 2023
Merged

GH-15187: [Java] Made reader initialization lazy and added new getTransferPair() function that takes in a Field type #34424

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
bb538c6
Made reader initialization lazy and added new getTransferPair() funct…
rtadepalli Mar 3, 2023
ca11887
Fix javadoc to add @param annotation for BufferAllocator
rtadepalli Mar 3, 2023
778b26f
Merge remote-tracking branch 'upstream/main' into GH-15187
rtadepalli Jun 4, 2023
76219f5
GH-15187: Make reader instantiation thread safe in BaseValueVector
rtadepalli Jun 4, 2023
aa7e1d7
Merge remote-tracking branch 'upstream/main' into GH-15187
rtadepalli Jun 14, 2023
7f73e3c
GH-15187: Change markdown from devils tick to link
rtadepalli Jun 14, 2023
fe52705
GH-15187: Change class instantiation
rtadepalli Jun 14, 2023
fa74584
GH-15187: add new lambda to return implementation of the reader
rtadepalli Jun 16, 2023
5dcce2e
Merge remote-tracking branch 'upstream/main' into GH-15187
rtadepalli Jun 16, 2023
24deeeb
GH-15187: Fix comments
rtadepalli Jun 16, 2023
94fe92d
GH-15187: Move supplier and just return new instance of FieldReader t…
rtadepalli Jun 17, 2023
1ec9a35
Merge remote-tracking branch 'upstream/main' into GH-15187
rtadepalli Jun 17, 2023
b4a0c9d
GH-15187: remove unneeded Supplier imports
rtadepalli Jun 17, 2023
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
7 changes: 7 additions & 0 deletions java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,13 @@ 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 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
28 changes: 25 additions & 3 deletions java/vector/src/main/java/org/apache/arrow/vector/BigIntVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;

import java.util.function.Supplier;

import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.complex.impl.BigIntReaderImpl;
Expand All @@ -37,7 +39,7 @@
*/
public final class BigIntVector extends BaseFixedWidthVector implements BaseIntVector {
public static final byte TYPE_WIDTH = 8;
private final FieldReader reader;
private Supplier<FieldReader> reader;

/**
* Instantiate a BigIntVector. This doesn't allocate any memory for
Expand Down Expand Up @@ -71,7 +73,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);
reader = () -> {
Copy link
Contributor Author

@rtadepalli rtadepalli Mar 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not thread safe, but I am not sure if adding the Google core libraries dependency to get Suppliers::memoize is allowed. I don't know of other ways to make this thread safe without adding any throws clause.

I tried adding apache.commons.lang3#LazyInitializer but that I think requires that I add throws ConcurrentException to the getReader function, doesn't seem that great.

Copy link
Contributor

@lwhite1 lwhite1 Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to liberate the necessary code from either of these libraries? I'm not sure how much baggage they would drag with them

Copy link

@prashanthbdremio prashanthbdremio Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this rtadepalli. Instead of the above logic, should we add a new constructor with a new boolean parameter createReader. We create the reader based on the boolean flag. And in the getReader if the reader is null, we throw an exception. Callers who dont need the reader can call the new constructor, and if they try accessing the reader then they get an exception.

Copy link
Contributor Author

@rtadepalli rtadepalli Jun 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prashanthbdremio I think adding a new constructor would not be backwards compatible, unless we just default the boolean to true in the BigIntVector(field, allocator); constructor (and constructors in other classes). If we do this, I guess we're not really changing much, since the reader will be eagerly created anyways. If you think that is fine, I can make the change.

@lwhite1 I have made some changes to the way the reader is being initialized -- please take another look. Thanks.

final FieldReader fieldReader = new BigIntReaderImpl(BigIntVector.this);
reader = () -> fieldReader;
return fieldReader;
};
}

/**
Expand All @@ -81,7 +87,7 @@ public BigIntVector(Field field, BufferAllocator allocator) {
*/
@Override
public FieldReader getReader() {
return reader;
return reader.get();
}

/**
Expand Down Expand Up @@ -298,6 +304,18 @@ 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 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 +349,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
28 changes: 25 additions & 3 deletions java/vector/src/main/java/org/apache/arrow/vector/BitVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import static org.apache.arrow.memory.util.LargeMemoryUtil.capAtMaxInt;
import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;

import java.util.function.Supplier;

import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.util.ArrowBufPointer;
Expand All @@ -46,7 +48,7 @@ public final class BitVector extends BaseFixedWidthVector {

private static final int HASH_CODE_FOR_ONE = 19;

private final FieldReader reader;
private Supplier<FieldReader> reader;

/**
* Instantiate a BitVector. This doesn't allocate any memory for
Expand Down Expand Up @@ -80,7 +82,11 @@ public BitVector(String name, FieldType fieldType, BufferAllocator allocator) {
*/
public BitVector(Field field, BufferAllocator allocator) {
super(field, allocator, 0);
reader = new BitReaderImpl(BitVector.this);
reader = () -> {
final FieldReader fieldReader = new BitReaderImpl(BitVector.this);
reader = () -> fieldReader;
return fieldReader;
};
}

/**
Expand All @@ -90,7 +96,7 @@ public BitVector(Field field, BufferAllocator allocator) {
*/
@Override
public FieldReader getReader() {
return reader;
return reader.get();
}

/**
Expand Down Expand Up @@ -554,6 +560,18 @@ 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 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 +590,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
28 changes: 25 additions & 3 deletions java/vector/src/main/java/org/apache/arrow/vector/DateDayVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;

import java.util.function.Supplier;

import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.vector.complex.impl.DateDayReaderImpl;
Expand All @@ -38,7 +40,7 @@
public final class DateDayVector extends BaseFixedWidthVector {

public static final byte TYPE_WIDTH = 4;
private final FieldReader reader;
private Supplier<FieldReader> reader;

/**
* Instantiate a DateDayVector. This doesn't allocate any memory for
Expand Down Expand Up @@ -72,7 +74,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);
reader = () -> {
final FieldReader fieldReader = new DateDayReaderImpl(DateDayVector.this);
reader = () -> fieldReader;
return fieldReader;
};
}

/**
Expand All @@ -82,7 +88,7 @@ public DateDayVector(Field field, BufferAllocator allocator) {
*/
@Override
public FieldReader getReader() {
return reader;
return reader.get();
}

/**
Expand Down Expand Up @@ -302,6 +308,18 @@ 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 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 +338,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
27 changes: 24 additions & 3 deletions java/vector/src/main/java/org/apache/arrow/vector/DateMilliVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;

import java.time.LocalDateTime;
import java.util.function.Supplier;

import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
Expand All @@ -40,7 +41,7 @@
*/
public final class DateMilliVector extends BaseFixedWidthVector {
public static final byte TYPE_WIDTH = 8;
private final FieldReader reader;
private Supplier<FieldReader> reader;

/**
* Instantiate a DateMilliVector. This doesn't allocate any memory for
Expand Down Expand Up @@ -74,7 +75,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);
reader = () -> {
final FieldReader fieldReader = new DateMilliReaderImpl(DateMilliVector.this);
reader = () -> fieldReader;
return fieldReader;
};
}

/**
Expand All @@ -84,7 +89,7 @@ public DateMilliVector(Field field, BufferAllocator allocator) {
*/
@Override
public FieldReader getReader() {
return reader;
return reader.get();
}

/**
Expand Down Expand Up @@ -305,6 +310,18 @@ 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 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 +340,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
27 changes: 24 additions & 3 deletions java/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.math.BigDecimal;
import java.nio.ByteOrder;
import java.util.function.Supplier;

import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.BufferAllocator;
Expand All @@ -46,7 +47,7 @@ public final class Decimal256Vector extends BaseFixedWidthVector {
public static final int MAX_PRECISION = 76;
public static final byte TYPE_WIDTH = 32;
private static final boolean LITTLE_ENDIAN = ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN;
private final FieldReader reader;
private Supplier<FieldReader> reader;

private final int precision;
private final int scale;
Expand Down Expand Up @@ -85,7 +86,11 @@ public Decimal256Vector(String name, FieldType fieldType, BufferAllocator alloca
public Decimal256Vector(Field field, BufferAllocator allocator) {
super(field, allocator, TYPE_WIDTH);
ArrowType.Decimal arrowType = (ArrowType.Decimal) field.getFieldType().getType();
reader = new Decimal256ReaderImpl(Decimal256Vector.this);
reader = () -> {
final FieldReader fieldReader = new Decimal256ReaderImpl(Decimal256Vector.this);
reader = () -> fieldReader;
return fieldReader;
};
this.precision = arrowType.getPrecision();
this.scale = arrowType.getScale();
}
Expand All @@ -97,7 +102,7 @@ public Decimal256Vector(Field field, BufferAllocator allocator) {
*/
@Override
public FieldReader getReader() {
return reader;
return reader.get();
}

/**
Expand Down Expand Up @@ -544,6 +549,18 @@ 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 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 @@ -563,6 +580,10 @@ public TransferImpl(String ref, BufferAllocator allocator) {
Decimal256Vector.this.scale);
}

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

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