Skip to content
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

Jump to bottom

[Java] Allocate the FieldReader for Arrow Vectors only on demand. Also introduce getTransferPair which takes Field as parameter. #15187

Closed
prashanthbdremio opened this issue Jan 4, 2023 · 2 comments · Fixed by #34424
Closed

[Java] Allocate the FieldReader for Arrow Vectors only on demand. Also introduce getTransferPair which takes Field as parameter. #15187

prashanthbdremio opened this issue Jan 4, 2023 · 2 comments · Fixed by #34424
Assignees
@rtadepalli
Labels
Component: Java Type: enhancement
Milestone
13.0.0

Comments

@prashanthbdremio
Copy link

Describe the enhancement requested

[Java] Allocate the FieldReader for Arrow Vectors only on demand. Also introduce getTransferPair which takes Field as parameter.

  1. FieldReader objects are created in the constructor of arrow vectors. FieldReader objects might not be needed in all cases. So create them only if required. https://github.com/apache/arrow/blob/master/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java#L83

  2. Introduce a new getTransferPair method which takes 'Field' as a parameter. In some cases its useful to pass a 'Field' object directly and not create a new 'Field' everytime. https://github.com/apache/arrow/blob/master/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java#L553

Component(s)

Java
@rtadepalli rtadepalli mentioned this issue Mar 3, 2023
Merged
@rtadepalli
Copy link
Contributor

take

@lwhite1
Copy link
Contributor

lwhite1 commented Mar 9, 2023

Other than my comment about trying to lift the code for a thread-safe supplier from Guava, this looks good to me. FWIW, we did something similar with org.apache.arrow.util.Preconditions in the memory component.

rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 4, 2023
@rtadepalli
Merge remote-tracking branch 'upstream/main' into apacheGH-15187
778b26f
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 4, 2023
@rtadepalli
apacheGH-15187: Make reader instantiation thread safe in BaseValueVector
76219f5
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 14, 2023
@rtadepalli
Merge remote-tracking branch 'upstream/main' into apacheGH-15187
aa7e1d7
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 14, 2023
@rtadepalli
apacheGH-15187: Change markdown from devils tick to link
7f73e3c
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 14, 2023
@rtadepalli
apacheGH-15187: Change class instantiation
fe52705
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 16, 2023
@rtadepalli
apacheGH-15187: add new lambda to return implementation of the reader
fa74584
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 16, 2023
@rtadepalli
Merge remote-tracking branch 'upstream/main' into apacheGH-15187
5dcce2e
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 16, 2023
@rtadepalli
apacheGH-15187: Fix comments
24deeeb
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 17, 2023
@rtadepalli
apacheGH-15187: Move supplier and just return new instance of FieldRe…
94fe92d 
…ader type when asked for impl
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 17, 2023
@rtadepalli
Merge remote-tracking branch 'upstream/main' into apacheGH-15187
1ec9a35
rtadepalli added a commit to rtadepalli/arrow that referenced this issue Jun 17, 2023
@rtadepalli
apacheGH-15187: remove unneeded Supplier imports
b4a0c9d
@lidavidm lidavidm added this to the 13.0.0 milestone Jun 18, 2023
lidavidm pushed a commit that referenced this issue Jun 18, 2023
@rtadepalli
GH-15187: [Java] Made reader initialization lazy and added new `get…
c3eaf4c 
…TransferPair()` function that takes in a `Field` type (#34424)

### Rationale for this change
This PR closes #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.

### What changes are included in this PR?

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

### Are these changes tested?

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

### Are there any user-facing changes?

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

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

Authored-by: Ramasai <ramasai.tadepalli+3108@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
lriggs pushed a commit to dremio/arrow that referenced this issue Sep 7, 2023
@rtadepalli @lriggs
apacheGH-15187: [Java] Made reader initialization lazy and added ne…
c795890 
…w `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>
lriggs added a commit to dremio/arrow that referenced this issue Sep 8, 2023
@lriggs @rtadepalli
Cherry pick Gh 15187 into dremio 24.3 12 (#47)
c2ad17b 
* 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>
@xxlaykxx xxlaykxx mentioned this issue Oct 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rtadepalli rtadepalli

Labels
Component: Java Type: enhancement
Projects
None yet
Milestone
13.0.0
4 participants
@lidavidm @lwhite1 @prashanthbdremio @rtadepalli