andygrove opened a new pull request, #23881:
URL: https://github.com/apache/datafusion/pull/23881
## 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.
A new benchmark, `datafusion/spark/benches/bin.rs`, covers `bin` at 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`.
### 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 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]
