This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 1535dc4d38 perf(variant): build BinaryView arrays directly (#10640)
1535dc4d38 is described below
commit 1535dc4d3842d1d4eea74ffa975a3e11fea8e869
Author: cakeni <[email protected]>
AuthorDate: Sat Aug 22 15:12:22 2026 +0800
perf(variant): build BinaryView arrays directly (#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.
## AI assistance
I identified and evaluated this optimization, chose the final approach,
and reviewed and refined the implementation and benchmark. OpenAI Codex
assisted with code exploration, drafting, and benchmark preparation.
Co-authored-by: Jeffrey Vo <[email protected]>
---
parquet-variant-compute/benches/variant_kernels.rs | 20 ++++++++++++++-
.../src/variant_array_builder.rs | 29 +++++++++++-----------
2 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/parquet-variant-compute/benches/variant_kernels.rs
b/parquet-variant-compute/benches/variant_kernels.rs
index 800633b571..7fec1ed786 100644
--- a/parquet-variant-compute/benches/variant_kernels.rs
+++ b/parquet-variant-compute/benches/variant_kernels.rs
@@ -18,7 +18,7 @@
use arrow::array::{Array, ArrayRef, BinaryViewArray, BinaryViewBuilder,
StringArray, StructArray};
use arrow::buffer::Buffer;
use arrow_schema::{DataType, Field, FieldRef, Fields};
-use criterion::{Criterion, criterion_group, criterion_main};
+use criterion::{BatchSize, Criterion, criterion_group, criterion_main};
use parquet_variant::{EMPTY_VARIANT_METADATA_BYTES, Variant, VariantBuilder,
VariantPath};
use parquet_variant_compute::{
GetOptions, VariantArray, VariantArrayBuilder, json_to_variant,
variant_get,
@@ -33,6 +33,23 @@ use std::fmt::Write;
use std::sync::Arc;
const VARIANT_GET_UNSHREDDED_OBJECT_ROWS: usize = 262_144;
+const VARIANT_ARRAY_BUILD_ROWS: usize = 262_144;
+
+fn variant_array_builder_build_bench(c: &mut Criterion) {
+ c.bench_function("variant_array_builder_build_262k_small_values", |b| {
+ b.iter_batched(
+ || {
+ let mut builder =
VariantArrayBuilder::new(VARIANT_ARRAY_BUILD_ROWS);
+ for value in 0..VARIANT_ARRAY_BUILD_ROWS {
+ builder.append_variant(Variant::Int8((value % 128) as i8));
+ }
+ builder
+ },
+ |builder| std::hint::black_box(builder.build()),
+ BatchSize::LargeInput,
+ )
+ });
+}
fn benchmark_batch_json_string_to_variant(c: &mut Criterion) {
let input_array =
StringArray::from_iter_values(json_repeated_struct(8000));
@@ -189,6 +206,7 @@ criterion_group!(
variant_get_bench,
variant_get_shredded_utf8_bench,
variant_get_unshredded_object_path_bench,
+ variant_array_builder_build_bench,
benchmark_batch_json_string_to_variant
);
criterion_main!(benches);
diff --git a/parquet-variant-compute/src/variant_array_builder.rs
b/parquet-variant-compute/src/variant_array_builder.rs
index 3dd9714ce2..d7bcd08937 100644
--- a/parquet-variant-compute/src/variant_array_builder.rs
+++ b/parquet-variant-compute/src/variant_array_builder.rs
@@ -18,7 +18,9 @@
//! [`VariantArrayBuilder`] implementation
use crate::VariantArray;
-use arrow::array::{ArrayRef, BinaryViewArray, BinaryViewBuilder,
NullBufferBuilder, StructArray};
+use arrow::array::builder::make_view;
+use arrow::array::{ArrayRef, BinaryViewArray, NullBufferBuilder, StructArray};
+use arrow::buffer::Buffer;
use arrow_schema::{ArrowError, DataType, Field, Fields};
use parquet_variant::{
BuilderSpecificState, ListBuilder, MetadataBuilder, ObjectBuilder,
Variant, VariantBuilderExt,
@@ -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));
start = end;
}
- builder.finish()
+
+ // SAFETY: `make_view` constructs every view from an in-bounds slice of
buffer 0, and there
+ // are no nulls. The buffer length check above guarantees every offset
fits in a `u32`.
+ unsafe { BinaryViewArray::new_unchecked(views.into(), vec![buffer], None) }
}
#[cfg(test)]
