perf: Optimize NULL handling in substr (#21519)

## Which issue does this PR close?

- Closes #21518.

## Rationale for this change

Similar to other recent changes, `substr` currently checks for NULLs and
builds the result NULL bitmap on a per-row basis. It is faster to
instead compute the result NULL bitmap in bulk via bitwise AND.

Benchmarks (ARM64):
```
  - substr, no count, short strings/substr_large_string [size=1024]: 21.4µs → 20.9µs (-2.3%)
  - substr, no count, short strings/substr_large_string [size=4096]: 83.1µs → 83.0µs (-0.1%)
  - substr, no count, short strings/substr_string [size=1024]: 20.5µs → 19.8µs (-3.4%)
  - substr, no count, short strings/substr_string [size=4096]: 78.8µs → 77.0µs (-2.3%)
  - substr, no count, short strings/substr_string_view [size=1024]: 18.9µs → 16.1µs (-14.8%)
  - substr, no count, short strings/substr_string_view [size=4096]: 74.0µs → 61.6µs (-16.8%)
  - substr, scalar args, long strings/substr_large_string [size=1024]: 35.2µs → 34.0µs (-3.4%)
  - substr, scalar args, long strings/substr_large_string [size=4096]: 140.6µs → 134.5µs (-4.3%)
  - substr, scalar args, long strings/substr_string [size=1024]: 35.5µs → 33.8µs (-4.8%)
  - substr, scalar args, long strings/substr_string [size=4096]: 138.9µs → 134.2µs (-3.4%)
  - substr, scalar args, long strings/substr_string_view [size=1024]: 34.0µs → 31.0µs (-8.8%)
  - substr, scalar args, long strings/substr_string_view [size=4096]: 132.0µs → 121.8µs (-7.7%)
  - substr, scalar args, short strings/substr_string [size=1024]: 31.0µs → 29.2µs (-5.8%)
  - substr, scalar args, short strings/substr_string [size=4096]: 120.8µs → 111.5µs (-7.7%)
  - substr, scalar args, short strings/substr_string_view [size=1024]: 26.8µs → 23.1µs (-13.8%)
  - substr, scalar args, short strings/substr_string_view [size=4096]: 101.6µs → 86.4µs (-14.9%)
  - substr, scalar start, no count, long strings/substr_string [size=1024]: 34.5µs → 33.2µs (-3.8%)
  - substr, scalar start, no count, long strings/substr_string [size=4096]: 134.4µs → 133.6µs (-0.6%)
  - substr, scalar start, no count, long strings/substr_string_view [size=1024]: 32.9µs → 29.4µs (-10.6%)
  - substr, scalar start, no count, long strings/substr_string_view [size=4096]: 126.1µs → 115.2µs (-8.6%)
  - substr, scalar start, no count, short strings/substr_string [size=1024]: 20.9µs → 20.1µs (-3.8%)
  - substr, scalar start, no count, short strings/substr_string [size=4096]: 80.1µs → 77.5µs (-3.2%)
  - substr, scalar start, no count, short strings/substr_string_view [size=1024]: 19.9µs → 16.7µs (-16.1%)
  - substr, scalar start, no count, short strings/substr_string_view [size=4096]: 74.4µs → 62.4µs (-16.1%)
  - substr, short count, long strings/substr_large_string [size=1024]: 30.3µs → 28.4µs (-6.3%)
  - substr, short count, long strings/substr_large_string [size=4096]: 117.1µs → 112.0µs (-4.4%)
  - substr, short count, long strings/substr_string [size=1024]: 30.2µs → 28.3µs (-6.3%)
  - substr, short count, long strings/substr_string [size=4096]: 118.0µs → 111.0µs (-5.9%)
  - substr, short count, long strings/substr_string_view [size=1024]: 26.1µs → 22.8µs (-12.6%)
  - substr, short count, long strings/substr_string_view [size=4096]: 101.5µs → 87.7µs (-13.6%)
  - substr, with count, long strings/substr_large_string [size=1024]: 34.6µs → 32.8µs (-5.2%)
  - substr, with count, long strings/substr_large_string [size=4096]: 136.7µs → 133.0µs (-2.7%)
  - substr, with count, long strings/substr_string [size=1024]: 34.2µs → 32.7µs (-4.4%)
  - substr, with count, long strings/substr_string [size=4096]: 136.6µs → 132.3µs (-3.1%)
  - substr, with count, long strings/substr_string_view [size=1024]: 33.3µs → 30.3µs (-9.0%)
  - substr, with count, long strings/substr_string_view [size=4096]: 129.1µs → 119.6µs (-7.4%)
```

## What changes are included in this PR?

* Implement optimization
* Rename `make_and_append_view` to `append_view`, and have callers deal
with NULL handling; making it part of `append_view` encourages per-row
NULL computations, which should be avoided when possible.
* Mark `append_view` as never-inline; this avoids a performance
regression on some of the `substr` microbenchmarks, where LLVM is a
little eager to inline a large-ish function into a hot loop.

## Are these changes tested?

Yes.

## Are there any user-facing changes?

No.

Author: neilconway

datafusion/functions/src/string/common.rs

@@ -19,7 +19,7 @@
 
 use std::sync::Arc;
 
-use crate::strings::make_and_append_view;
+use crate::strings::append_view;
 use arrow::array::{
     Array, ArrayRef, GenericStringArray, GenericStringBuilder, NullBufferBuilder,
     OffsetSizeTrait, StringViewArray, StringViewBuilder, new_null_array,
@@ -152,13 +152,8 @@ fn string_view_trim<Tr: Trimmer>(args: &[ArrayRef]) -> Result<ArrayRef> {
             {
                 if let Some(src_str) = src_str_opt {
                     let (trimmed, offset) = Tr::trim_ascii_char(src_str, b' ');
-                    make_and_append_view(
-                        &mut views_buf,
-                        &mut null_builder,
-                        raw_view,
-                        trimmed,
-                        offset,
-                    );
+                    append_view(&mut views_buf, raw_view, trimmed, offset);
+                    null_builder.append_non_null();
                 } else {
                     null_builder.append_null();
                     views_buf.push(0);
@@ -204,13 +199,8 @@ fn string_view_trim<Tr: Trimmer>(args: &[ArrayRef]) -> Result<ArrayRef> {
                         pattern.clear();
                         pattern.extend(characters.chars());
                         let (trimmed, offset) = Tr::trim(src_str, &pattern);
-                        make_and_append_view(
-                            &mut views_buf,
-                            &mut null_builder,
-                            raw_view,
-                            trimmed,
-                            offset,
-                        );
+                        append_view(&mut views_buf, raw_view, trimmed, offset);
+                        null_builder.append_non_null();
                     } else {
                         null_builder.append_null();
                         views_buf.push(0);
@@ -261,7 +251,8 @@ fn trim_and_append_view<Tr: Trimmer>(
 ) {
     if let Some(src_str) = src_str_opt {
         let (trimmed, offset) = Tr::trim(src_str, pattern);
-        make_and_append_view(views_buf, null_builder, original_view, trimmed, offset);
+        append_view(views_buf, original_view, trimmed, offset);
+        null_builder.append_non_null();
     } else {
         null_builder.append_null();
         views_buf.push(0);

datafusion/functions/src/strings.rs

@@ -21,7 +21,7 @@ use datafusion_common::{Result, exec_datafusion_err, internal_err};
 
 use arrow::array::{
     Array, ArrayAccessor, ArrayDataBuilder, BinaryArray, ByteView, LargeStringArray,
-    NullBufferBuilder, StringArray, StringViewArray, StringViewBuilder, make_view,
+    StringArray, StringViewArray, StringViewBuilder, make_view,
 };
 use arrow::buffer::{MutableBuffer, NullBuffer};
 use arrow::datatypes::DataType;
@@ -372,7 +372,9 @@ impl LargeStringArrayBuilder {
     }
 }
 
-/// Append a new view to the views buffer with the given substr
+/// Append a new view to the views buffer with the given substr.
+///
+/// Callers are responsible for their own null tracking.
 ///
 /// # Safety
 ///
@@ -381,13 +383,15 @@ impl LargeStringArrayBuilder {
 ///
 /// # Arguments
 /// - views_buffer: The buffer to append the new view to
-/// - null_builder: The buffer to append the null value to
 /// - original_view: The original view value
 /// - substr: The substring to append. Must be a valid substring of the original view
 /// - start_offset: The start offset of the substring in the view
-pub fn make_and_append_view(
+///
+/// LLVM is apparently overly eager to inline this function into some hot loops,
+/// which bloats them and regresses performance, so we disable inling for now.
+#[inline(never)]
+pub fn append_view(
     views_buffer: &mut Vec<u128>,
-    null_builder: &mut NullBufferBuilder,
     original_view: &u128,
     substr: &str,
     start_offset: u32,
@@ -401,11 +405,9 @@ pub fn make_and_append_view(
             view.offset + start_offset,
         )
     } else {
-        // inline value does not need block id or offset
         make_view(substr.as_bytes(), 0, 0)
     };
     views_buffer.push(sub_view);
-    null_builder.append_non_null();
 }
 
 #[derive(Debug)]

datafusion/functions/src/unicode/substr.rs

@@ -17,13 +17,13 @@
 
 use std::sync::Arc;
 
-use crate::strings::make_and_append_view;
+use crate::strings::append_view;
 use crate::utils::make_scalar_function;
 use arrow::array::{
-    Array, ArrayRef, AsArray, Int64Array, NullBufferBuilder, StringArrayType,
-    StringViewArray, StringViewBuilder,
+    Array, ArrayRef, AsArray, Int64Array, StringArrayType, StringViewArray,
+    StringViewBuilder,
 };
-use arrow::buffer::ScalarBuffer;
+use arrow::buffer::{NullBuffer, ScalarBuffer};
 use arrow::datatypes::DataType;
 use datafusion_common::cast::as_int64_array;
 use datafusion_common::types::{
@@ -278,39 +278,32 @@ fn string_view_substr(
     let enable_ascii_fast_path =
         enable_ascii_fast_path(&string_view_array, start_array, count_array_opt);
 
+    // Combine null bitmaps from all inputs in bulk.
+    let nulls = NullBuffer::union(
+        NullBuffer::union(string_view_array.nulls(), start_array.nulls()).as_ref(),
+        count_array_opt.and_then(|a| a.nulls()),
+    );
+
     let mut views_buf = Vec::with_capacity(string_view_array.len());
-    let mut null_builder = NullBufferBuilder::new(string_view_array.len());
-
-    for i in 0..string_view_array.len() {
-        if string_view_array.is_null(i)
-            || start_array.is_null(i)
-            || count_array_opt.map(|a| a.is_null(i)).unwrap_or(false)
-        {
-            null_builder.append_null();
+
+    for (i, raw_view) in string_view_array.views().iter().enumerate() {
+        if nulls.as_ref().is_some_and(|n| n.is_null(i)) {
             views_buf.push(0);
             continue;
         }
 
         let string = string_view_array.value(i);
         let start = start_array.value(i);
         let count = count_array_opt.map(|a| a.value(i));
-        let raw_view = string_view_array.views()[i];
 
         let (start, end) =
             get_true_start_end(string, start, count, enable_ascii_fast_path)?;
         let substr = &string[start..end];
 
-        make_and_append_view(
-            &mut views_buf,
-            &mut null_builder,
-            &raw_view,
-            substr,
-            start as u32,
-        );
+        append_view(&mut views_buf, raw_view, substr, start as u32);
     }
 
     let views_buf = ScalarBuffer::from(views_buf);
-    let nulls_buf = null_builder.finish();
 
     // Safety:
     // (1) The blocks of the given views are all provided
@@ -320,7 +313,7 @@ fn string_view_substr(
         let array = StringViewArray::new_unchecked(
             views_buf,
             string_view_array.data_buffers().to_vec(),
-            nulls_buf,
+            nulls,
         );
         Ok(Arc::new(array) as ArrayRef)
     }
@@ -336,13 +329,16 @@ where
     let enable_ascii_fast_path =
         enable_ascii_fast_path(&string_array, start_array, count_array_opt);
 
+    // Combine null bitmaps from all inputs in bulk.
+    let nulls = NullBuffer::union(
+        NullBuffer::union(string_array.nulls(), start_array.nulls()).as_ref(),
+        count_array_opt.and_then(|a| a.nulls()),
+    );
+
     let mut result_builder = StringViewBuilder::new();
 
     for i in 0..string_array.len() {
-        if string_array.is_null(i)
-            || start_array.is_null(i)
-            || count_array_opt.map(|a| a.is_null(i)).unwrap_or(false)
-        {
+        if nulls.as_ref().is_some_and(|n| n.is_null(i)) {
             result_builder.append_null();
             continue;
         }