Introduce new Chunk iteration APIs #8553
Merged
+524
−17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
teh-cmc
commented
Dec 20, 2024
•
teh-cmc
commented
- Way more ergonomic
- Same performance
- Less code
- Support for structs!
- Basis for new upcoming codegen'd deserialization APIs (...at some point)
teh-cmc
added
enhancement
|
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
teh-cmc
commented
Dec 20, 2024
Wumpf
approved these changes
Jan 3, 2025
Comment on lines
+871
to
+921
| let Some(inner_list_array) = array.as_any().downcast_ref::<Arrow2ListArray<i32>>() else { | ||
| if cfg!(debug_assertions) { | ||
| panic!("downcast failed for {component_name}, data discarded"); | ||
| } else { | ||
| re_log::error_once!("downcast failed for {component_name}, data discarded"); | ||
| } | ||
| return Either::Left(std::iter::empty()); | ||
| }; | ||
|
|
||
| let inner_offsets = inner_list_array.offsets(); | ||
| let inner_lengths = inner_list_array.offsets().lengths().collect_vec(); | ||
|
|
||
| let Some(fixed_size_list_array) = inner_list_array | ||
| .values() | ||
| .as_any() | ||
| .downcast_ref::<Arrow2FixedSizeListArray>() | ||
| else { | ||
| if cfg!(debug_assertions) { | ||
| panic!("downcast failed for {component_name}, data discarded"); | ||
| } else { | ||
| re_log::error_once!("downcast failed for {component_name}, data discarded"); | ||
| } | ||
| return Either::Left(std::iter::empty()); | ||
| }; | ||
|
|
||
| let Some(values) = fixed_size_list_array | ||
| .values() | ||
| .as_any() | ||
| .downcast_ref::<Arrow2PrimitiveArray<T>>() | ||
| else { | ||
| if cfg!(debug_assertions) { | ||
| panic!("downcast failed for {component_name}, data discarded"); | ||
| } else { | ||
| re_log::error_once!("downcast failed for {component_name}, data discarded"); | ||
| } | ||
| return Either::Left(std::iter::empty()); | ||
| }; | ||
|
|
||
| let size = fixed_size_list_array.size(); | ||
| let values = values.values(); | ||
|
|
||
| // NOTE: No need for validity checks here, `iter_offsets` already takes care of that. | ||
| Either::Right(component_offsets.map(move |(idx, len)| { | ||
| let inner_offsets = &inner_offsets.as_slice()[idx..idx + len]; | ||
| let inner_lengths = &inner_lengths.as_slice()[idx..idx + len]; | ||
| izip!(inner_offsets, inner_lengths) | ||
| .map(|(&idx, &len)| { | ||
| let idx = idx as usize; | ||
| bytemuck::cast_slice(&values[idx * size..idx * size + len * size]) | ||
| }) | ||
| .collect_vec() |
There was a problem hiding this comment.
everything except collect_vec is not dependent on N -> we likely can save a lot of template instantiations by putting this into a second level utility method that only depends on T but not on N
There was a problem hiding this comment.
I don't think I'm ready to add even more layers and complex return types to all of this (we need the intermediary values too...) for something I don't really have a way to measure right now :/ Also this is private, no the number of instantiations is fixed, at least.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.