andygrove opened a new pull request, #23884:
URL: https://github.com/apache/datafusion/pull/23884
## Which issue does this PR close?
N/A
## Rationale for this change
Spark's `url_encode` built a `String` for every row:
```rust
fn encode(value: &str) -> Result<String> {
Ok(byte_serialize(value.as_bytes()).collect::<String>())
}
```
and then collected the results into a `StringArray`, so each row paid for its
own allocation plus the growth of that `String` as the encoded fragments were
appended.
The encoded form is only needed long enough to be copied into the output
array,
so a single scratch buffer reused across rows does the same job with one
allocation for the whole batch.
## What changes are included in this PR?
- The three type branches (`Utf8`, `LargeUtf8`, `Utf8View`) each build a
pre-sized builder and run a shared `encode_all!` loop that clears and
refills
one `String` per batch, appending each result to the builder.
- The `Utf8` and `LargeUtf8` builders are sized from the input's
`value_data()`
length; percent-encoding only grows the output, so this is a floor rather
than
a bound, but it removes the early reallocations.
- `UrlEncode::encode` is removed. It returned a `Result` whose error arm was
never constructed, and it now has no callers.
Output is unchanged.
## Are these changes tested?
Existing coverage pins the behaviour: `spark/url/url_encode.slt` asserts
concrete encodings including spaces, reserved characters, and the null path,
and
the `datafusion-spark` unit tests cover all three string types. All 5
`spark/url` sqllogictest files and the 258 unit tests pass.
The benchmark used below is added separately in #23882, so the baseline can
be
measured on `main` before this change lands.
### Benchmarks
Criterion, `apache/main` @ `f1ab86dad` as baseline. Median of the reported
change interval.
| Benchmark | 1024 | 8192 |
| --- | --- | --- |
| `url_encode/utf8` | −57.1% | −48.2% |
| `url_encode/largeutf8` | −56.1% | −49.7% |
| `url_encode/utf8view` | −56.0% | −46.5% |
## Are there any user-facing changes?
No. `url_encode` produces byte-identical output; this is purely an allocation
change. `UrlEncode::encode` was a private associated function.
--
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]