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 ec21627533 perf(arrow-array): speed up `RunArray::try_new` by avoiding 
the `ArrayData` roundtrip (#10807)
ec21627533 is described below

commit ec21627533987201d690691918da21f152950a50
Author: Liam Bao <[email protected]>
AuthorDate: Thu Sep 3 06:59:04 2026 -0700

    perf(arrow-array): speed up `RunArray::try_new` by avoiding the `ArrayData` 
roundtrip (#10807)
    
    # Which issue does this PR close?
    
    <!--
    We generally require a GitHub issue to be filed for all bug fixes and
    enhancements and this helps us generate change logs for our releases.
    You can link an issue to this PR using the GitHub syntax.
    -->
    
    - Part of #9298.
    
    # Rationale for this change
    
    <!--
    Why are you proposing this change? If this is already explained clearly
    in the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand
    your changes and offer better suggestions for fixes.
    -->
    
    # What changes are included in this PR?
    
    <!--
    There is no need to duplicate the description in the issue here but it
    is sometimes worth providing a summary of the individual changes in this
    PR.
    -->
    
    Eliminate the `make_array` roundtrip in `RunArray::try_new` to improve
    performance
    
    # Are these changes tested?
    
    <!--
    We typically require tests for all PRs in order to:
    1. Prevent the code from being accidentally broken by subsequent changes
    2. Serve as another way to document the expected behavior of the code
    
    If tests are not included in your PR, please explain why (for example,
    are they covered by existing tests)?
    
    If this PR claims a performance improvement, please include evidence
    such as benchmark results.
    -->
    Correctness is ensured by existing tests
    
    Bench is extracted #10809. Local benchmark gave the results below:
    | Bench | Before | After | Change |
    |---|---|---|---|
    | i32, 256 | 497 ns | 222 ns | −59% |
    | utf8, 256 | 585 ns | 246 ns | −56% |
    | i32, 1024 | 999 ns | 664 ns | −37% |
    | utf8, 1024 | 988 ns | 518 ns | −48% |
    | i32, 4096 | 1.98 µs | 1.80 µs | −9% |
    | utf8, 4096 | 2.01 µs | 1.79 µs | −11% |
    | i32, 8192 | 3.64 µs | 3.50 µs | −4% |
    | utf8, 8192 | 3.66 µs | 3.49 µs | −5% |
    
    # Are there any user-facing changes?
    
    <!--
    If there are user-facing changes then we may require documentation to be
    updated before approving the PR.
    
    If there are any breaking changes to public APIs, please call them out.
    -->
    
    No
    
    ---------
    
    Co-authored-by: Jefffrey <[email protected]>
---
 arrow-array/src/array/run_array.rs | 70 ++++++++++++++++++++++++++------------
 1 file changed, 49 insertions(+), 21 deletions(-)

diff --git a/arrow-array/src/array/run_array.rs 
b/arrow-array/src/array/run_array.rs
index d4eed52134..7255eb50d3 100644
--- a/arrow-array/src/array/run_array.rs
+++ b/arrow-array/src/array/run_array.rs
@@ -116,30 +116,58 @@ impl<R: RunEndIndexType> RunArray<R> {
     /// - If `run_ends` has any null values
     /// - If `run_ends` doesn't consist of strictly increasing positive 
integers
     pub fn try_new(run_ends: &PrimitiveArray<R>, values: &dyn Array) -> 
Result<Self, ArrowError> {
-        let run_ends_type = run_ends.data_type().clone();
-        let values_type = values.data_type().clone();
-        let ree_array_type = DataType::RunEndEncoded(
-            Arc::new(Field::new("run_ends", run_ends_type, false)),
-            Arc::new(Field::new("values", values_type, true)),
-        );
-        let len = RunArray::logical_len(run_ends);
-        let builder = ArrayDataBuilder::new(ree_array_type)
-            .len(len)
-            .add_child_data(run_ends.to_data())
-            .add_child_data(values.to_data());
+        // 1. run_ends and values must have the same (physical) length.
+        if run_ends.len() != values.len() {
+            return Err(ArrowError::InvalidArgumentError(format!(
+                "The run_ends array length should be the same as values array 
length. Run_ends array length is {}, values array length is {}",
+                run_ends.len(),
+                values.len()
+            )));
+        }
+
+        // 2. run_ends must not contain null values.
+        if run_ends.nulls().is_some() {
+            return Err(ArrowError::InvalidArgumentError(
+                "Found null values in run_ends array. The run_ends array 
should not have null values."
+                    .to_string(),
+            ));
+        }
+
+        // 3. run_ends must be strictly increasing, strictly positive integers.
+        if let Some(first) = run_ends.values().first() {
+            let mut prev_value = *first;
+            if prev_value <= R::Native::usize_as(0) {
+                return Err(ArrowError::InvalidArgumentError(format!(
+                    "The values in run_ends array should be strictly positive. 
Found value {prev_value:?} at index 0 that does not match the criteria."
+                )));
+            }
 
-        // `build_unchecked` is used to avoid recursive validation of child 
arrays.
-        let array_data = unsafe { builder.build_unchecked() };
+            for (ix, &run_end) in run_ends.values().iter().enumerate().skip(1) 
{
+                if run_end <= prev_value {
+                    return Err(ArrowError::InvalidArgumentError(format!(
+                        "The values in run_ends array should be strictly 
increasing. Found value {run_end:?} at index {ix} with previous value 
{prev_value:?} that does not match the criteria."
+                    )));
+                }
+                prev_value = run_end;
+            }
+        }
 
-        // Safety: `validate_data` checks below
-        //    1. The given array data has exactly two child arrays.
-        //    2. The first child array (run_ends) has valid data type.
-        //    3. run_ends array does not have null values
-        //    4. run_ends array has non-zero and strictly increasing values.
-        //    5. The length of run_ends array and values array are the same.
-        array_data.validate_data()?;
+        let data_type = DataType::RunEndEncoded(
+            Arc::new(Field::new("run_ends", run_ends.data_type().clone(), 
false)),
+            Arc::new(Field::new("values", values.data_type().clone(), true)),
+        );
 
-        Ok(array_data.into())
+        let logical_len = RunArray::logical_len(run_ends);
+        // Safety: validated above that the run ends are strictly increasing, 
strictly
+        // positive integers, so the last value equals the logical length.
+        let run_ends_buffer =
+            unsafe { RunEndBuffer::new_unchecked(run_ends.values().clone(), 0, 
logical_len) };
+
+        Ok(Self {
+            data_type,
+            run_ends: run_ends_buffer,
+            values: values.slice(0, values.len()),
+        })
     }
 
     /// Create a new [`RunArray`] from the provided parts, without validation

Reply via email to