You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "cakeni (via GitHub)" <gi...@apache.org> on 2026/08/11 16:31:56 UTC
[PR] perf(variant): build BinaryView arrays directly [arrow-rs]
cakeni opened a new pull request, #10640:
URL: https://github.com/apache/arrow-rs/pull/10640
# Which issue does this PR close?
- Closes #10621.
# Rationale for this change
Finalizing VariantArrayBuilder replayed every offset through BinaryViewBuilder::try_append_view, repeating bounds and value validation for buffers and offsets produced internally by the Variant builders.
# What changes are included in this PR?
- Add a focused Criterion benchmark that times only build() for 262,144 small values.
- Construct views directly from the recorded offset slices.
- Build the final BinaryViewArray from those validated views, with the safety invariants documented at the unchecked constructor.
# Are these changes tested?
- cargo +stable-x86_64-pc-windows-gnu test -p parquet-variant-compute --lib (347 passed)
- cargo +stable-x86_64-pc-windows-gnu bench -p parquet-variant-compute --bench variant_kernels -- variant_array_builder_build_262k_small_values --noplot
- Before: [2.9213 ms, 2.9390 ms, 2.9577 ms]
- After: [2.0393 ms, 2.0538 ms, 2.0689 ms]
- Criterion change: [-30.816%, -30.118%, -29.428%], p = 0.00
- cargo fmt --all -- --check
- git diff --check
# Are there any user-facing changes?
No API or behavior changes. VariantArrayBuilder::build is approximately 30% faster in the focused many-small-values benchmark.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(variant): build BinaryView arrays directly [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #10640:
URL: https://github.com/apache/arrow-rs/pull/10640
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(variant): build BinaryView arrays directly [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10640:
URL: https://github.com/apache/arrow-rs/pull/10640#discussion_r3818107588
##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -458,22 +460,21 @@ impl VariantBuilderExt for VariantValueArrayBuilderExt<'_> {
}
fn binary_view_array_from_buffers(buffer: Vec<u8>, offsets: Vec<usize>) -> BinaryViewArray {
- // All offsets are less than or equal to the buffer length, so we can safely cast all offsets
- // inside the loop below, as long as the buffer length fits in u32.
- u32::try_from(buffer.len()).expect("buffer length should fit in u32");
-
- let mut builder = BinaryViewBuilder::with_capacity(offsets.len());
- let block = builder.append_block(buffer.into());
- // TODO this can be much faster if it creates the views directly during append
- let mut start = 0;
+ // Each builder records the current buffer length after appending a row, so offsets are
+ // monotonically increasing and bounded by the final buffer length.
+ assert!(buffer.len() < u32::MAX as usize);
+
+ let buffer = Buffer::from(buffer);
+ let mut views = Vec::with_capacity(offsets.len());
+ let mut start = 0_usize;
for end in offsets {
- let end = end as u32; // Safe cast: validated max offset fits in u32 above
- builder
- .try_append_view(block, start, end - start)
- .expect("Failed to append view");
+ views.push(make_view(&buffer[start..end], 0, start as u32));
Review Comment:
buffer & offsets here is guaranteed to come from trusted/correct data?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(variant): build BinaryView arrays directly [arrow-rs]
Posted by "cakeni (via GitHub)" <gi...@apache.org>.
cakeni commented on code in PR #10640:
URL: https://github.com/apache/arrow-rs/pull/10640#discussion_r3832876007
##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -458,22 +460,21 @@ impl VariantBuilderExt for VariantValueArrayBuilderExt<'_> {
}
fn binary_view_array_from_buffers(buffer: Vec<u8>, offsets: Vec<usize>) -> BinaryViewArray {
- // All offsets are less than or equal to the buffer length, so we can safely cast all offsets
- // inside the loop below, as long as the buffer length fits in u32.
- u32::try_from(buffer.len()).expect("buffer length should fit in u32");
-
- let mut builder = BinaryViewBuilder::with_capacity(offsets.len());
- let block = builder.append_block(buffer.into());
- // TODO this can be much faster if it creates the views directly during append
- let mut start = 0;
+ // Each builder records the current buffer length after appending a row, so offsets are
+ // monotonically increasing and bounded by the final buffer length.
+ assert!(buffer.len() < u32::MAX as usize);
+
+ let buffer = Buffer::from(buffer);
+ let mut views = Vec::with_capacity(offsets.len());
+ let mut start = 0_usize;
for end in offsets {
- let end = end as u32; // Safe cast: validated max offset fits in u32 above
- builder
- .try_append_view(block, start, end - start)
- .expect("Failed to append view");
+ views.push(make_view(&buffer[start..end], 0, start as u32));
Review Comment:
Yes. Both come from the private builders here, and the offsets are recorded from the same buffer that `build()` consumes. Invalid ranges would panic on `buffer[start..end]` before reaching `new_unchecked`, so the required invariants are guaranteed.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(variant): build BinaryView arrays directly [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10640:
URL: https://github.com/apache/arrow-rs/pull/10640#issuecomment-5378953343
thanks @cakeni
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org