andygrove opened a new pull request, #23880:
URL: https://github.com/apache/datafusion/pull/23880
## Which issue does this PR close?
N/A
## Rationale for this change
Spark's `soundex` and `quote` both allocated a fresh `String` for every row
and
collected the results into a `StringArray`.
`soundex` allocated twice per row: once for the code buffer, and again for
the
`format!("{soundex_code:0<4}")` that zero-pads it. `quote` allocated a
`String`
sized to the input, then copied the input into it one `char` at a time.
Neither function needs to allocate per row. A soundex code is always exactly
four ASCII characters, so it fits in a stack buffer. `quote` only ever wraps
the
input and escapes embedded quotes, so it can write straight into the output
buffer and copy the runs between quotes rather than character by character.
## What changes are included in this PR?
`soundex.rs`:
- `compute_soundex(&str) -> String` becomes `append_soundex(&mut
StringBuilder, &str)`,
building the four-character code in a `[u8; 4]` initialised to `b'0'` —
which is
the zero-padding, so the trailing `format!` disappears.
- Strings that do not start with an ASCII letter are passed through
unchanged;
previously this path called `s.to_string()`, now it appends by reference.
- The `Utf8`/`LargeUtf8` and `Utf8View` entry points share one `soundex_impl`
over `Option<&str>`, pre-sizing the builder at 4 bytes per row.
`quote.rs`:
- `compute_quote(&str) -> String` becomes `append_quoted(&mut StringBuilder,
&str)`,
writing into the builder's buffer via the `fmt::Write` impl and finalising
with
`append_value("")`.
- Escaping uses `str::split('\'')` to copy the runs between quotes in one
memcpy
each, instead of pushing every `char` individually.
- The builder is pre-sized from the input's `value_data()` length plus two
bytes
per row for the surrounding quotes.
Output is unchanged in both cases.
## Are these changes tested?
Existing coverage pins the behaviour: `spark/string/soundex.slt` and
`spark/string/quote.slt` assert concrete outputs, including the
non-alphabetic
passthrough, codes shorter than four characters (padding), codes truncated at
four, and strings with and without embedded quotes. All 59 `spark/string`
sqllogictest files and the 258 `datafusion-spark` unit tests pass.
A new benchmark, `datafusion/spark/benches/soundex_quote.rs`, covers both
functions over `Utf8` and `Utf8View` at 1024 and 8192 rows, with 20% nulls.
The
`quote` input is a mix of strings with and without embedded quotes so the
escaping path is exercised without dominating.
### Benchmarks
Criterion, `apache/main` @ `f1ab86dad` as baseline. Median of the reported
change interval.
| Benchmark | 1024 | 8192 |
| --- | --- | --- |
| `soundex/utf8` | −52.9% | −49.5% |
| `soundex/utf8view` | −54.5% | −50.2% |
| `quote/utf8` | −60.7% | −61.5% |
| `quote/utf8view` | −60.5% | −61.3% |
## Are there any user-facing changes?
No. Both functions produce byte-identical output; this is purely an
allocation
change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
