Repository navigation
[Perf][Qwen3-TTS] Enable event-driven orchestration by default - #7088
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. |
|
This PR appears to be related to model: qwen-tts. Model owners: @FayeSpica @Sy0307, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
| return value.strip().lower() in ("1", "true", "yes", "on") | ||
|
|
||
|
|
||
| def _event_driven_orch_default_for_pipeline(pipeline_model_type: str | None) -> bool: |
There was a problem hiding this comment.
??? why this is only related to qwen3-tts? it seems we can valiate this for other models as well without any changes to the codebase
|
Could you extend the validation to other streaming pipelines using For each tested configuration, please include:
This would help establish which pipelines can safely share the default and which should remain opt-in. |
9ae3e2c to
72669b3
Compare
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>
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>
linyueqian
left a comment
There was a problem hiding this comment.
Approving at 98966bb9. The default change is narrow and the precedence is right: _event_driven_orch_enabled now returns the pipeline default only when VLLM_OMNI_EVENT_DRIVEN_ORCH is unset, any explicit value still parses as before, and _event_driven_orch_default_for_pipeline is true for qwen3_tts alone. The default is computed once in OmniEngineBase._set_pipeline_runtime_config, handed to the orchestrator at construction, and read back by the frontend drain through the engine attribute, so the orchestrator loop and the async_omni_base drain cannot disagree for a production engine; the getattr fallback to False only matters for an engine object that never ran that init path, and even then both modes consume the same janus queue with a bounded wait, so the failure would be latency, not a hang. Nothing writes os.environ, so initialising Qwen3-TTS cannot leak the default into another model in the same process, which was the property I most wanted to hold.
The two side changes check out. The dispatch-queue gauge is two integer stores on the orchestrator loop with no lock, the high-water mark is a max rather than a sum, and the post-dequeue update means a peak is not double counted; one small window-rollover artefact is inline as a suggestion. The CudaProfilerWrapper wiring uses the same vllm.profiler.wrapper import the diffusion worker already relies on, and /stop_profile reaches WorkerProfiler.stop through the existing profile(is_start=False) RPC, so the Profiling is not enabled failure described in the PR is closed rather than moved.
Two things before this merges. First, main has moved under vllm_omni/entrypoints/async_omni_base.py since this branch's base (the #7006 abort-path move), so please rebase onto current main and push; the branch carries ready, so a push does not start a build by itself, ping here and I will re-fire the lane. Second, hsliuustc0106's question about validating the switch on other streaming pipelines is still open on the thread; my approval covers the code as scoped (qwen3_tts only, others opt-in), and whether the default should widen is his call to settle with you.
Validation: static read of the worktree against origin/main plus a two-model panel; no PR code executed. The general Buildkite lane is green at this head, and GitHub Actions pre-commit, wheel build and DCO are green.
|
Adding to the rebase ask above: #6849 merged as |
Signed-off-by: Sy03 <1370724210@qq.com>
|
@linyueqian Addressed the main-sync request in ea7de7e by merging main at 8efd3c2 and resolving the typed-config test imports while preserving main's shared abort path. The pipeline default reads Self-review at ea7de7e: checked the merge resolution, pipeline-default propagation, explicit environment override, and dispatch-queue rollover semantics. On H200 with vLLM 0.29.0, 259 related unit regressions passed, and the same Qwen3-TTS CustomVoice streaming smoke passed separately with |
Signed-off-by: Sy03 <1370724210@qq.com>
Signed-off-by: Sy03 <1370724210@qq.com>
Signed-off-by: Sy03 <1370724210@qq.com>
…project#7088) Signed-off-by: Sy03 <1370724210@qq.com> Signed-off-by: y-null <y-null@users.noreply.github.com>
…project#7088) Signed-off-by: Sy03 <1370724210@qq.com>
…project#7088) Signed-off-by: Sy03 <1370724210@qq.com> Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
…project#7088) Signed-off-by: Sy03 <1370724210@qq.com>
Summary
Enable the event-driven orchestration loop by default for the
qwen3_ttspipeline.The default remains unchanged for other pipelines. An explicit
VLLM_OMNI_EVENT_DRIVEN_ORCH=0or1always takes precedence, so operators canstill select the legacy polling loop for A/B testing or rollback.
The PR also adds three low-risk diagnostics/fixes:
and the per-window high-water mark. The high-water mark is updated when a
reader enqueues output, so short bursts are not hidden by a dispatcher that
drains them before the next 1-second sample.
CudaProfilerWrapperwhenprofiler_config.profiler="cuda". Before this,/start_profilesucceeded but/stop_profilefailed withProfiling is not enabled, and Nsight Systemsproduced no capture for Omni AR stages.
warmup,
/start_profile//stop_profile, and Nsight Systems capture mode.Pipeline defaults are kept local to each
AsyncOmniEngine/Orchestrator; theimplementation does not mutate
os.environ, so initializing Qwen3-TTS cannotleak the default into another model in the same process.
Motivation
The event-driven loop avoids the legacy 1 ms per-replica polling cadence and
reduces output wakeup delay for streaming TTS. This is especially relevant to
Qwen3-TTS high-concurrency serving, where the Talker emits frequent codec
chunks and Code2Wav consumes them asynchronously.
Related: #4680, #5221
Scope
Only
qwen3_ttsreceives the pipeline-specific default. Other async-chunkpipelines are intentionally left opt-in because their streaming state machines,
duplex behavior, and output ordering need separate validation.
A parallel dispatcher is not included here. No existing PR was found that
provides a correctness-safe parallel dispatcher for the shared mutable
orchestrator state. Profile evidence shows that current Qwen3-TTS C64
process_outputsandroute_outputare small compared with the legacy pollingcost. The current event-driven implementation still serializes stateful routing,
preserving request/chunk ordering. A future implementation must keep
same-request ordering and cleanup serialized while parallelizing only
independent requests.
The existing multi-API/lane-sharding work is tracked separately in draft #6923;
it should not be conflated with this model-default change.
Validation
Real C64 A/B
Qwen/Qwen3-TTS-12Hz-1.7B-CustomVoiceqwen3_tts_high_concurrency.yaml: stage 0max_num_seqs=64, stage 1max_num_seqs=10seed_tts_smoketext prompts512/512, request throughput17.83-18.67 req/s512/512, request throughput18.61 req/s3274.91 msvs3429.75 ms; P99 E2E4011.66 msvs5392.31 ms0; stage output queues high-water0Host/GPU profile
cuda_api_sum:cudaEventSynchronizewas51.3%,cudaGraphLaunch29.7%, andcudaStreamSynchronize8.4%of CUDA API time.27.8%GPU busy union in the narrow torch window;Code2Wav stage1 had approximately
1.1%GPU busy union and long host waits.next investigation area, not a proven need for parallel output processing.
graph replay; these are diagnostic candidates, not production latency claims.
Tests and checks
46 passedin the final focused run12 passedThe C64 benchmark is a real serving A/B, but the performance delta is based on
few valid on runs and should be treated as directional rather than a stable
guaranteed percentage. A second on-arm restart did not produce a valid result
because stage initialization failed in the shared test environment and was not
counted. The benchmark also exposed an existing streaming continuity issue
under this C64 configuration; that is independent of this default-selection
change.