Skip to content

Remove one level of indirection in ArrowSchemaConverter. #921

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
Merged
emkornfield merged 5 commits into from
Mar 15, 2023
Merged

Remove one level of indirection in ArrowSchemaConverter. #921

emkornfield merged 5 commits into from
Mar 15, 2023

Conversation

@emkornfield
Copy link
Collaborator

@emkornfield emkornfield commented Mar 14, 2023 •
edited by davidrabinowitz
Loading

  • Make ArrowSchemaConverter Generic
  • Changes all accessors to now extend ArrowSchemaConverter
  • Instantiates new classes via static factory method instead of direct constructor.
  • Moves null checks to each concrete implementation. While I believe JVM should be able to optimize based on the parent method, I'm not 100% sure and this way JVM should be able to avoid megamorphic dispatch for sure (also eliminate old work around from a bug in Arrow 0.9 for checks.
  • Move child fields to StructAccessor (hopefully keeps more common classes smaller).
Remove one level of indirection in ArrowSchemaConverter.
8e36e57 
- Changes all accessors to now extend ArrowSchemaConverter - Instatiates new classes via static factory method. - Moves common null checks to the individual accessors that require int - Move child fields to StructAccessor
@emkornfield
Copy link
Collaborator Author

emkornfield commented Mar 14, 2023

Closed #920
@emkornfield
Copy link
Collaborator Author

emkornfield commented Mar 14, 2023

/gcbrun
@emkornfield
Copy link
Collaborator Author

emkornfield commented Mar 14, 2023

/gcbrun
@davidrabinowitz davidrabinowitz self-requested a review March 14, 2023 17:23
@davidrabinowitz
Copy link
Member

davidrabinowitz commented Mar 14, 2023

/gcbrun
@emkornfield
Copy link
Collaborator Author

emkornfield commented Mar 14, 2023

/gcbrun
@emkornfield
Copy link
Collaborator Author

emkornfield commented Mar 14, 2023

/gcbrun
@emkornfield
Copy link
Collaborator Author

emkornfield commented Mar 15, 2023

@davidrabinowitz I thought I replied here, but I think changes are probably ready for review (I'm not sure what to do about codecov)
@emkornfield
Copy link
Collaborator Author

emkornfield commented Mar 15, 2023

/gcbrun
@emkornfield emkornfield merged commit a8f1cd9 into GoogleCloudDataproc:master Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@emkornfield @davidrabinowitz