You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/04/09 21:39:10 UTC
(datafusion) branch gh-readonly-queue/main/pr-21519-e8d217a8b54feb82c192ae856141cf1f615604cf created (now 8939726929)
This is an automated email from the ASF dual-hosted git repository.
github-bot pushed a change to branch gh-readonly-queue/main/pr-21519-e8d217a8b54feb82c192ae856141cf1f615604cf
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 8939726929 perf: Optimize NULL handling in `substr` (#21519)
This branch includes the following new commits:
new 8939726929 perf: Optimize NULL handling in `substr` (#21519)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: perf: Optimize NULL handling in `substr` (#21519)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-bot pushed a commit to branch gh-readonly-queue/main/pr-21519-e8d217a8b54feb82c192ae856141cf1f615604cf
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 89397269299b56655c583fdfea379e39913b8e94
Author: Neil Conway <ne...@gmail.com>
AuthorDate: Thu Apr 9 17:38:49 2026 -0400
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.
---
datafusion/functions/src/string/common.rs | 23 +++++---------
datafusion/functions/src/strings.rs | 16 +++++-----
datafusion/functions/src/unicode/substr.rs | 48 ++++++++++++++----------------
3 files changed, 38 insertions(+), 49 deletions(-)
diff --git a/datafusion/functions/src/string/common.rs b/datafusion/functions/src/string/common.rs
index cc97c1b2c1..bda6b429ac 100644
--- a/datafusion/functions/src/string/common.rs
+++ b/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);
diff --git a/datafusion/functions/src/strings.rs b/datafusion/functions/src/strings.rs
index b0e988a3a8..ebc58490b2 100644
--- a/datafusion/functions/src/strings.rs
+++ b/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)]
diff --git a/datafusion/functions/src/unicode/substr.rs b/datafusion/functions/src/unicode/substr.rs
index 43f7e58fa2..9e11534e2c 100644
--- a/datafusion/functions/src/unicode/substr.rs
+++ b/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,15 +278,16 @@ 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;
}
@@ -294,23 +295,15 @@ fn string_view_substr(
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;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org