Rich-T-kid commented on code in PR #8716:
URL: https://github.com/apache/arrow-rs/pull/8716#discussion_r3633086080
##########
arrow-cast/src/cast/run_array.rs:
##########
@@ -169,3 +171,396 @@ pub(crate) fn cast_to_run_end_encoded<K: RunEndIndexType>(
let run_array = RunArray::<K>::try_new(&run_ends_array,
values_array.as_ref())?;
Ok(Arc::new(run_array))
}
+
+fn compute_run_boundaries(array: &ArrayRef) -> (Vec<usize>, Vec<usize>) {
+ if array.is_empty() {
+ return (Vec::new(), Vec::new());
+ }
+
+ use arrow_schema::DataType::*;
+
+ let array = array.as_ref();
+ downcast_primitive_array! {
+ array => runs_for_primitive(array),
+ Null => (vec![array.len()], vec![0]),
+ Boolean => runs_for_boolean(array.as_boolean()),
+ Utf8 => runs_for_string(array.as_string::<i32>()),
+ LargeUtf8 => runs_for_string(array.as_string::<i64>()),
+ Binary => runs_for_binary(array.as_binary::<i32>()),
+ LargeBinary => runs_for_binary(array.as_binary::<i64>()),
+ FixedSizeBinary(_) =>
runs_for_fixed_size_binary(array.as_fixed_size_binary()),
+ Dictionary(key_type, _) => match key_type.as_ref() {
+ Int8 => runs_for_dictionary::<Int8Type>(array.as_dictionary()),
+ Int16 => runs_for_dictionary::<Int16Type>(array.as_dictionary()),
+ Int32 => runs_for_dictionary::<Int32Type>(array.as_dictionary()),
+ Int64 => runs_for_dictionary::<Int64Type>(array.as_dictionary()),
+ UInt8 => runs_for_dictionary::<UInt8Type>(array.as_dictionary()),
+ UInt16 => runs_for_dictionary::<UInt16Type>(array.as_dictionary()),
+ UInt32 => runs_for_dictionary::<UInt32Type>(array.as_dictionary()),
+ UInt64 => runs_for_dictionary::<UInt64Type>(array.as_dictionary()),
+ _ => runs_generic(array),
+ },
+ _ => runs_generic(array),
+ }
+}
+
+fn runs_for_boolean(array: &BooleanArray) -> (Vec<usize>, Vec<usize>) {
+ let len = array.len();
+ if let Some(runs) = trivial_runs(len) {
+ return runs;
+ }
+
+ let mut run_boundaries = Vec::with_capacity(len / 64 + 2);
Review Comment:
Is there something special about `len/64 + 2` ? can we add a comment
explaining why this was choosen.
##########
arrow-cast/src/cast/run_array.rs:
##########
@@ -169,3 +171,396 @@ pub(crate) fn cast_to_run_end_encoded<K: RunEndIndexType>(
let run_array = RunArray::<K>::try_new(&run_ends_array,
values_array.as_ref())?;
Ok(Arc::new(run_array))
}
+
+fn compute_run_boundaries(array: &ArrayRef) -> (Vec<usize>, Vec<usize>) {
+ if array.is_empty() {
+ return (Vec::new(), Vec::new());
+ }
+
+ use arrow_schema::DataType::*;
+
+ let array = array.as_ref();
+ downcast_primitive_array! {
+ array => runs_for_primitive(array),
+ Null => (vec![array.len()], vec![0]),
+ Boolean => runs_for_boolean(array.as_boolean()),
+ Utf8 => runs_for_string(array.as_string::<i32>()),
+ LargeUtf8 => runs_for_string(array.as_string::<i64>()),
+ Binary => runs_for_binary(array.as_binary::<i32>()),
+ LargeBinary => runs_for_binary(array.as_binary::<i64>()),
+ FixedSizeBinary(_) =>
runs_for_fixed_size_binary(array.as_fixed_size_binary()),
+ Dictionary(key_type, _) => match key_type.as_ref() {
+ Int8 => runs_for_dictionary::<Int8Type>(array.as_dictionary()),
+ Int16 => runs_for_dictionary::<Int16Type>(array.as_dictionary()),
+ Int32 => runs_for_dictionary::<Int32Type>(array.as_dictionary()),
+ Int64 => runs_for_dictionary::<Int64Type>(array.as_dictionary()),
+ UInt8 => runs_for_dictionary::<UInt8Type>(array.as_dictionary()),
+ UInt16 => runs_for_dictionary::<UInt16Type>(array.as_dictionary()),
+ UInt32 => runs_for_dictionary::<UInt32Type>(array.as_dictionary()),
+ UInt64 => runs_for_dictionary::<UInt64Type>(array.as_dictionary()),
+ _ => runs_generic(array),
+ },
+ _ => runs_generic(array),
+ }
+}
+
+fn runs_for_boolean(array: &BooleanArray) -> (Vec<usize>, Vec<usize>) {
+ let len = array.len();
+ if let Some(runs) = trivial_runs(len) {
+ return runs;
+ }
+
+ let mut run_boundaries = Vec::with_capacity(len / 64 + 2);
Review Comment:
This is used repeatedly.
two constants with comments explaining why these numbers were chosen would
be nice
```rust
// ...
const BASE_DIVISOR = 64
// ...
const OFFSET = 2
```
##########
arrow-cast/src/cast/run_array.rs:
##########
@@ -169,3 +171,396 @@ pub(crate) fn cast_to_run_end_encoded<K: RunEndIndexType>(
let run_array = RunArray::<K>::try_new(&run_ends_array,
values_array.as_ref())?;
Ok(Arc::new(run_array))
}
+
+fn compute_run_boundaries(array: &ArrayRef) -> (Vec<usize>, Vec<usize>) {
+ if array.is_empty() {
+ return (Vec::new(), Vec::new());
+ }
+
+ use arrow_schema::DataType::*;
+
+ let array = array.as_ref();
+ downcast_primitive_array! {
+ array => runs_for_primitive(array),
+ Null => (vec![array.len()], vec![0]),
+ Boolean => runs_for_boolean(array.as_boolean()),
+ Utf8 => runs_for_string(array.as_string::<i32>()),
+ LargeUtf8 => runs_for_string(array.as_string::<i64>()),
+ Binary => runs_for_binary(array.as_binary::<i32>()),
+ LargeBinary => runs_for_binary(array.as_binary::<i64>()),
+ FixedSizeBinary(_) =>
runs_for_fixed_size_binary(array.as_fixed_size_binary()),
+ Dictionary(key_type, _) => match key_type.as_ref() {
+ Int8 => runs_for_dictionary::<Int8Type>(array.as_dictionary()),
+ Int16 => runs_for_dictionary::<Int16Type>(array.as_dictionary()),
+ Int32 => runs_for_dictionary::<Int32Type>(array.as_dictionary()),
+ Int64 => runs_for_dictionary::<Int64Type>(array.as_dictionary()),
+ UInt8 => runs_for_dictionary::<UInt8Type>(array.as_dictionary()),
+ UInt16 => runs_for_dictionary::<UInt16Type>(array.as_dictionary()),
+ UInt32 => runs_for_dictionary::<UInt32Type>(array.as_dictionary()),
+ UInt64 => runs_for_dictionary::<UInt64Type>(array.as_dictionary()),
+ _ => runs_generic(array),
+ },
+ _ => runs_generic(array),
+ }
+}
+
+fn runs_for_boolean(array: &BooleanArray) -> (Vec<usize>, Vec<usize>) {
+ let len = array.len();
+ if let Some(runs) = trivial_runs(len) {
+ return runs;
+ }
+
+ let mut run_boundaries = Vec::with_capacity(len / 64 + 2);
+ let mut current_valid = array.is_valid(0);
+ let mut current_value = if current_valid { array.value(0) } else { false };
+
+ for idx in 1..len {
+ // Treat a change in validity the same as a change in value so null
boundaries are recorded.
+ let valid = array.is_valid(idx);
+ let mut boundary = false;
+ if current_valid && valid {
+ let value = array.value(idx);
+ if value != current_value {
+ current_value = value;
+ boundary = true;
+ }
+ } else if current_valid != valid {
+ boundary = true;
+ if valid {
+ current_value = array.value(idx);
+ }
+ }
Review Comment:
@Weijun-H you mentioned
> The no-null primitive path uses scan_run_end, which compares 16 bytes at a
time via u128 loads. When a chunk differs, it falls back to scalar
iteration—reducing branches and bounds checks in the hot loop.
could there be a similar optimization for Boolean arrays where we read in
large chunks of the bit-packed buffer?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
