[FEA] Multi-String Output & LTO Transform Examples - #23173
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
5ab62fb to
9f364ec
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an HTTP log transformation example with regex, precompiled, JIT, and LTO execution paths, embedded request-line parsing kernels, configurable fragment dependencies, relocated embed outputs, updated UDF signatures, and memory-resource propagation. ChangesHTTP log transformations
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Rows | Variant | Cold (s) | Warm (ms) | Warm Mrows/s | Warm GiB/s | Peak (GiB) | Alloc/call (GiB) | Warm speedup |
|---|---|---|---|---|---|---|---|---|
| 1,000,000 | precompiled | 0.0677 ± 0.0002 | 69.364 ± 0.206 | 14.4 | 0.961 | 0.076 | 0.076 | 1.0x |
| 1,000,000 | JIT | 1.4394 ± 0.0066 | 3.128 ± 0.060 | 319.8 | 21.327 | 0.042 | 0.053 | 22.2x |
| 1,000,000 | LTO | 1.5334 ± 0.0041 | 3.200 ± 0.017 | 312.5 | 20.841 | 0.042 | 0.053 | 21.7x |
| 10,000,000 | precompiled | 0.5678 ± 0.0008 | 576.842 ± 0.654 | 17.3 | 1.156 | 0.759 | 0.762 | 1.0x |
| 10,000,000 | JIT | 1.4549 ± 0.0162 | 17.398 ± 0.359 | 574.9 | 38.333 | 0.423 | 0.535 | 33.2x |
| 10,000,000 | LTO | 1.5532 ± 0.0083 | 17.597 ± 0.360 | 568.4 | 37.899 | 0.423 | 0.535 | 32.8x |
| 50,000,000 | precompiled | 3.3342 ± 0.0224 | 3,362.385 ± 17.594 | 14.9 | 0.991 | 3.797 | 3.809 | 1.0x |
| 50,000,000 | JIT | 1.4785 ± 0.0135 | 42.777 ± 0.349 | 1,168.9 | 77.925 | 2.115 | 2.674 | 78.6x |
| 50,000,000 | LTO | 1.5598 ± 0.0181 | 43.013 ± 0.216 | 1,162.5 | 77.496 | 2.115 | 2.674 | 78.2x |
| 149,829,350 | precompiled | 12.0598 ± 0.2866 | 12,087.652 ± 281.887 | 12.4 | 0.873 | 11.378 | 11.413 | 1.0x |
| 149,829,350 | JIT | 1.5285 ± 0.0016 | 101.220 ± 0.676 | 1,480.3 | 104.201 | 6.337 | 8.012 | 119.4x |
| 149,829,350 | LTO | 1.6104 ± 0.0222 | 101.314 ± 0.972 | 1,479.0 | 104.107 | 6.337 | 8.012 | 119.3x |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/src/strings/extract/extract.cu (1)
163-163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
extract_single's temporary buffer still bypasses the caller'smr.auto indices = rmm::device_uvector<string_index_pair>(input.size(), stream); omits
mr, the exact same anti-pattern that Line 88's fix just addressed forextract. The final column at Line 170 correctly usesmr, but this intermediate allocation still goes through the default resource — the memory-resource fix in this PR appears incomplete.🔧 Proposed fix
- auto indices = rmm::device_uvector<string_index_pair>(input.size(), stream); + auto indices = rmm::device_uvector<string_index_pair>(input.size(), stream, mr);As per coding guidelines,
**/*changes should "Add unit tests and unit benchmarks for feature and bug-fix contributions" — worth confirming a test exercisesextract_singlewith a non-default MR (e.g., a tracking/statistics resource) to catch this class of bug going forward.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/src/strings/extract/extract.cu` at line 163, Update the temporary indices allocation in extract_single to pass the caller-provided mr, matching the existing allocation pattern in extract and the final column allocation. Confirm or add coverage that invokes extract_single with a non-default memory resource and verifies the allocation uses it.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/examples/string_transforms/http_logs/fragments.cu`:
- Around line 15-39: Guard malformed request lines before calculating offsets in
fragments.cu ranges 15-39 and 43-72, and in transforms.cpp range 38-93. Update
each duplicated request-line parsing implementation to detect missing separators
or the HTTP/ marker and return the established failure result before assigning
path_size or version_size, preventing negative sizes from reaching the
offset-building scan.
---
Outside diff comments:
In `@cpp/src/strings/extract/extract.cu`:
- Line 163: Update the temporary indices allocation in extract_single to pass
the caller-provided mr, matching the existing allocation pattern in extract and
the final column allocation. Confirm or add coverage that invokes extract_single
with a non-default memory resource and verifies the allocation uses it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 06f2f8bd-5d19-41b5-ac22-1696bad211b3
⛔ Files ignored due to path filters (1)
cpp/examples/string_transforms/http_logs/logs.csvis excluded by!**/*.csv
📒 Files selected for processing (6)
cpp/cmake/Modules/AddFragment.cmakecpp/examples/string_transforms/CMakeLists.txtcpp/examples/string_transforms/README.mdcpp/examples/string_transforms/http_logs/fragments.cucpp/examples/string_transforms/http_logs/transforms.cppcpp/src/strings/extract/extract.cu
…into multi-string-output
- Implemented a CUDA-based URL parser in `fragments.cu` that extracts components such as protocol, host, port, path, query, and fragment from URLs. - Created a CSV log file `logs.csv` containing sample log entries with various URL formats for testing purposes. - Developed a comprehensive transformation utility in `transforms.cpp` that utilizes the URL parser to process log entries and extract URL components. - Added support for multiple transformation methods including regex, precompiled, JIT, and LTO. - Enhanced error handling and input validation in the main execution flow.
|
| Implementation | Throughput (GB/s) | Rows/s | Peak memory (GB) | NSYS total allocations (GB) |
|---|---|---|---|---|
| Regex | 0.092 | 0.696M | 11.39 | 70.64 |
| Precompiled | 2.641 | 19.98M | 38.51 | 357.44 |
| JIT | 34.244 | 259.13M | 5.57 | 22.31 |
| LTO | 33.961 | 256.99M | 5.57 | 22.31 |
Peak memory is the executable's logical RMM high-water mark. NSYS total allocations is
cumulative CUDA allocation traffic across one cold and one warm call; it is not simultaneous
resident memory and can exceed physical GPU capacity.
Compilation latency
These are medians of three 100K-row trials. Each value includes compilation and the measured
transform call:
| Compilation state | Median time |
|---|---|
| JIT, no pre-existing PCH | 1.551 s |
| JIT, live warm PCH | 0.577 s |
| LTO, cold cache | 0.225 s |
The warm-PCH JIT call is 62.8% faster than cold JIT. Cold LTO is 6.9x faster than cold
JIT while retaining equivalent warm throughput. For the PCH comparison, the libcudf kernel
cache and CUDA codegen cache were disabled; both JIT calls compile in one process, allowing
the second call to reuse NVRTC's live automatic PCH without reusing a compiled kernel.
Capacity
The largest verified row count and the immediately following failed probe were measured for
each implementation:
| Implementation | Maximum verified | First failed probe |
|---|---|---|
| Regex | 190M | 200M |
| Precompiled | 82M | 83M |
| JIT | 290M | 291M |
| LTO | 290M | 291M |
JIT/LTO reaches 290M rows with a 20.18 GB executable allocation high-water mark. Precompiled
runs out of allocatable GPU memory at a much lower row count because its sequence of string-operations creates substantially more intermediate data and allocation traffic.
Methodology and environment
- GPU: NVIDIA RTX A6000, 49,140 MiB; driver 595.84; CUDA 13.3
- Tools: NSYS 2026.1.3, NCU 2026.2.1, Linux perf 7.0.12
- Throughput is decimal GB/s:
(input bytes + output bytes) / warm seconds / 1e9
|
Is the regex timing correct in #23173 (comment) above? 300s for 150M rows is much higher than expected. Usually regex is in the 1 GB/s throughput in the worst case. Also the cold JIT times seem a bit too fast in the "benchmark.csv" |
|
@GregoryKimball, the timing is as expected. |
|
/ok to test 8bdc9aa |
URL log transform benchmark resultsThese benchmarks compare the regex implementation, the existing precompiled string-API ThroughputAt 67,108,864 rows—the largest size successfully processed by every implementation—the warmed JIT
Both JIT implementations process approximately 18.7 GiB/s, compared with 1.89 GiB/s for the The JIT implementations also reached 268,435,456 rows successfully. The precompiled implementation Compilation timeCompilation was measured for a one-row input using Nsight Systems and the cuDF NVTX ranges Before every profile:
Warming the PCH reduces CUDA source compilation time by approximately 71%. Using precompiled LTO
The PCH-warmed measurement covers compilation of the two benchmark kernels after the PCH has been Memory PressureAt the largest size supported by every implementation, the JIT paths require substantially less
Relative to the precompiled string-API implementation, the JIT paths reduce peak allocation by All CUDA JIT and LTO JIT warm-up modes have the same measured allocation behavior. The compilation SummaryThe JIT implementation provides roughly a 10× throughput improvement over the existing string-API This makes LTO JIT the strongest overall configuration: near-identical runtime performance to CUDA Profile DataQuestionsWhy is the regex implementation so slow?The regex path is slow because it executes a complex, general-purpose regex program rather than a The main causes are:
This is why regex remains around 0.054 GiB/s while the warmed JIT paths reach approximately This result is specific to this relatively complex extraction workload; it should not be interpreted |

Description
This pull request implements an HTTP request line parser. It compares cuDF's regex API, JIT-transform API, and the new LTO-transform API.
It is intended to illustrate:
This pull request also fixes a bug where the memory resource wasn't being used in the regex extract API
Checklist