bench: add Criterion benchmarks for Spark bin, soundex, quote, url_encode, url_decode, and next_day#23882
Open
andygrove wants to merge 3 commits into
Open
bench: add Criterion benchmarks for Spark bin, soundex, quote, url_encode, url_decode, and next_day#23882andygrove wants to merge 3 commits into
andygrove wants to merge 3 commits into
Conversation
…l_encode None of these four functions had a benchmark. Adding them separately from any optimization means the harness can land first, so a baseline can be measured on main and compared against a follow-up change. Each covers 1024 and 8192 rows with 20% nulls: - bin: small values that render to a few binary digits, and full-range values that render to the maximum 64. - soundex: words of varying length, so both truncated and zero-padded codes are exercised. Utf8 and Utf8View. - quote: a mix of strings with and without embedded quotes, so the escaping path is covered without dominating. Utf8 and Utf8View. - url_encode: characters mostly passed through with a minority needing percent-escaping. Utf8, LargeUtf8, and Utf8View.
andygrove
added a commit
to andygrove/datafusion
that referenced
this pull request
Jul 25, 2026
Both functions built a fresh String for every row and collected the results into a StringArray. soundex allocated twice per row -- once for the code buffer and once more for the format! that zero-pads it -- and quote allocated a String sized to the input before copying it in character at a time. Neither needs to allocate. A soundex code is always exactly four ASCII characters, so it is built in a stack buffer. quote writes straight into the builder and copies the runs between quotes rather than one char at a time. soundex -50%, quote -61% against the benchmarks added in apache#23882.
andygrove
added a commit
to andygrove/datafusion
that referenced
this pull request
Jul 25, 2026
bin formatted every row with format!("{value:b}"), and char built a String
for a single character with ch.to_string(), before collecting into a
StringArray.
Both render into a stack buffer instead: bin writes its digits right-aligned
in a [u8; 64] and char uses char::encode_utf8 into a [u8; 4]. Values are
appended to a pre-sized StringBuilder rather than collected.
bin -74% on small values and -50% on full-width ones, char -76%. The bin
benchmark is added in apache#23882.
This was referenced Jul 25, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23882 +/- ##
==========================================
- Coverage 80.65% 80.65% -0.01%
==========================================
Files 1091 1092 +1
Lines 371031 371106 +75
Branches 371031 371106 +75
==========================================
+ Hits 299256 299315 +59
- Misses 53935 53939 +4
- Partials 17840 17852 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Covers the two shapes that behave differently: a scalar day-of-week (next_day(col, 'MONDAY')), where the name is constant across the batch, and a day-of-week column, where it varies per row. Day names are mixed case so the case-insensitive parse is exercised.
andygrove
changed the title
Splits input by whether it actually needs unescaping: 'plain' rows contain no percent-escapes, so the decoded value can borrow its input, while 'escaped' rows have roughly a third of their segments escaped. Covers Utf8, LargeUtf8, and Utf8View.
andygrove
added a commit
to andygrove/datafusion
that referenced
this pull request
Jul 25, 2026
…key array directly url_decode ended with .map(|parsed| parsed.into_owned()). Both replace_plus and decode_utf8 borrow their input when there is nothing to rewrite, so that allocated a String for every row even when the value needed no decoding at all. decode now returns a Cow and the array paths append it to a pre-sized builder. To let the Cow survive, spark_handled_url_decode's error handler changes from a closure over Result<Option<String>> to an OnDecodeError enum. Its only caller, try_url_decode, passed a closure that mapped Err to Ok(None), which is exactly OnDecodeError::Null. parse_url built its all-null key array for the 2-argument form by calling append_null() once per row on an uncapacitated builder; new_null_array does it in one allocation. url_decode -29% where nothing needs unescaping, -4% where it does, against the benchmark added in apache#23882.
andygrove
changed the title
athlcode
pushed a commit
to athlcode/datafusion
that referenced
this pull request
Jul 25, 2026
…3881) ## Which issue does this PR close? N/A ## Rationale for this change Two Spark functions allocated a `String` per row purely to render a small, bounded amount of text: - `bin` called `format!("{value:b}")` for every row. - `char` called `ch.to_string()` for every row — a heap allocation for a single character. In both cases the output has a known upper bound (64 binary digits for an `i64`, 4 bytes for a UTF-8 character), so the rendering fits in a stack buffer and the result can be appended straight to a pre-sized builder. ## What changes are included in this PR? `math/bin.rs`: - `spark_bin` now writes digits right-aligned into a caller-supplied `[u8; 64]` and returns a `&str` borrowed from it, instead of returning an owned `String`. - The `collect::<StringArray>()` becomes an explicit loop over a `StringBuilder::with_capacity`, sized at 8 digits per row. - Negative values still render as their two's-complement bit pattern, matching `{:b}`. The digit loop is a `loop`, not a `while`, so zero renders as `"0"` rather than the empty string. `string/char.rs`: - `ch.to_string()` becomes `ch.encode_utf8(&mut encoded)` against a `[u8; 4]` hoisted out of the loop. Output is unchanged in both cases. ## Are these changes tested? Existing coverage pins the behaviour. `spark/math/bin.slt` asserts concrete output for the cases the rewrite had to get right: zero, negative values, `i64::MIN` (`-9223372036854775808`), `i64::MAX`, and `-2147483648` / `-32768` widened from narrower integer types. `spark/string/char.slt` covers the negative-input empty string, the null path, and characters on both sides of the ASCII boundary (`char(256)` and above wrap via `% 256`). All 119 `spark/math` and `spark/string` sqllogictest files pass, along with the 258 `datafusion-spark` unit tests. The `bin` benchmark used below, `datafusion/spark/benches/bin.rs`, is added separately in apache#23882 so the baseline can be measured on `main` before this change lands. It covers 1024 and 8192 rows with 20% nulls over two value distributions: small values that render to a handful of digits, and full-range values that render to the maximum 64. `char` already had `datafusion/spark/benches/char.rs` on `main`, so no benchmark change is needed for it. ### Benchmarks Criterion, `apache/main` @ `f1ab86dad` as baseline. Median of the reported change interval. | Benchmark | 1024 | 8192 | | --- | --- | --- | | `bin/small` | −75.7% | −73.7% | | `bin/wide` | −47.8% | −49.8% | `char` (1024 rows): −76.1%. ## Are there any user-facing changes? No. Both functions produce byte-identical output; this is purely an allocation change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
N/A
Rationale for this change
bin,soundex,quote,url_encode,url_decode, andnext_dayindatafusion-sparkhad no benchmark coverage.This PR adds the benchmarks on their own, with no change to any function, so
they can be merged ahead of the optimizations that follow. That way the baseline
is measurable on
mainand CI can compare the optimization PRs against it,rather than the harness and the change arriving together.
What changes are included in this PR?
Six new Criterion benchmarks under
datafusion/spark/benches/, registered indatafusion/spark/Cargo.toml. Each runs at 1024 and 8192 rows with a 20% nulldensity and a fixed RNG seed:
bin.rssoundex_quote.rs(soundex)Utf8andUtf8Viewsoundex_quote.rs(quote)Utf8andUtf8Viewurl_encode.rsUtf8,LargeUtf8, andUtf8Viewnext_day.rsurl_decode.rsUtf8,LargeUtf8, andUtf8ViewNo function implementation is touched.
The optimization PRs that build on these baselines:
soundexandquotebin(andchar, which already had a benchmark)url_encodeandurl_decodenext_dayAre these changes tested?
Benchmarks are not tests, but all six compile and run against unmodified
main;the numbers they produce on
mainare the baselines quoted in the optimizationPRs above.
Are there any user-facing changes?
No. This adds benchmark files only.