Skip to content

Commit b8d3b0b

Browse files
andygroveJefffrey
andauthored
perf: optimize find_in_set (up to 24x faster) (#23460)
## Which issue does this PR close? N/A ## Rationale for this change Improve performance of existing expression. ## What changes are included in this PR? Replace per-row O(set_len) linear scan in find_in_set's constant-list path with a one-time HashMap lookup (threshold-guarded so short lists keep the linear scan), giving O(1) per-row probing for large sets. ## Are these changes tested? Existing tests + new tests Benchmark (criterion): - long_list_256: 95.827% faster (base 1246412ns -> cand 52009ns) - ~24x faster - short_list_4: 2.13% faster (base 64343ns -> cand 62972ns) - long_list_64: 88.562% faster (base 422083ns -> cand 48276ns) ## Are there any user-facing changes? No <!-- 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. --> Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
1 parent e9a75bf commit b8d3b0b

3 files changed

Lines changed: 173 additions & 5 deletions

File tree

datafusion/functions/Cargo.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,11 @@ harness = false
330330
name = "find_in_set"
331331
required-features = ["unicode_expressions"]
332332

333+
[[bench]]
334+
harness = false
335+
name = "find_in_set_literal"
336+
required-features = ["unicode_expressions"]
337+
333338
[[bench]]
334339
harness = false
335340
name = "contains"
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
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+
//! Benchmarks the `find_in_set(column, constant_list)` path where the set is a
19+
//! scalar literal. A long list exercises the pre-built lookup; a short list
20+
//! stays on the per-row linear scan.
21+
22+
use arrow::array::StringArray;
23+
use arrow::datatypes::{DataType, Field};
24+
use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main};
25+
use datafusion_common::ScalarValue;
26+
use datafusion_common::config::ConfigOptions;
27+
use datafusion_expr::{ColumnarValue, ScalarFunctionArgs};
28+
use rand::prelude::StdRng;
29+
use rand::{Rng, SeedableRng};
30+
use std::hint::black_box;
31+
use std::sync::Arc;
32+
33+
const N_ROWS: usize = 8192;
34+
35+
/// Builds a string column whose values are drawn from `entries` plus a small
36+
/// fraction of misses, so both hits and misses are exercised.
37+
fn build_column(entries: &[String]) -> StringArray {
38+
let mut rng = StdRng::seed_from_u64(42);
39+
let values: Vec<Option<String>> = (0..N_ROWS)
40+
.map(|_| {
41+
let r = rng.random::<f32>();
42+
if r < 0.1 {
43+
None
44+
} else if r < 0.4 {
45+
Some("__miss__".to_string())
46+
} else {
47+
let idx = rng.random_range(0..entries.len());
48+
Some(entries[idx].clone())
49+
}
50+
})
51+
.collect();
52+
StringArray::from(values)
53+
}
54+
55+
fn bench_case(c: &mut Criterion, label: &str, num_entries: usize) {
56+
let find_in_set = datafusion_functions::unicode::find_in_set();
57+
let entries: Vec<String> = (0..num_entries).map(|i| format!("item{i}")).collect();
58+
let list = entries.join(",");
59+
60+
let column = build_column(&entries);
61+
let args = vec![
62+
ColumnarValue::Array(Arc::new(column)),
63+
ColumnarValue::Scalar(ScalarValue::Utf8(Some(list))),
64+
];
65+
let arg_fields = args
66+
.iter()
67+
.map(|arg| Field::new("a", arg.data_type().clone(), true).into())
68+
.collect::<Vec<_>>();
69+
let return_field = Arc::new(Field::new("f", DataType::Int32, true));
70+
let config_options = Arc::new(ConfigOptions::default());
71+
72+
c.bench_with_input(
73+
BenchmarkId::new("find_in_set_literal", label),
74+
&num_entries,
75+
|b, _| {
76+
b.iter(|| {
77+
black_box(find_in_set.invoke_with_args(ScalarFunctionArgs {
78+
args: args.clone(),
79+
arg_fields: arg_fields.clone(),
80+
number_rows: N_ROWS,
81+
return_field: Arc::clone(&return_field),
82+
config_options: Arc::clone(&config_options),
83+
}))
84+
})
85+
},
86+
);
87+
}
88+
89+
fn criterion_benchmark(c: &mut Criterion) {
90+
// Short list stays on the linear scan (below the lookup threshold).
91+
bench_case(c, "short_list_4", 4);
92+
// Long lists exercise the pre-built lookup.
93+
bench_case(c, "long_list_64", 64);
94+
bench_case(c, "long_list_256", 256);
95+
}
96+
97+
criterion_group!(benches, criterion_benchmark);
98+
criterion_main!(benches);

datafusion/functions/src/unicode/find_in_set.rs

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use arrow_buffer::NullBuffer;
2525

2626
use crate::utils::utf8_to_int_type;
2727
use datafusion_common::{
28-
Result, ScalarValue, exec_err, internal_err, utils::take_function_args,
28+
HashMap, Result, ScalarValue, exec_err, internal_err, utils::take_function_args,
2929
};
3030
use datafusion_expr::TypeSignature::Exact;
3131
use datafusion_expr::{
@@ -316,6 +316,11 @@ where
316316
Ok(Arc::new(PrimitiveArray::<T>::new(values.into(), nulls)) as ArrayRef)
317317
}
318318

319+
/// Minimum set length at which a pre-built lookup beats a per-row linear scan.
320+
/// Below this, the linear scan's small constant factor wins, so short sets are
321+
/// left untouched to avoid regressing them.
322+
const FIND_IN_SET_LOOKUP_THRESHOLD: usize = 16;
323+
319324
fn find_in_set_right_literal<'a, T, V>(
320325
string_array: V,
321326
str_list: &[&str],
@@ -329,16 +334,34 @@ where
329334
let nulls = string_array.nulls().cloned();
330335
let zero = T::Native::from_usize(0).unwrap();
331336

337+
// The set (`str_list`) is constant across all rows. For a large set, the
338+
// per-row `position` linear scan is O(set_len). Building a lookup from each
339+
// distinct entry to its 1-based position once turns each row into an O(1)
340+
// probe (first occurrence wins, exactly matching `position`). Below the
341+
// threshold the linear scan's small constant factor is faster, so the map is
342+
// built at most once here rather than per row.
343+
let map: Option<HashMap<&str, usize>> =
344+
(str_list.len() >= FIND_IN_SET_LOOKUP_THRESHOLD).then(|| {
345+
let mut map = HashMap::with_capacity(str_list.len());
346+
for (idx, entry) in str_list.iter().enumerate() {
347+
map.entry(*entry).or_insert(idx + 1);
348+
}
349+
map
350+
});
351+
332352
let values: Vec<T::Native> = (0..len)
333353
.map(|i| {
334354
if nulls.as_ref().is_some_and(|n| n.is_null(i)) {
335355
return zero;
336356
}
337357
let string = string_array.value(i);
338-
let position = str_list
339-
.iter()
340-
.position(|s| *s == string)
341-
.map_or(0, |idx| idx + 1);
358+
let position = match &map {
359+
Some(map) => map.get(string).copied().unwrap_or(0),
360+
None => str_list
361+
.iter()
362+
.position(|s| *s == string)
363+
.map_or(0, |idx| idx + 1),
364+
};
342365
T::Native::from_usize(position).unwrap()
343366
})
344367
.collect();
@@ -545,4 +568,46 @@ mod tests {
545568
],
546569
Int32Array::from(vec![None::<i32>; 3])
547570
);
571+
572+
// Exercises both the lookup-map path (list length >= threshold) and the
573+
// linear-scan path (short list), including a duplicate entry to confirm the
574+
// first occurrence wins in both.
575+
#[test]
576+
fn test_right_literal_lookup_matches_linear() {
577+
use super::find_in_set_right_literal;
578+
use arrow::datatypes::Int32Type;
579+
580+
// 40 unique entries plus a duplicate of "item5" appended at index 40, so
581+
// the length is well over FIND_IN_SET_LOOKUP_THRESHOLD.
582+
let mut long_list: Vec<String> = (0..40).map(|i| format!("item{i}")).collect();
583+
long_list.push("item5".to_string());
584+
let long_refs: Vec<&str> = long_list.iter().map(|s| s.as_str()).collect();
585+
let short_refs = ["a", "b", "c"];
586+
587+
let strings = StringArray::from(vec![
588+
Some("item0"),
589+
Some("item39"),
590+
Some("item5"),
591+
Some("missing"),
592+
None,
593+
Some("b"),
594+
]);
595+
596+
let long =
597+
find_in_set_right_literal::<Int32Type, _>(&strings, &long_refs).unwrap();
598+
let long = long.as_any().downcast_ref::<Int32Array>().unwrap();
599+
assert_eq!(long.value(0), 1);
600+
assert_eq!(long.value(1), 40);
601+
assert_eq!(long.value(2), 6); // first occurrence of "item5"
602+
assert_eq!(long.value(3), 0);
603+
assert!(long.is_null(4));
604+
assert_eq!(long.value(5), 0);
605+
606+
let short =
607+
find_in_set_right_literal::<Int32Type, _>(&strings, &short_refs).unwrap();
608+
let short = short.as_any().downcast_ref::<Int32Array>().unwrap();
609+
assert_eq!(short.value(0), 0);
610+
assert!(short.is_null(4));
611+
assert_eq!(short.value(5), 2); // "b" at position 2
612+
}
548613
}

0 commit comments

Comments
 (0)