zhuqi-lucas commented on code in PR #7873:
URL: https://github.com/apache/arrow-rs/pull/7873#discussion_r2199447035
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -473,13 +473,78 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
/// Note: this function does not attempt to canonicalize / deduplicate
values. For this
/// feature see [`GenericByteViewBuilder::with_deduplicate_strings`].
pub fn gc(&self) -> Self {
- let mut builder =
GenericByteViewBuilder::<T>::with_capacity(self.len());
+ // 1) Read basic properties once
+ let len = self.len(); // number of elements
+ let views = self.views(); // raw u128 "view" values per slot
+ let nulls = self.nulls().cloned(); // reuse & clone existing null
bitmap
+
+ // 1.5) Fast path: if there are buffers, just reuse original views and
no data blocks
+ if self.data_buffers().is_empty() {
+ return unsafe {
+ GenericByteViewArray::new_unchecked(
+ self.views().clone(),
+ vec![], // empty data blocks
+ nulls,
+ )
+ };
+ }
- for v in self.iter() {
- builder.append_option(v);
+ // 2) Calculate total size of all non-inline data and detect if any
exists
+ let total_large = self.total_buffer_bytes_used();
+
+ // 2.5) Fast path: if there is no non-inline data, avoid buffer
allocation & processing
+ if total_large == 0 {
+ // Views are inline-only or all null; just reuse original views
and no data blocks
+ return unsafe {
+ GenericByteViewArray::new_unchecked(
+ self.views().clone(),
+ vec![], // empty data blocks
+ nulls,
+ )
+ };
}
- builder.finish()
+ // 3) Allocate exactly capacity for all non-inline data
+ let mut data_buf = Vec::with_capacity(total_large);
+
+ // 4) Iterate over views and process each inline/non-inline view
+ let views_buf: Vec<u128> = (0..len)
+ .map(|i| self.process_view(i, views, &mut data_buf))
+ .collect();
+
+ // 5) Wrap up buffers
+ let data_block = Buffer::from_vec(data_buf);
+ let views_scalar = ScalarBuffer::from(views_buf);
+ let data_blocks = vec![data_block];
+
+ // SAFETY: views_scalar, data_blocks, and nulls are correctly aligned
and sized
+ unsafe { GenericByteViewArray::new_unchecked(views_scalar,
data_blocks, nulls) }
+ }
+
+ // This is the helper function that processes a view at index `i`,
+ // extracting the data from the buffers if necessary.
+ // It used by `gc` function to process each view.
+ #[inline(always)]
+ fn process_view(&self, i: usize, views: &[u128], data_buf: &mut Vec<u8>)
-> u128 {
Review Comment:
Thank you @alamb for review and good suggestion, addressed in latest PR.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
