YimingQiao commented on code in PR #10708:
URL: https://github.com/apache/arrow-rs/pull/10708#discussion_r3794759792
##########
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:
Yep, makes sense. I removed the benchmark from the patch. I left the numbers
in the PR description as a temporary local check, but there is no new benchmark
to maintain.
##########
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:
Thanks, this is clearer. I renamed it to `data_buffers_cloned` and used the
suggested docs. I only changed `string data` to `non-inline string or binary
data`, since the API is shared by StringView and BinaryView, and kept the O(1)
note.
--
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]
