cua_s1: add opt-in exact-length CUDA Graph replay - #52
Conversation
|
Reviewed I found one failure-recovery issue worth fixing before merge: [P2] Clear the returned CUDA error after an invalidated capture — I reproduced this by recording an embedding and then calling stream synchronization inside the capture closure. Synchronization returns 900, and EndCapture returns 901. The stream exits capture, but 901 remains in the calling thread's CUDA last-error state. A subsequent valid embedding on that same thread reports stale 901 through Please preserve the original return code and clear the already-reported error inside this CUDA library, after graph cleanup: if (graph) cudaGraphDestroy(graph);
if (e != cudaSuccess) (void)cudaGetLastError();
return e;The exact failing probe passes with this isolated change, including when the recording closure propagates the synchronization error and when EndCapture supplies the error. NVIDIA documents last-error state per host thread and CUDA runtime instance; clearing through an unrelated dynamically loaded runtime would not be sufficient here. The current regression's immediate Rust Independent validation of the unchanged head:
Two small follow-ups: For my next CUDA contribution, I opened #53 to export multimodal reference tensors, including image features, language inputs and 3D position IDs. With Graph work underway in #52/#50, would you prefer a follow-up adding native language-model support for image embeddings and 3D rotary positions, using #53 as the correctness reference, or is there a specific CUDA kernel or validation gap you would like me to prioritize? |
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
6d2a96c to
30ae42a
Compare
|
Reviewed Correctness looks good:
The cost is the first request at each length, which runs the forward pass twice (the eager warm-up, then the replay). Graph mode, milliseconds, mean of the two starts:
On this card the replays ran at eager speed: over those 32 lengths the repeats averaged 100.1 and 102.9 ms in the two graph starts, against 102.5 ms eager (a first eager start ran about 11% faster throughout, so I left it out of this comparison). With the 240 other lengths sent once each, the mean was 209.6 ms with graphs against 99.9 ms eager, about twice, and the 8-client run took 65.5 s against 32.4 s. Cua-S1 prompts carry the current screen state, so I'd expect exact lengths to repeat rarely in real use. A small change would remove most of this: on a miss, return the eager result and capture without replaying it, since capture only records. Here the first request cost two forward passes and nothing measurable beyond that (first minus twice the repeat time averaged under 0.5 ms in both starts), so capture itself is cheap. A length cap would also help: the 16,384-token prompt took 3.3 s the first time against 1.6 s eager, and your numbers show no gain for long prompts. Keeping the most recently used lengths instead of the first ones in would help when a few lengths do repeat. Minor points:
On the overlap with #50: let's go with this PR for the graphs. Once it lands, I'll rework #50 on top of it to keep only the GEMM timing, or close it if that isn't wanted. |
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Purpose
Adds opt-in CUDA Graph replay to the native Cua-S1 text worker from merged #19. Rebased onto
mainata6f7b23.Repeated native Cua-S1 requests currently launch the entire prefill eagerly. Setting
CUA_S1_GRAPH=1warms the GEMM plans and captures the forward pass for each exact prompt length, then replays it with newly uploaded token ids. Up to eight captures are retained with FIFO eviction; scratch growth invalidates them before freeing buffers. Eager execution remains the default. The CUDA runtime wrapper owns graph cleanup, and the worker/library ABI moves to 3, requiring both to be rebuilt together.Scope is CUDA Graph replay only. Kernel arithmetic and GEMM selection are unchanged. Benchmark evidence and reproduction commands are in
docs/benchmarks/cua-s1-cuda-graphs/.Test Plan
cargo fmt --all --checkcargo clippy --workspace --locked --all-targets -- -D warningscargo test --workspace --lockedcargo build --workspace --release --lockedSystem1-Omni Version / Commit: benchmark base
8367333; benchmark implementation61238a0; current review fixesa55a111onmainata6f7b23. The original latency measurements precede capture-failure cleanup and later test/documentation changes; the latency benchmark was not rerun for this recovery fix.Test Result
Formatting, Clippy, release build and CUDA compilation passed. CI passed on the current head
a55a111. CPU suite: 19 tests passed, 3 GPU tests ignored. Full-model comparison: all 17 response bodies matched exactly, including probabilities and choices.Warm p50 HTTP latency in milliseconds, both runs:
The 139–292-token cases improved 6.6–11.4% in the mean of run medians; the long case had no meaningful gain. First post-readiness fixture: 25.23 ms eager versus 33.66 ms Graph, reflecting capture overhead. These findings apply to this fixed workload and L20X, not the B300/27B chart. Compact raw latency samples, warmups, the 17/17 correctness summary, controls and limitations are checked in; full response bodies are retained locally.
Review-fix validation on 2026-09-30: the stronger capture-invalidation regression fails against the original CUDA library with stale error 901 at embedding recapture, then passes with the cleanup fix. All three GPU tests pass on scheduler-reserved GPU 3 (NVIDIA L20X, sm_89), CUDA 13.0, driver 570.133.20, Rust 1.98.1. Formatting, Clippy, all 19 CPU tests, workspace release build and CUDA compilation also pass. The library clears already-reported CUDA last errors at graph begin/end/launch while preserving each original return code.
Minor-review validation on 2026-10-01: capture failure now returns the completed eager result, logs the error, releases cached graphs and disables graphs for that worker lifetime, avoiding retries. On the same reserved L20X, the expanded GPU regression covers begin and launch argument errors followed by successful recapture; it fails against the previous library at the begin-error recovery and passes with the fix. All three GPU tests and all 19 CPU tests pass; formatting, Clippy, release build and CUDA compilation pass. Separate task-owned fault-injection libraries force begin and end failures in the real worker: each produces 17 HTTP responses byte-for-byte equal to the eager control and exactly one failed capture attempt, with no retries across later requests. Fault copies and raw results are kept outside the PR source. The recipe and benchmark docs now distinguish scratch growth from shorter prompts reusing the allocation.
Unverified in this local validation: the normal successful-Graph full-model comparison and latency benchmark were not rerun for these recovery fixes; full fp32-reference accuracy, concurrency, other GPUs/CUDA versions, Compute Sanitizer and frontend overhead were not evaluated. The original successful-Graph 17/17 full-model comparisons and latency samples above remain historical evidence.
Self-review