This is an automated email from the ASF dual-hosted git repository.

etseidl 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 3566d07ee4 perf(parquet): scan DELTA_BYTE_ARRAY shared prefixes a 
block at a time (#10549)
3566d07ee4 is described below

commit 3566d07ee451686d1d0f2ae2c46f7f0e7b4d6aaf
Author: Adrian Garcia Badaracco <[email protected]>
AuthorDate: Tue Aug 4 15:29:02 2026 -0500

    perf(parquet): scan DELTA_BYTE_ARRAY shared prefixes a block at a time 
(#10549)
    
    > **Stacked on https://github.com/apache/arrow-rs/pull/10550.** This
    branch is
    > that PR's benchmark commit plus one commit of its own, so that the
    numbers
    > below are reproducible with `cargo bench` on this branch alone. Review
    only
    > the second commit here; the diff shrinks to
    > `+139/-15` once #10550 merges and this rebases onto `main`.
    
    # Which issue does this PR close?
    
    None directly. Split out of
    https://github.com/apache/arrow-rs/pull/10505 so that the correctness
    fix there can be reviewed without an unrelated performance change
    attached to it.
    
    # Rationale for this change
    
    `DELTA_BYTE_ARRAY` stores each value as the number of leading bytes it
    shares with its predecessor plus the remaining suffix, so writing a
    value runs a shared-prefix scan against the previous value. Both encoder
    paths implement that scan as a byte-at-a-time loop:
    
    - `DeltaByteArrayEncoder::put` in
    `parquet/src/encodings/encoding/mod.rs` (the generic
    `SerializedFileWriter` path)
    - `FallbackEncoder::encode`'s `Delta` arm in
    `parquet/src/arrow/arrow_writer/byte_array.rs` (the `ArrowWriter` path)
    
    The scan runs once per value, and on exactly the data the encoding
    exists for — near-identical consecutive values — it covers essentially
    the whole value. That makes its throughput, not its per-call overhead,
    the thing that matters, and a byte-at-a-time loop is the slowest way to
    do it.
    
    # What changes are included in this PR?
    
    Extract the two duplicated loops into
    `crate::util::prefix::common_prefix_length` and compare a 32-byte block
    at a time instead of a byte at a time.
    
    32 is the widest block that both aarch64 and x86-64 still expand inline;
    at 64 bytes x86-64 drops to an out-of-line `bcmp` call, which costs more
    than the extra width buys. Measured on aarch64, every width from 16 up
    performs the same, so this sits in the middle of a flat optimum rather
    than on a tuned peak.
    
    No behavior change: the function returns the same prefix length the
    byte-wise loops did, and no page layout, encoding, or file output
    changes.
    
    # Are these changes tested?
    
    Existing coverage: the full `parquet` suite passes unmodified (1307
    tests), including the `DELTA_BYTE_ARRAY` round-trip tests.
    
    Those round trips are weaker evidence than they look, in two ways.
    First, they never reach the new code path: `ByteArrayType::test` and
    `FixedLenByteArrayType::test` feed random values, which share no prefix,
    and the `ArrowWriter` cases write values a handful of bytes long — so
    nothing in the suite writes two consecutive values sharing 32 bytes, and
    the block loop never runs. Second, a round trip is structurally blind to
    an under-counted prefix: the encoder just emits a correspondingly longer
    suffix and the decoder reconstructs the same bytes either way. Only
    over-counting shows up. `test_estimated_data_encoded_size` does assert
    an exact encoded size, but on 2- and 3-byte values that can never enter
    the block loop.
    
    New coverage, unit level, in `parquet/src/util/prefix.rs`: the boundary
    cases the block loop introduces — empty inputs, prefixes shorter than /
    equal to / longer than one block, a mismatch in the first and last byte
    of a block, and unequal lengths where one input is a strict prefix of
    the other. Plus unequal lengths combined with a mismatch past a block
    boundary (where truncating to the shorter length interacts with the
    block loop), non-zero slice start offsets (callers pass sub-slices into
    shared Arrow buffers, not freshly allocated `Vec`s), and a prefix ending
    mid-UTF-8-codepoint — byte-level prefixes may split a multi-byte
    character, which was true of the byte-wise scan too and is worth pinning
    now that the scan is wider.
    
    New coverage, end to end, for both call sites:
    `test_delta_byte_array_long_shared_prefix{,_fixed_len}` in
    `parquet/src/encodings/encoding/mod.rs` and
    `delta_byte_array_long_shared_prefix` in
    `parquet/src/arrow/arrow_writer/mod.rs`. Each writes values with a
    1000-byte shared prefix — not a multiple of the 32-byte block, so the
    scan has to resolve a partial block — and asserts on the prefix lengths
    actually written, decoded back out of the page, rather than on a round
    trip alone.
    
    Those assertions were checked for power by mutation: dropping the
    sub-block tail scan from `common_prefix_length` (so it under-counts by
    up to 31 bytes) leaves all 156 `arrow::arrow_writer` tests passing on
    `main`, and fails all three new tests.
    
    Benchmarked with `parquet/benches/arrow_writer.rs`'s
    `bench_delta_byte_array_writers`, added in #10512. Results in a comment
    below.
    
    # Are there any user-facing changes?
    
    No API changes and no change to written output. `DELTA_BYTE_ARRAY`
    writes get faster.
    
    ---------
    
    Co-authored-by: Claude Opus 5 <[email protected]>
    Co-authored-by: Ed Seidl <[email protected]>
---
 parquet/src/arrow/arrow_writer/byte_array.rs |  10 +-
 parquet/src/encodings/encoding/mod.rs        |  10 +-
 parquet/src/util/mod.rs                      |   1 +
 parquet/src/util/prefix.rs                   | 133 +++++++++++++++++++++++++++
 4 files changed, 139 insertions(+), 15 deletions(-)

diff --git a/parquet/src/arrow/arrow_writer/byte_array.rs 
b/parquet/src/arrow/arrow_writer/byte_array.rs
index ea45edeea2..93a16f7693 100644
--- a/parquet/src/arrow/arrow_writer/byte_array.rs
+++ b/parquet/src/arrow/arrow_writer/byte_array.rs
@@ -30,6 +30,7 @@ use crate::geospatial::statistics::GeospatialStatistics;
 use crate::schema::types::ColumnDescPtr;
 use crate::util::bit_util::num_required_bits;
 use crate::util::interner::{Interner, Storage};
+use crate::util::prefix::common_prefix_length;
 use arrow_array::types::ByteArrayType;
 use arrow_array::{
     Array, ArrayAccessor, BinaryArray, BinaryViewArray, DictionaryArray, 
FixedSizeBinaryArray,
@@ -201,15 +202,8 @@ impl FallbackEncoder {
                 for idx in indices {
                     let value = values.value(idx);
                     let value = value.as_ref();
-                    let mut prefix_length = 0;
-
-                    while prefix_length < last_value.len()
-                        && prefix_length < value.len()
-                        && last_value[prefix_length] == value[prefix_length]
-                    {
-                        prefix_length += 1;
-                    }
 
+                    let prefix_length = common_prefix_length(last_value, 
value);
                     let suffix_length = value.len() - prefix_length;
 
                     last_value.clear();
diff --git a/parquet/src/encodings/encoding/mod.rs 
b/parquet/src/encodings/encoding/mod.rs
index eeabcf4ba5..06ad3ac10e 100644
--- a/parquet/src/encodings/encoding/mod.rs
+++ b/parquet/src/encodings/encoding/mod.rs
@@ -26,6 +26,7 @@ use crate::encodings::rle::RleEncoder;
 use crate::errors::{ParquetError, Result};
 use crate::schema::types::ColumnDescPtr;
 use crate::util::bit_util::{BitWriter, num_required_bits};
+use crate::util::prefix::common_prefix_length;
 
 use byte_stream_split_encoder::{ByteStreamSplitEncoder, 
VariableWidthByteStreamSplitEncoder};
 use bytes::Bytes;
@@ -698,13 +699,8 @@ impl<T: DataType> Encoder<T> for DeltaByteArrayEncoder<T> {
 
         for byte_array in values {
             let current = byte_array.data();
-            // Maximum prefix length that is shared between previous value and 
current
-            // value
-            let prefix_len = cmp::min(self.previous.len(), current.len());
-            let mut match_len = 0;
-            while match_len < prefix_len && self.previous[match_len] == 
current[match_len] {
-                match_len += 1;
-            }
+            // Number of leading bytes shared with the previous value
+            let match_len = common_prefix_length(&self.previous, current);
             prefix_lengths.push(match_len as i32);
             suffixes.push(byte_array.slice(match_len, byte_array.len() - 
match_len));
             // Update previous for the next prefix
diff --git a/parquet/src/util/mod.rs b/parquet/src/util/mod.rs
index 145cdd693e..4d54975313 100644
--- a/parquet/src/util/mod.rs
+++ b/parquet/src/util/mod.rs
@@ -19,6 +19,7 @@
 pub mod bit_util;
 mod bit_pack;
 pub(crate) mod interner;
+pub(crate) mod prefix;
 
 pub mod push_buffers;
 #[cfg(any(test, feature = "test_common"))]
diff --git a/parquet/src/util/prefix.rs b/parquet/src/util/prefix.rs
new file mode 100644
index 0000000000..1e2c743aac
--- /dev/null
+++ b/parquet/src/util/prefix.rs
@@ -0,0 +1,133 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Shared-prefix scanning for `DELTA_BYTE_ARRAY`.
+
+/// Returns the length in bytes of the longest common prefix of `a` and `b`.
+///
+/// `DELTA_BYTE_ARRAY` stores each value as the number of leading bytes it
+/// shares with its predecessor plus the remaining suffix, so this runs once
+/// per value written with that encoding. When consecutive values are
+/// near-identical — the case the encoding exists for — the scan covers
+/// essentially the whole value, which makes its throughput, not its overhead,
+/// the thing that matters.
+#[inline]
+pub(crate) fn common_prefix_length(a: &[u8], b: &[u8]) -> usize {
+    // Comparing a block at a time rather than a byte at a time is what keeps
+    // this off the critical path. 32 is the widest block that both aarch64
+    // and x86-64 still expand inline: at 64 bytes x86-64 drops to an
+    // out-of-line `bcmp` call, which costs more than the extra width buys.
+    // Measured on aarch64, every width from 16 up performs the same (~15x a
+    // byte-wise loop over a 2 MiB shared prefix), so this sits in the middle
+    // of a flat optimum rather than on a tuned peak.
+    const BLOCK: usize = 32;
+
+    let n = a.len().min(b.len());
+    let (a, b) = (&a[..n], &b[..n]);
+
+    let mut matched = 0;
+    for (x, y) in a.chunks_exact(BLOCK).zip(b.chunks_exact(BLOCK)) {
+        if x != y {
+            break;
+        }
+        matched += BLOCK;
+    }
+
+    // At most one block plus the sub-block tail is left unresolved: either the
+    // block the loop stopped on, or the remainder `chunks_exact` never 
yielded.
+    matched
+        + a[matched..]
+            .iter()
+            .zip(&b[matched..])
+            .take_while(|(x, y)| x == y)
+            .count()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The definition, kept deliberately naive, to check the fast path 
against.
+    fn naive(a: &[u8], b: &[u8]) -> usize {
+        let mut i = 0;
+        while i < a.len() && i < b.len() && a[i] == b[i] {
+            i += 1;
+        }
+        i
+    }
+
+    #[test]
+    fn test_common_prefix_length_edge_cases() {
+        assert_eq!(common_prefix_length(b"", b""), 0);
+        assert_eq!(common_prefix_length(b"", b"abc"), 0);
+        assert_eq!(common_prefix_length(b"abc", b""), 0);
+        assert_eq!(common_prefix_length(b"abc", b"xyz"), 0);
+        assert_eq!(common_prefix_length(b"abc", b"abc"), 3);
+        // One value a strict prefix of the other, in both orders.
+        assert_eq!(common_prefix_length(b"abc", b"abcdef"), 3);
+        assert_eq!(common_prefix_length(b"abcdef", b"abc"), 3);
+    }
+
+    #[test]
+    fn test_common_prefix_length_around_block_boundaries() {
+        // Mismatches placed on, either side of, and well past the 32-byte
+        // block boundary the scan steps in.
+        for len in [31, 32, 33, 63, 64, 65, 127, 128, 129, 1024] {
+            for mismatch in 0..=len {
+                let a = vec![b'x'; len];
+                let mut b = a.clone();
+                if mismatch < len {
+                    b[mismatch] = b'y';
+                }
+                let expected = if mismatch < len { mismatch } else { len };
+                assert_eq!(
+                    common_prefix_length(&a, &b),
+                    expected,
+                    "len={len} mismatch={mismatch}"
+                );
+                assert_eq!(common_prefix_length(&a, &b), naive(&a, &b));
+            }
+        }
+    }
+
+    #[test]
+    fn test_common_prefix_length_unequal_lengths() {
+        // Result is capped by the shorter value even when the longer one
+        // continues to match, across block boundaries.
+        for a_len in 0..80usize {
+            for b_len in 0..80usize {
+                let a = vec![b'x'; a_len];
+                let b = vec![b'x'; b_len];
+                assert_eq!(common_prefix_length(&a, &b), a_len.min(b_len));
+                assert_eq!(common_prefix_length(&a, &b), naive(&a, &b));
+            }
+        }
+    }
+
+    #[test]
+    fn test_common_prefix_length_matches_naive_on_varied_data() {
+        // Non-uniform bytes, so a block compare cannot accidentally succeed
+        // on data a byte-wise scan would reject.
+        let a: Vec<u8> = (0..500u32).map(|i| (i * 7 % 251) as u8).collect();
+        for mismatch in 0..a.len() {
+            let mut b = a.clone();
+            b[mismatch] = b[mismatch].wrapping_add(1);
+            assert_eq!(common_prefix_length(&a, &b), naive(&a, &b));
+            assert_eq!(common_prefix_length(&a, &b), mismatch);
+        }
+    }
+}

Reply via email to