Jefffrey commented on code in PR #11281:
URL: https://github.com/apache/arrow-rs/pull/11281#discussion_r4131284703
##########
arrow-select/src/interleave.rs:
##########
@@ -344,6 +359,24 @@ fn interleave_views<T: ByteViewType>(
Ok(Arc::new(array))
}
+/// Returns the index of `buffer` in `buffers`, adding it if not already
present.
+///
+/// Multiple input arrays, e.g. slices of the same array, may reference the
same
+/// buffer, so this checks by identity to ensure it is only emitted once
+#[inline(never)]
+fn push_buffer(
+ seen: &mut BTreeMap<(*const u8, usize), u32>,
Review Comment:
a minor optimization could be to hash only on pointer then always accept the
buffer with larger len (which means retroactively changing the buffer in
`buffers`), but i suppose thats more trouble than its worth for presumably a
rare case 🤔
- and probably less performant
##########
arrow-select/src/interleave.rs:
##########
@@ -314,8 +317,17 @@ fn interleave_views<T: ByteViewType>(
offsets.push(total_buffers);
}
+ // Marks a buffer in `buffer_to_new_index` that has not yet been
referenced.
+ //
+ // The view's buffer index is a signed 32-bit integer in the Arrow
specification,
+ // so `u32::MAX` can never be a valid buffer index
+ const UNASSIGNED: u32 = u32::MAX;
+
// contains the mapping from old buffer index to new buffer index
- let mut buffer_to_new_index = vec![None; total_buffers];
+ let mut buffer_to_new_index = vec![UNASSIGNED; total_buffers];
Review Comment:
is switching from option to a sentinel value a performance optimization or
ergonomic change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
