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.

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

Replace checked casts with as for performance #1918

Closed
alamb opened this issue Jun 21, 2022 · 0 comments · Fixed by #2793
Closed

Replace checked casts with as for performance #1918

alamb opened this issue Jun 21, 2022 · 0 comments · Fixed by #2793
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers performance

Comments

@alamb
Copy link
Contributor

alamb commented Jun 21, 2022

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
There are places in the code where checked casts are used (e.g. try_into().unwrap()) where the only codepath that leads to this panic are invalid array data, which the code goes through great lengths to avoid (via validate and validate_full).

Describe the solution you'd like

  1. replace try_into for such cases with an as cast
  2. Run performance benchmarks and see if it improves things (and make a PR if so)

Describe alternatives you've considered
N/A

Additional context
suggested by @tustvold here: https://github.com/apache/arrow-rs/pull/1912/files#r901284237

@alamb alamb added arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog performance good first issue Good for newcomers labels Jun 21, 2022
tustvold added a commit to tustvold/arrow-rs that referenced this issue Sep 27, 2022
Remove num dependency from arrow-buffer

Deprecate unnecessary methods
tustvold added a commit that referenced this issue Sep 28, 2022
Remove num dependency from arrow-buffer

Deprecate unnecessary methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant