Skip to content

perf: Optimize string split functions and avoid intermediate heap allocations - #5416

Open
kazantsev-maksim wants to merge 80 commits into
apache:mainfrom
kazantsev-maksim:perf_split
Open

perf: Optimize string split functions and avoid intermediate heap allocations#5416
kazantsev-maksim wants to merge 80 commits into
apache:mainfrom
kazantsev-maksim:perf_split

Conversation

@kazantsev-maksim

@kazantsev-maksim kazantsev-maksim commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

  • N/A

Rationale for this change

Optimize existing expression.

What changes are included in this PR?

This PR improves the performance and memory efficiency of spark_split and related string split expressions by optimizing buffer allocations, avoiding unnecessary regex compilation for literal delimiters, and reusing scratch buffers.

  1. Fast-path for literal / single-char delimiters:
  • Detects if the pattern is a literal string (without regex metacharacters) and bypasses the regex::Regex DFA engine.
  • For single-character delimiters (e.g. ',', '|'), uses fast std::str::split(char) pattern matching instead of general substring or regex searches.
  1. Eliminated per-row heap allocations on limit = 0:
  • Reused a single scratch: Vec<&str> buffer across rows using .clear() instead of collecting an allocated vector for every single string in the batch.
  1. Pre-allocated Arrow builders with capacity:
  • Pre-allocated string value and offset buffers based on the input batch size and total byte length (value_data().len()), eliminating frequent realloc spikes.
  1. Optimized scalar branches:
  • Removed intermediate Vec allocations for scalar inputs, building Arrow buffers directly from borrowed string slices.

How are these changes tested?

Existing tests.

Benchmark (criterion):

Benchmark Baseline Optimized Throughput Diff Time Diff
literal_char_default_limit (1024) 96.3 µs 68.5 µs +41.2% -29.2%
literal_char_default_limit (8192) 754 µs 649 µs +15.3% -13.3%
literal_char_limit_0 (1024) 106 µs 39.8 µs +165.5% -62.3%
literal_char_limit_0 (8192) 840 µs 319 µs +169.3% -62.9%
regex_pattern (8192) 3.16 ms 3.18 ms Within noise +1.8%

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Reviewed immutable head 4f382219f7c1dbddeff784501a5b1b518c859f6b against base 2699f59b71788e17a2714910e166a3f83deed937 with five independent review scopes, followed by separate verification. One introduced P2 remains: scalar split_sql changes the meaning of an empty literal delimiter and can return incorrect split_part results. The inline comment includes the supported scalar-subquery case and executable before/after evidence.

Prior state and problem

The previous regex split path compiled a regex even for plain literal delimiters, and its limit-zero path collected a fresh temporary vector for each row. Scalar paths also materialized owned intermediate strings before constructing Arrow arrays. The optimization targets this allocation and matching overhead; preserving existing literal SQL-split semantics is essential because split_sql also implements Spark 4.x split_part.

Design approach

The patch classifies patterns conservatively using regex metacharacters, then selects character, substring, or regex matching. It reuses a borrowed-slice scratch vector across rows and reserves Arrow output buffers using input size estimates. Scalar results are assembled directly into Arrow buffers instead of going through the previous owned-string vectors.

Correctness / compatibility analysis

Exact-base/head native probes, real Spark 4.0.2 reference queries with ANSI on and off, and a native DataFusion/UDF/ListExtract composition independently reproduce the empty-delimiter regression. Scalar subqueries survive normal constant folding, and exact routing source shows their string results reach the changed scalar branch; the unchanged array paths still preserve the whole string. The ordinary StringSplit default JVM-backed route, its opt-in Rust regex differences, and non-default-collation fallback are unchanged and are not additional findings.

The existing split-module tests pass at both pins: six at head and fourteen at base. These are focused module tests, not a full Comet native/JVM or newly built JNI integration run. The requested 72-file two-commit comparison was inspected in full; the three-file merge-base contribution was also checked so unrelated newer-base changes were not misidentified as regressions.

Key design decisions

The literal detector leaves regex metacharacters on the regex path, while the single-character path advances by that character's UTF-8 byte length. Scratch references remain borrowed from the input until their bytes are copied into the output buffers. Array offset widths and validity propagation retain their existing structure; the verified problem is the new scalar empty-delimiter policy, not a demonstrated aliasing or lifetime failure.

Implementation sketch

split.rs adds literal/character split helpers and direct offset/value-buffer assembly, and adapts the regex helper to reuse scratch storage. The scalar SQL-split branch is rewritten separately, which is where it diverges from push_split_sql_parts. The other two contributed files register and implement the new Criterion benchmark.

Behavioral changes worth calling out

The intended changes reduce matching and allocation overhead, but scalar SQL splitting currently changes observable results for an empty delimiter: a multi-character input becomes individual characters while the column-input form stays intact. The benchmark's six array-input scenarios compile and pass smoke execution; the PR's release speedup figures were not independently reproduced. At the reviewed head, CodeQL, Delta Contrib Build Gate, and CI report action_required, each with zero jobs, so there is no successful CI execution to rely on.

Suggested improvements

Preserve the original string as one element in the scalar empty-delimiter branch, including an empty input, matching Spark and the existing array helper. Restore equivalent public-entry-point coverage for empty delimiters across scalar and array inputs, and include a scalar-subquery SQL case so constant folding cannot conceal the regression. This directly addresses the single inline P2 without expanding the scope of the optimization.

str_offsets.append(0);

if delimiter.is_empty() {
for ch in string.chars() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve the whole string for an empty SQL delimiter

StringSplitSQL treats an empty delimiter as "do not split", but this scalar branch now emits one item per character. For a Parquet table t(s) containing 'abc', SELECT split_part((SELECT max(s) FROM t), '', 1) FROM t retains the scalar subquery through normal Spark optimization, and Comet's 4.x route passes its string result to split_sql as a scalar. Exact-base/head native UDF + ListExtract probes return 'abc' versus 'a' (part 2 changes from '' to 'b'), while Spark 4.0.2 and the unchanged array paths preserve the whole input. This is the default literal SQL-split route, not the incompatible-regex opt-in. Please append the whole string once for an empty delimiter and retain a scalar/array regression test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, i tried addressed your comments.

let cap = (limit - 1) as usize;
let mut count = 0;
let mut last_end = 0;
for (start, _) in string.match_indices(delimiter) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Make both literal split loops pass the Clippy gate

This manual counter and the equivalent loop in push_split_char (line 333) trigger clippy::explicit_counter_loop. Fresh Clippy runs on the exact base/head module, with the crate's lint attributes and -D warnings, pass at the base but fail at the head on these two loops. .github/actions/rust-test/action.yaml runs cargo clippy --color=never --all-targets --workspace -- -D warnings, so these loops will fail the Rust-test check once the currently blocked workflows run. Please use .enumerate() in both helpers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants