This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24071-f6d4f04dc419a5f7b7fcc374e468a43026a66221 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit d813358848a555474a7af25c79ead10ba1c68cac Author: JS <[email protected]> AuthorDate: Tue Aug 4 22:52:30 2026 +0200 perf: use Vec in ArrowBytesMap (#24071) ## Which issue does this PR close? - Closes #13867. ## Rationale for this change `ArrowBytesMap` only requires growable byte storage while values are being inserted. `BufferBuilder<u8>` wraps `MutableBuffer`, whose allocations use 64-byte alignment. This alignment is unnecessary for byte storage and makes buffer growth more expensive. A `Vec<u8>` provides the required append, lookup, length, and capacity operations. When the map is materialized, `Buffer::from_vec` transfers the allocation into an Arrow `Buffer` without copying it. ## What changes are included in this PR? - Replace the internal `BufferBuilder<u8>` in `ArrowBytesMap` with `Vec<u8>`. - Use `extend_from_slice` when storing new values. - Convert the completed `Vec<u8>` into an Arrow `Buffer` without copying. - Add a focused Criterion benchmark covering: - unique 4-byte values; - unique 32-byte values; - 32-byte values with low cardinality. This PR intentionally changes only `ArrowBytesMap`. The other structures mentioned in #13867 are left for separate follow-up PRs. ## Are these changes tested? Yes. Validation performed on Ubuntu 24.04 under WSL2 with Rust 1.97.0: - `cargo fmt --all --check` - `cargo clippy --all-targets --all-features -- -D warnings` - `cargo test -p datafusion-physical-expr-common --all-features` - 80 unit tests passed - 8 doctests passed - `cargo test -p datafusion-physical-plan group_values` - 65 tests passed ### Benchmark results Criterion comparison against the `BufferBuilder<u8>` implementation at commit `f9dde71ec`, using 100 samples, a 3-second warm-up, and a 5-second measurement period: | Benchmark | BufferBuilder | Vec | Criterion result | |---|---:|---:|---| | `short_unique` | 306.39 us | 302.80 us | Within noise threshold | | `long_unique` | 410.23 us | 159.08 us | 61.24% lower time | | `long_low_cardinality` | 49.90 us | 48.97 us | No change detected | For `long_unique`, throughput increased by approximately 158%. Repeating the comparison with the execution order reversed produced approximately 158.57 us for `Vec` and 440.05 us for `BufferBuilder`. No stable performance regression was observed in the short-value or low-cardinality cases. ## Are there any user-facing changes? No. This is an internal implementation and performance change with no public API or behavior changes. Co-authored-by: Andrew Lamb <[email protected]> --- datafusion/physical-expr-common/src/binary_map.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/datafusion/physical-expr-common/src/binary_map.rs b/datafusion/physical-expr-common/src/binary_map.rs index ad184d6500..44ca35c7f8 100644 --- a/datafusion/physical-expr-common/src/binary_map.rs +++ b/datafusion/physical-expr-common/src/binary_map.rs @@ -19,12 +19,12 @@ //! StringArray / LargeStringArray / BinaryArray / LargeBinaryArray. use arrow::array::{ - Array, ArrayRef, BufferBuilder, GenericBinaryArray, GenericStringArray, - NullBufferBuilder, OffsetSizeTrait, + Array, ArrayRef, GenericBinaryArray, GenericStringArray, NullBufferBuilder, + OffsetSizeTrait, cast::AsArray, types::{ByteArrayType, GenericBinaryType, GenericStringType}, }; -use arrow::buffer::{NullBuffer, OffsetBuffer, ScalarBuffer}; +use arrow::buffer::{Buffer, NullBuffer, OffsetBuffer, ScalarBuffer}; use arrow::datatypes::DataType; use datafusion_common::hash_utils::RandomState; use datafusion_common::hash_utils::create_hashes; @@ -218,8 +218,8 @@ where map: hashbrown::hash_table::HashTable<Entry<O, V>>, /// Total size of the map in bytes map_size: usize, - /// In progress arrow `Buffer` containing all values - buffer: BufferBuilder<u8>, + /// In progress buffer containing all values + buffer: Vec<u8>, /// Offsets into `buffer` for each distinct value. These offsets as used /// directly to create the final `GenericBinaryArray`. The `i`th string is /// stored in the range `offsets[i]..offsets[i+1]` in `buffer`. Null values @@ -248,7 +248,7 @@ where output_type, map: hashbrown::hash_table::HashTable::with_capacity(INITIAL_MAP_CAPACITY), map_size: 0, - buffer: BufferBuilder::new(INITIAL_BUFFER_CAPACITY), + buffer: Vec::with_capacity(INITIAL_BUFFER_CAPACITY), offsets: vec![O::default()], // first offset is always 0 random_state: RandomState::default(), hashes_buffer: vec![], @@ -405,7 +405,7 @@ where // Put the small values into buffer and offsets so it appears // the output array, but store the actual bytes inline for // comparison - self.buffer.append_slice(value); + self.buffer.extend_from_slice(value); self.offsets.push(O::usize_as(self.buffer.len())); let payload = make_payload_fn(Some(value)); let new_header = Entry { @@ -433,7 +433,7 @@ where // Need to compare the bytes in the buffer // SAFETY: buffer is only appended to, and we correctly inserted values and offsets let existing_value = - unsafe { self.buffer.as_slice().get_unchecked(header.range()) }; + unsafe { self.buffer.get_unchecked(header.range()) }; value == existing_value }); @@ -446,7 +446,7 @@ where // appears the output array, and store that offset // so the bytes can be compared if needed let offset = self.buffer.len(); // offset of start for data - self.buffer.append_slice(value); + self.buffer.extend_from_slice(value); self.offsets.push(O::usize_as(self.buffer.len())); let payload = make_payload_fn(Some(value)); @@ -488,7 +488,7 @@ where map: _, map_size: _, offsets, - mut buffer, + buffer, random_state: _, hashes_buffer: _, null, @@ -502,7 +502,7 @@ where // SAFETY: the offsets were constructed correctly in `insert_if_new` -- // monotonically increasing, overflows were checked. let offsets = unsafe { OffsetBuffer::new_unchecked(ScalarBuffer::from(offsets)) }; - let values = buffer.finish(); + let values = Buffer::from_vec(buffer); match output_type { OutputType::Binary => { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
