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]

Reply via email to