Skip to content

Commit eb33141

Browse files
authored
feat: unify left and right functions and benches (#20114)
## Which issue does this PR close? - Closes #20103 ## Rationale for this change A refactoring PR for performance improvement PRs for left #19749 and right #20068. ## What changes are included in this PR? 1. Removed a lot of code duplication by extracting a common stringarray / stringview implementation. Now left and right UDFs entry points are leaner. Differences are only in slicing - from the left or from the right - which is implemented in a generic trait parameter, following the design of trim. 2. Switched `left` to use `make_view` to avoid buffer tinkering in datafusion code. 4. Combine left and right benches together ## Are these changes tested? - Existing unit tests - Existing SLTs passed - Benches show the same performance improvement of 60-85% Bench results against pre-optimisation commit 458b491: <details> left size=1024/string_array positive n/1024 time: [34.150 µs 34.694 µs 35.251 µs] change: [−71.694% −70.722% −69.818%] (p = 0.00 < 0.05) Performance has improved. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) high mild left size=1024/string_array negative n/1024 time: [30.860 µs 31.396 µs 31.998 µs] change: [−85.846% −85.294% −84.759%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 2 (2.00%) low mild 4 (4.00%) high mild 2 (2.00%) high severe left size=4096/string_array positive n/4096 time: [112.19 µs 114.28 µs 116.98 µs] change: [−71.673% −70.934% −70.107%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 2 (2.00%) high mild 1 (1.00%) high severe left size=4096/string_array negative n/4096 time: [126.71 µs 129.06 µs 131.26 µs] change: [−84.204% −83.809% −83.455%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 3 (3.00%) low mild 2 (2.00%) high mild left size=1024/string_view_array positive n/1024 time: [30.249 µs 30.887 µs 31.461 µs] change: [−75.288% −74.499% −73.743%] (p = 0.00 < 0.05) Performance has improved. Found 4 outliers among 100 measurements (4.00%) 3 (3.00%) low mild 1 (1.00%) high mild left size=1024/string_view_array negative n/1024 time: [48.404 µs 49.007 µs 49.608 µs] change: [−66.827% −65.727% −64.652%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 1 (1.00%) low mild 1 (1.00%) high mild 1 (1.00%) high severe left size=4096/string_view_array positive n/4096 time: [145.25 µs 148.47 µs 151.85 µs] change: [−68.913% −67.836% −66.770%] (p = 0.00 < 0.05) Performance has improved. left size=4096/string_view_array negative n/4096 time: [203.11 µs 206.31 µs 209.98 µs] change: [−57.411% −56.773% −56.142%] (p = 0.00 < 0.05) Performance has improved. Found 15 outliers among 100 measurements (15.00%) 1 (1.00%) low mild 13 (13.00%) high mild 1 (1.00%) high severe right size=1024/string_array positive n/1024 time: [30.820 µs 31.674 µs 32.627 µs] change: [−84.230% −83.842% −83.402%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 5 (5.00%) high mild right size=1024/string_array negative n/1024 time: [32.434 µs 33.170 µs 33.846 µs] change: [−88.796% −88.460% −88.164%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 3 (3.00%) high mild right size=4096/string_array positive n/4096 time: [124.71 µs 126.54 µs 128.27 µs] change: [−83.321% −82.902% −82.537%] (p = 0.00 < 0.05) Performance has improved. Found 2 outliers among 100 measurements (2.00%) 2 (2.00%) high mild right size=4096/string_array negative n/4096 time: [125.05 µs 127.67 µs 130.35 µs] change: [−89.376% −89.193% −89.004%] (p = 0.00 < 0.05) Performance has improved. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) high mild right size=1024/string_view_array positive n/1024 time: [29.110 µs 29.608 µs 30.141 µs] change: [−79.807% −79.330% −78.683%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 6 (6.00%) high mild 2 (2.00%) high severe right size=1024/string_view_array negative n/1024 time: [44.883 µs 45.656 µs 46.511 µs] change: [−71.157% −70.546% −69.874%] (p = 0.00 < 0.05) Performance has improved. Found 6 outliers among 100 measurements (6.00%) 5 (5.00%) high mild 1 (1.00%) high severe right size=4096/string_view_array positive n/4096 time: [139.57 µs 142.18 µs 144.96 µs] change: [−75.610% −75.088% −74.549%] (p = 0.00 < 0.05) Performance has improved. Found 2 outliers among 100 measurements (2.00%) 2 (2.00%) high severe right size=4096/string_view_array negative n/4096 time: [221.47 µs 224.47 µs 227.72 µs] change: [−64.625% −64.047% −63.504%] (p = 0.00 < 0.05) Performance has improved. Found 3 outliers among 100 measurements (3.00%) 3 (3.00%) high mild </details> ## 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 add the `api change` label. -->
1 parent 2f90194 commit eb33141

8 files changed

Lines changed: 327 additions & 580 deletions

File tree

datafusion/functions/Cargo.toml

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -308,12 +308,7 @@ required-features = ["string_expressions"]
308308

309309
[[bench]]
310310
harness = false
311-
name = "left"
312-
required-features = ["unicode_expressions"]
313-
314-
[[bench]]
315-
harness = false
316-
name = "right"
311+
name = "left_right"
317312
required-features = ["unicode_expressions"]
318313

319314
[[bench]]

datafusion/functions/benches/left.rs

Lines changed: 0 additions & 140 deletions
This file was deleted.
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
extern crate criterion;
19+
20+
use std::hint::black_box;
21+
use std::sync::Arc;
22+
23+
use arrow::array::{ArrayRef, Int64Array};
24+
use arrow::datatypes::{DataType, Field};
25+
use arrow::util::bench_util::{
26+
create_string_array_with_len, create_string_view_array_with_len,
27+
};
28+
use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main};
29+
use datafusion_common::config::ConfigOptions;
30+
use datafusion_expr::{ColumnarValue, ScalarFunctionArgs};
31+
use datafusion_functions::unicode::{left, right};
32+
33+
fn create_args(
34+
size: usize,
35+
str_len: usize,
36+
use_negative: bool,
37+
is_string_view: bool,
38+
) -> Vec<ColumnarValue> {
39+
let string_arg = if is_string_view {
40+
ColumnarValue::Array(Arc::new(create_string_view_array_with_len(
41+
size, 0.1, str_len, true,
42+
)))
43+
} else {
44+
ColumnarValue::Array(Arc::new(create_string_array_with_len::<i32>(
45+
size, 0.1, str_len,
46+
)))
47+
};
48+
49+
// For negative n, we want to trigger the double-iteration code path
50+
let n_values: Vec<i64> = if use_negative {
51+
(0..size).map(|i| -((i % 10 + 1) as i64)).collect()
52+
} else {
53+
(0..size).map(|i| (i % 10 + 1) as i64).collect()
54+
};
55+
let n_array = Arc::new(Int64Array::from(n_values));
56+
57+
vec![
58+
string_arg,
59+
ColumnarValue::Array(Arc::clone(&n_array) as ArrayRef),
60+
]
61+
}
62+
63+
fn criterion_benchmark(c: &mut Criterion) {
64+
let left_function = left();
65+
let right_function = right();
66+
67+
for function in [left_function, right_function] {
68+
for is_string_view in [false, true] {
69+
for is_negative in [false, true] {
70+
for size in [1024, 4096] {
71+
let function_name = function.name();
72+
let mut group =
73+
c.benchmark_group(format!("{function_name} size={size}"));
74+
75+
let bench_name = format!(
76+
"{} {} n",
77+
if is_string_view {
78+
"string_view_array"
79+
} else {
80+
"string_array"
81+
},
82+
if is_negative { "negative" } else { "positive" },
83+
);
84+
let return_type = if is_string_view {
85+
DataType::Utf8View
86+
} else {
87+
DataType::Utf8
88+
};
89+
90+
let args = create_args(size, 32, is_negative, is_string_view);
91+
group.bench_function(BenchmarkId::new(bench_name, size), |b| {
92+
let arg_fields = args
93+
.iter()
94+
.enumerate()
95+
.map(|(idx, arg)| {
96+
Field::new(format!("arg_{idx}"), arg.data_type(), true)
97+
.into()
98+
})
99+
.collect::<Vec<_>>();
100+
let config_options = Arc::new(ConfigOptions::default());
101+
102+
b.iter(|| {
103+
black_box(
104+
function
105+
.invoke_with_args(ScalarFunctionArgs {
106+
args: args.clone(),
107+
arg_fields: arg_fields.clone(),
108+
number_rows: size,
109+
return_field: Field::new(
110+
"f",
111+
return_type.clone(),
112+
true,
113+
)
114+
.into(),
115+
config_options: Arc::clone(&config_options),
116+
})
117+
.expect("should work"),
118+
)
119+
})
120+
});
121+
122+
group.finish();
123+
}
124+
}
125+
}
126+
}
127+
}
128+
129+
criterion_group!(benches, criterion_benchmark);
130+
criterion_main!(benches);

0 commit comments

Comments
 (0)