Repository navigation
[Perf][VoxCPM2] Add unified decode graph optimization - #4606
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
c8b1747 to
66b5174
Compare
|
Thanks for this @Sy0307. I ran an end-to-end A/B on the current head ( [Blocker] Decode tail graph degrades audio at batch sizes above 8Method. Identical texts, identical hardware and environment; the only change between the two columns is
The baseline on the exact same serving path is clean, so this is not a harness or environment artifact. Isolated to the tail graph. Disabling only the unified graph while keeping the tail graph on still degrades at 16 concurrent (mean WER 0.29), whereas the bs=1 unified graph is clean in every run. So the regression is specifically the decode tail graph at batch sizes above 8. The output is genuine synthesis drift, not ASR noise. Two examples (graphs ON, 16 concurrent):
This is the kind of degradation a bytes-per-request check cannot catch, since the degraded audio is still the right length. Repro. Serve the default Suggestion. Gate or fix the decode tail graph for batch sizes above 8 before enabling it by default (the default ships [Minor, unconfirmed] One worker crash during graph captureIn one early offline run the engine worker died (hard CUDA abort, no Python traceback) right after the bs=8 tail-graph capture. It did not reproduce in roughly seven later runs and coincided with unrelated GPU load on the shared box, so it is most likely transient contention rather than a defect; I could not confirm it. Noting it only for awareness. The new try/except capture-to-segmented fallback is a good safety net, though it will not catch a hard CUDA abort. |
hsliuustc0106
left a comment
There was a problem hiding this comment.
Unified decode graph optimization looks correct.
|
The latest patch now defaults VoxCPM2 to the full unified decode graph path and removes the decode-tail graph path, so we only keep one graph optimization strategy. Based on the c=64, prompts=500 benchmark sweep, large admission batches caused ITL to grow quickly; we set the deploy default to |
|
Thanks for iterating so quickly @Sy0307. Removing the tail graph and consolidating on a single strategy was the right call. I went through the latest head end to end on an H200-class card, including fixing the boot crash locally so I could exercise the new path. Notes below. 🔴 [blocking] The default config crashes at startup, with a root cause and a fixServing the default What worked for me: compile the capture estimator and feat-encoder fresh from the eager module, via the existing helper that disables inductor cudagraphs during compile, instead of reusing _est_eager = getattr(original_estimator, "_orig_mod", original_estimator)
capture_estimator = self._voxcpm2_compile_without_inductor_cudagraphs(_est_eager, mode=None, fullgraph=False)
tts.feat_decoder.estimator = capture_estimator
# same for feat_encoder via _orig_modAfter that the unified graph captures bs=1..32 cleanly and serves normally. Happy to send the full diff. 🟢 Quality looks good at scaleWith the boot fix in place, I checked that the unified graph stays intelligible as batch grows: ASR word error rate 0.006 at bs<=8 and bs=16, and 0.000 at bs=32. So the unified path does not show the high-batch divergence the earlier tail-graph path had. ❓ [question] On throughput I could not separate the graph from batching on this hardwareI tested on an H200-class card, which runs roughly 2x the throughput of the H20 in your table, so batched-eager here already lands where your graph numbers are, and I cannot isolate the graph's marginal value from this hardware. Sustained req/s (median of reps), unified graph vs batched-eager at the same config:
Within noise here. Separately, turning batching off ( 🟡 [design] The shared model-runner change is large for a single-model gain
Where I land
|
| DEPLOY = REPO_ROOT / "vllm_omni" / "deploy" / "voxcpm2.yaml" | ||
|
|
||
|
|
||
| def test_ar_runner_exposes_runner_assisted_full_metadata_hook(): |
There was a problem hiding this comment.
This runner-assisted path needs at least one behavioral test. These checks mostly grep source strings, so the metadata request, padding, and context reset logic can break while this test still passes.
| enable_loc_dit_skip_qkv_contig: true | ||
| enable_loc_dit_reduce_overhead_no_cg: true | ||
| decode_graph_capture_policy: power2 | ||
| enable_unified_decode_graph: true |
There was a problem hiding this comment.
This enables the new graph path by default, but the PR evidence only checks success and output byte counts. Please add baseline-vs-PR audio quality validation for the same prompts/seeds before making this the default.
|
Benchmark update for the latest PR head ( I re-ran an apples-to-apples E2E benchmark on the H20 remote box with
The main improvement is throughput and request completion latency: wall mean drops by ~59% and throughput is ~2.4x higher. TTFB is more mixed under I also checked the PR server logs. The full unified graph path was actually used and captured max-64 buckets during warmup, including |
|
Correction to my previous comment: you are right, and I can now reproduce the win. After the boot fix (thanks for picking that up), I re-ran on an H200-class card at your
That is ~2.2x from the graph, in line with your 2.4x on H20. My earlier "within noise / it is mostly the batching" was a batch-size artifact: I had only measured up to bs=32, where the graph and batched-eager are tied on this card, so I missed the win that only shows up at the large decode batches the PR actually targets. It grows with batch because the segmented path still does per-request Python work each step, which the unified graph collapses into a single replay. Quality at bs=64 is clean as well (ASR word error rate 0.000 over 64 prompts), which also covers @hsliuustc0106's request for baseline-vs-PR audio validation before defaulting. And the boot fix holds: the unified graph captured bs=1..64 with no So the throughput question is settled in the PR's favor. The remaining items are the smaller ones already raised: the shared-runner footprint (still about +196/-47 and VoxCPM2-only, so a |
|
need to fix dco |
|
Please fix pre-commit too. |
|
|
||
| return False | ||
|
|
||
| def _voxcpm2_unified_decode_graph_admission_deferral_enabled(self) -> bool: |
There was a problem hiding this comment.
can this method be applied to other models?
Signed-off-by: Sy03 <1370724210@qq.com>
Signed-off-by: Sy03 <1370724210@qq.com>
Signed-off-by: Sy03 <1370724210@qq.com>
Signed-off-by: Sy03 <1370724210@qq.com>
Signed-off-by: Sy03 <1370724210@qq.com>
302f4e9 to
1d9cc60
Compare
Signed-off-by: Sy03 <1370724210@qq.com>
Added. |
|
Confirmed fixed on my side too. I re-ran the same A/B on the latest head
WER is a whisper round-trip on the same sentences; every OFF control is 0.000, which validates the scorer. So the batched (bs>1) corruption is gone (default was 0.74 to 0.80 before, bs=64 was 0.98), and the bs=64 unified path now runs about 2x over batched-eager instead of collapsing. Your root cause matches what I saw: the pre-capture path was capturing bs>1 graphs while the forward context still held bs=1 attention metadata, and removing that (lazy capture under runner-assisted FULL metadata) fixes both. Two small notes, neither a blocker:
Thanks for the quick turnaround on the fix. |
Signed-off-by: Sy03 <1370724210@qq.com>
| scheduler rule. | ||
| """ | ||
|
|
||
| def _unified_decode_graph_enabled(self) -> bool: |
There was a problem hiding this comment.
This gate does not match the effective model-side default. _VoxCPM2RuntimeConfig.from_vllm_config() returns the dataclass defaults when no voxcpm2_runtime_config is present, and that default currently has enable_unified_decode_graph=True; it also disables the graph when deterministic_cfm_noise is true. This scheduler only checks hf_config.voxcpm2_runtime_config, so direct/non-deploy launches can have the model trying to use unified graphs without the deferral policy, while deterministic-noise launches can still defer waiting even though the model cannot use the graph. Please derive this from the same normalized runtime config/source as the model so the admission policy matches the actual graph availability.
|
One remaining issue: the VoxCPM2 scheduler gate for unified decode graph does not match the model-side runtime config/defaults, so some launch paths can enable the graph without the admission policy, while deterministic-noise launches can defer even though the graph is disabled: #4606 (comment) The earlier core-module concern looks addressed: the shared scheduler now only exposes a generic deferral hook, and the VoxCPM2 policy is model-local. I did not see another model-specific leak in the shared runner hooks. Current gate risk: AMD/Intel/NPU Buildkite are still failing and the main Buildkite lane is pending. |
Thx and will handle it. |
Supersedes #4594 after branch rename to dev/voxcpm2-unified-decode-graph.
Purpose
This PR improves VoxCPM2 TTS serving latency by reducing Python routing and kernel launch overhead in the decode path.
VoxCPM2 decode currently crosses multiple small stages per step, including scaffold decode, FSQ/fusion, residual decode, CFM solve, feature encoding, stop prediction, and audio collection. The segmented path leaves overhead between these stages, especially under high concurrency.
This PR adds CUDA graph support for the VoxCPM2 decode path while keeping the default behavior quality-safe.
Key changes:
Review Follow-up
This branch keeps the runner changes opt-in and fail-closed:
get_runner_assisted_full_attention_metadata_requestkeep the normal runner path.Test Plan
ruff check .ruff format --check .python3 -m py_compile vllm_omni/model_executor/models/voxcpm2/voxcpm2_talker.py vllm_omni/worker/gpu_ar_model_runner.py tests/model_executor/models/voxcpm2/test_runner_assisted_unified_graph_static.pypython3 tests/model_executor/models/voxcpm2/test_runner_assisted_unified_graph_static.pyvLLM Version:
0.23.0
vLLM-Omni Commit:
d295fcd
Test Result
H20 benchmark, VoxCPM2 HTTP serving,
c=64.Kernel/profile evidence:
Latest review-follow-up smoke on H20, c=64:
64/64 success,0 errors,Bytes mean=544.2KB.TTFB mean=1.099s,Wall mean=4.478s,Throughput=11.8 req/s,Bytes mean=513.0KB.Additional validation:
64/64 success,0 errors,Bytes mean=525.9KB.TTFB mean=1.101s,Wall mean=4.566s,Throughput=11.7 req/s,Bytes mean=525.5KB.TTFB mean=1.060s,Wall mean=4.563s,Throughput=12.1 req/s,Bytes mean=527.8KB.Runner-assisted bucket logs confirmed non-power-of-two active batches are padded into bounded graph buckets, for example:
num_reqs=47 padded_reqs=48num_reqs=23 padded_reqs=24num_reqs=55 padded_reqs=56This avoids the previous exact-size/power-of-two fallback pattern without pre-capturing every batch size.
PTAK @linyueqian @hsliuustc0106