sunchao opened a new pull request, #7329:
URL: https://github.com/apache/datafusion-comet/pull/7329
## Which issue does this PR close?
No linked issue; this optimizes the existing native URL expressions.
## Rationale for this change
Native `parse_url` and `try_parse_url` currently allocate a temporary string
for each extracted component and run a general URI regex for ordinary
HTTP/HTTPS URLs. That work is repeated for every input row.
## What changes are included in this PR?
Borrow component slices until Arrow copies them into independently owned
output buffers. `FILE` uses the contiguous path/query slice, and query-key
matches retain the existing regex capture semantics. Lowercase HTTP/HTTPS
prefixes use delimiter decomposition with the existing validation; other URLs
keep the native regex parser.
Extend the benchmark to cover all three Arrow string layouts, short and long
URLs, nulls, Unicode, query keys, invalid inputs, and FTP controls.
## How are these changes tested?
- All 31 native URL tests pass, including new decomposition-parity,
FILE/query-capture, and sliced-array/output-ownership coverage for Utf8,
LargeUtf8 and Utf8View.
- Rust formatting and `git diff --check` pass.
- Focused Clippy passes with warnings treated as errors (`--lib --tests
--bench parse_url -- -D warnings`).
### Benchmark measurements
Compared source commit `a0c4ac35de` with upstream
`7f73d86fb841f5662f1343cf52b6e6b31eb17dfb`, using the identical extended
benchmark on both versions. These are complete native UDF calls, including
argument broadcasting and Arrow output construction, over **8,192 rows per
batch**. They are not end-to-end Spark query measurements.
The final paired run measured the non-null HTTP(S) component cases
**5.5–9.4x faster**, query-key extraction **3.1x faster**, and
sparse/dense-null PATH extraction **6.2x/4.1x faster**.
<details>
<summary>All 25 cases: Criterion mean elapsed time per batch</summary>
| Case | Base (ms) | Port (ms) | Speedup |
| --- | ---: | ---: | ---: |
| dense_nulls/Utf8/PATH | 1.099 | 0.265 | 4.15x |
| ftp/LargeUtf8/HOST | 9.105 | 8.632 | 1.05x |
| ftp/Utf8View/HOST | 8.813 | 8.678 | 1.02x |
| ftp/Utf8/HOST | 9.077 | 8.805 | 1.03x |
| http/Utf8/HOST | 9.291 | 1.591 | 5.84x |
| https/LargeUtf8/FILE | 9.604 | 1.365 | 7.03x |
| https/LargeUtf8/HOST | 8.884 | 1.513 | 5.87x |
| https/LargeUtf8/PATH | 8.837 | 1.317 | 6.71x |
| https/Utf8View/FILE | 9.676 | 1.392 | 6.95x |
| https/Utf8View/HOST | 9.363 | 1.491 | 6.28x |
| https/Utf8View/PATH | 9.197 | 1.339 | 6.87x |
| https/Utf8/AUTHORITY | 9.013 | 1.343 | 6.71x |
| https/Utf8/FILE | 9.635 | 1.328 | 7.26x |
| https/Utf8/HOST | 9.480 | 1.533 | 6.18x |
| https/Utf8/PATH | 9.114 | 1.326 | 6.87x |
| https/Utf8/PROTOCOL | 8.782 | 1.327 | 6.62x |
| https/Utf8/QUERY | 9.352 | 1.343 | 6.96x |
| https/Utf8/REF | 9.027 | 1.335 | 6.76x |
| https_long/LargeUtf8/PATH | 220.964 | 39.751 | 5.56x |
| https_long/Utf8View/PATH | 197.220 | 21.031 | 9.38x |
| https_long/Utf8/PATH | 218.043 | 39.967 | 5.46x |
| query_key | 10.783 | 3.438 | 3.14x |
| sparse_nulls/Utf8/PATH | 8.120 | 1.317 | 6.17x |
| try_invalid/Utf8/PATH | 8.091 | 1.516 | 5.34x |
| unicode/Utf8/PATH | 9.320 | 1.489 | 6.26x |
</details>
FTP exercises the generic parser. Measurements varied across runs: an early
run flagged a slowdown, but head/base/base/head repeats did not reproduce a
consistent direction. The table reports only the final source after an
equivalent prefix-check lint cleanup. Criterion flagged no regression in that
final run. These measurements do not establish statistical equivalence or a
precise FTP speedup.
Environment: Linux x86_64, AMD EPYC Milan, Rust 1.98.1, release profile with
LTO disabled and 16 codegen units, default Rust allocator. Full paired runs: 20
samples, 1-second warmup, 2-second requested measurement time; FTP repeats: 40
samples and 3-second requested measurement time. Criterion extended sampling
when necessary.
**Local validation limitation:** the environment's registry mirror lacks
versions pinned by the upstream lockfile, including `uuid 1.29.0`. Tests,
Clippy and both benchmark builds use the same uncommitted alternate lockfile in
an isolated source copy. It retains DataFusion 55.2.0 and Arrow 59.3.1 but
changes some transitive versions, including ICU 2.4 to 2.3. The submitted
manifests and lockfile are unchanged. Exact-lock workspace build/Clippy and JVM
integration tests remain for CI; the Maven portion of local `make format` could
not write to the default read-only Maven cache.
Reproduce the benchmark on each source revision with the same dependency
lock and environment:
```sh
cd native
CARGO_PROFILE_RELEASE_DEBUG=0 CARGO_PROFILE_RELEASE_LTO=false \
CARGO_PROFILE_RELEASE_CODEGEN_UNITS=16 \
cargo bench -p datafusion-comet-spark-expr --bench parse_url -- \
--sample-size 20 --warm-up-time 1 --measurement-time 2 --noplot \
--save-baseline main
# Repeat on the PR branch with --baseline main instead of --save-baseline
main.
```
--
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]
