Jefffrey commented on code in PR #10708:
URL: https://github.com/apache/arrow-rs/pull/10708#discussion_r3794669903
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -306,6 +306,16 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
&self.buffers
}
+ /// Returns shared ownership of the buffers storing non-inline values
+ ///
+ /// This operation is `O(1)` and does not clone the individual buffers or
+ /// their contents. See [`Self::data_buffers`] to inspect the buffers
+ /// without taking shared ownership.
+ #[inline]
+ pub fn data_buffers_shared(&self) -> Arc<[Buffer]> {
+ Arc::clone(&self.buffers)
Review Comment:
```suggestion
/// Returns a cloned `Arc` of the buffers storing string data.
///
/// This is useful when needing to construct a new byte view array from
this existing
/// array, but [`into_parts`] is not feasible (e.g. need to keep both
arrays around),
/// and trying to reconstruct the buffers from [`data_buffers`] would
require a
/// a `Vec` allocation and cloning of each buffer element, which can be
expensive
/// if there is a large number of buffers.
///
/// [`into_parts`]: Self::into_parts
/// [`data_buffers`]: Self::data_buffers
#[inline]
pub fn data_buffers_cloned(&self) -> Arc<[Buffer]> {
```
thoughts on adjusting it like so? makes it more clear what the use case is
##########
arrow/benches/take_kernels.rs:
##########
@@ -71,6 +72,26 @@ fn bench_take_bounds_check(values: &dyn Array, indices:
&UInt32Array) {
hint::black_box(take(values, indices, Some(TakeOptions { check_bounds:
true })).unwrap());
}
+/// Creates an array whose buffer entries all share the same payload
allocation.
+///
+/// Views reference every entry in turn, isolating the cost of cloning the
buffer
+/// collection from the size of the underlying string payload.
+fn create_string_view_array_with_buffers(size: usize, buffer_count: usize) ->
StringViewArray {
Review Comment:
personally i would say we can omit these benchmarks; the change is obvious
enough and the improvement is only for a very niche case
--
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]
