Repository navigation
[Bugfix][Metrics] Measure the codec stage TTFT and phase split after talker first audio - #9097
MinhaoLi0318 wants to merge 2 commits into
Conversation
…rst audio With talker first audio (the default Qwen3-TTS deploy: MRV2 + async_chunk), the orchestrator feeds the talker's first audio frame into the codec stage's output processor without IterationStats. Processing that frame clears req_state.is_prefilling but records no stats, so the codec stage's first engine-core output was recorded as a decode step: no first-token timestamp, vllm_ttft_ms left at its default 0.0, and vllm_prefill_ms / vllm_decode_ms (vllm-project#8575) missing. On an H100 run of Qwen3-TTS (--async-chunk --log-stats) this held for all 18 requests of two server boots; --no-async-chunk was complete. The same path affects the IterationStats passed to the stat logger for the codec stage: no TTFT sample, an ITL measured from a last-token timestamp of 0, a negative prefill time, and a decode time measured from a first-token timestamp of 0. This part was verified only in unit tests, at the IterationStats level, not on a /metrics scrape. Give the native stats their own prefill flag, cleared only by an output that is recorded with stats. Replace the vllm-project#8575 test that expected the split to be missing with tests on both output paths and through the orchestrator's first-audio flush. Older tests that set req_state.is_prefilling by hand no longer affected the native stats: they now set native_is_prefilling where they need the decode path, and otherwise leave the flag to the code under test. In the metrics doc, say that a non-final AR stage under MRV2 + async_chunk (the Qwen3-TTS talker) emits one output at stop, so its prefill covers its decode steps, and that the first-audio frame does not end the codec stage's prefill. Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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 belong to: docs/design/module/engine_orchestration.md, docs/design/module/input_output_modality_contracts.md, docs/design/module/entrypoints.md. Module owners: @Gaohan123 @alex-jw-brooks @amy-why-3459 Routing: @Gaohan123 via module of the changed files, CODEOWNERS; @alex-jw-brooks via module of the changed files; @amy-why-3459 via module of the changed files @MinhaoLi0318, 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. |
amy-why-3459
left a comment
There was a problem hiding this comment.
Thanks, a separate native_is_prefilling flag is a clean way to keep the stats-less first-audio flush (_flush_upstream_first_audio calls process_llm_raw_outputs(replica_id, OmniEngineCoreOutputs(outputs=[first_output])) with no iteration_stats) from ending the codec stage's prefill, and resetting it in apply_streaming_update together with native_text_stats keeps multi-segment requests consistent.
One gap on the path where codec outputs overtake the first-audio frame, see inline.
| The first engine-core output ends prefill even when it carries no token. A generation stage that emits one output when it stops therefore reports its whole run as `vllm_prefill_ms`, with a measured `vllm_decode_ms` of `0`. | ||
| For an AR stage that emits an output every step, the first engine-core output carries its first token. The first engine-core output ends prefill even when it carries no token. A generation stage that emits one output when it stops therefore reports its whole run as `vllm_prefill_ms`, with a measured `vllm_decode_ms` of `0`. The same holds for an AR stage that does not emit per-step outputs: with Model Runner V2 and `async_chunk`, a non-final AR stage (for example the Qwen3-TTS talker) emits one output when it stops, so its `vllm_prefill_ms` also covers its decode steps, and its `vllm_ttft_ms` is the time to that output. | ||
|
|
||
| With talker first audio (`talker_first_audio`), the orchestrator feeds the upstream stage's first audio frame into the codec stage's output processor. That frame is not an engine-core output of the codec stage and does not end its prefill: the codec stage's split, its `vllm_ttft_ms`, and the samples it adds to the wrapped vLLM histograms start at the codec's own first engine-core output. |
There was a problem hiding this comment.
This holds for the in-order case the new tests cover, but not when codec outputs arrive before the first-audio frame. _route_upstream_first_audio parks those in req_state.pending_first_audio_outputs, and _flush_upstream_first_audio later replays each one as OmniEngineCoreOutputs(outputs=[output]). That wrapper has no timestamp, so EngineCoreOutputs.__post_init__ stamps it with time.monotonic() at replay time and the engine-core timestamp of the batch it originally arrived in is lost.
With this change the first replayed output is now the one that ends prefill, so in this case:
vllm_prefill_ms(and the wrapped prefill histogram) includes the wait for the upstream frame, whileQUEUED/SCHEDULEDkeep their original event times;- several parked outputs replayed back-to-back get near-identical timestamps, so their ITLs are ~0 and the real gap lands on whichever output comes next.
vllm_ttft_ms is measured at replay regardless, since _process_llm_stage_outputs creates the IterationStats then; that may be the intended user-facing meaning, but it would help to say so here.
Could the original raw_outputs.timestamp be stored with each parked output and passed through on replay (OmniEngineCoreOutputs(outputs=[output], timestamp=ts)), plus a test where the codec's first output arrives before the first-audio frame?
There was a problem hiding this comment.
Confirmed on the previous head: the output parked by _route_upstream_first_audio lost its batch's timestamp, and the wrapper built in _flush_upstream_first_audio was stamped at replay time. So when codec outputs overtook the first-audio frame, prefill included the wait for the frame and outputs replayed back to back had ITLs of about 0.
Fixed in 74d2606ef:
- Each parked output is stored with
raw_outputs.timestampand replayed asOmniEngineCoreOutputs(outputs=[output], timestamp=ts). I kept one wrapper per parked output, as before, so replay order and terminal handling do not change. The in-order path is untouched. docs/design/metrics.mdnow says that held outputs keep the engine-core timestamps of their batches, and: "vllm_ttft_msand its histogram sample include that wait: as in vLLM, TTFT runs from the request's arrival to the time the output processor handles the first output, which for a held output is after the frame." TTFT can't use the engine-core timestamp anyway: it is measured against the wall-clock arrival time, and engine-core timestamps come fromtime.monotonic().- New test
test_codec_outputs_parked_for_first_audio_keep_engine_core_timestamps: the codec's first two outputs (engine-core times 1.75 and 2.25,SCHEDULEDat 1.25) arrive before the first-audio frame, and a terminal output at 3.0 arrives after it. It checks prefill 500 ms, decode 1250 ms, ITLs of 500 ms and 750 ms, and a TTFT of 4 s taken at replay (with the wall clock patched). It passes with the fix. On the previous head it fails:vllm_prefill_msis the replay-timetime.monotonic()minus theSCHEDULEDtime, andvllm_decode_msis left out as negative.
Local CPU run, not the CI image: pytest tests/engine/test_output_processor.py tests/metrics tests/engine/duplex/test_engine_session.py tests/engine/test_orchestrator.py tests/engine/test_orchestrator_first_audio.py gives 518 passed (517 on the previous head).
Thanks for the careful read.
AI assistance: drafted with Claude Code; I reviewed the change and this reply.
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
… for first audio When codec outputs overtake the talker's first-audio frame, the orchestrator parks them in pending_first_audio_outputs and, after the frame, replays each one in a new OmniEngineCoreOutputs. That wrapper had no timestamp, so EngineCoreOutputs.__post_init__ stamped it with time.monotonic() at replay time. Since the previous commit the first replayed output ends the codec stage's native prefill, so in this order vllm_prefill_ms and the wrapped prefill histogram included the wait for the frame, while QUEUED/SCHEDULED kept their engine-core times. Parked outputs replayed back to back also got near-identical timestamps: their ITLs were about 0 and the real gap moved to the next output. Park each output with the timestamp of the batch it arrived in and pass that timestamp to the replay wrapper. Keep one wrapper per parked output as before, so replay order and terminal handling do not change; outputs from the same batch get the same timestamp either way. The in-order path is unchanged. vllm_ttft_ms is still taken at replay: IterationStats is created there, and, as in vLLM, TTFT runs from arrival to the time the output processor handles the first output. Say so in docs/design/metrics.md. Add a test where the codec's first two outputs arrive before the first-audio frame. On the previous commit it fails: vllm_prefill_ms is the replay-time monotonic clock minus the SCHEDULED time, and vllm_decode_ms is left out as negative. Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
amy-why-3459
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. Looked at 74d2606 (the delta since my last review):
- Each parked output now carries
raw_outputs.timestampof the batch it arrived in and the replay wrapper is built withOmniEngineCoreOutputs(outputs=[output], timestamp=timestamp), soEngineCoreOutputs.__post_init__no longer re-stamps it with replay-timetime.monotonic(). The prefill end and the ITLs now come from engine-core time, and same-batch outputs share a timestamp as they would on the in-order path. - The tuple shape change is consistent: the append in
_route_upstream_first_audioand the unpack in_flush_upstream_first_audioare the only producer/consumer ofpending_first_audio_outputs. - The new test covers the out-of-order case I asked about (two codec outputs parked before the frame) and checks prefill 500 ms, ITLs [500, 750] ms, and TTFT measured at replay (4000 ms), which matches the clarified wording in
docs/design/metrics.md.
No further concerns on this delta.
Contributor intent (required human answers)
1. What outcome do you want to achieve?
I want the codec stage of Qwen3-TTS, under the default async-chunk deployment, to record TTFT correctly and to provide a usable prefill/decode latency split, so that users can use it to analyze performance and locate bottlenecks.
2. What is the highest-leverage variable for that outcome?
Whether the prefill boundary of the native stats is accurate. In particular, the codec's first output that is recorded with stats must be handled as prefill; the first-audio frame that arrives before it without stats must not end that phase early.
3. How will this work move that variable?
I keep a separate prefill state for the native stats, which only an output carrying
IterationStatsclears, and which is reset together with them on a streaming update. This way the first-audio frame no longer ends the prefill phase in the stats, and the existing meaning ofis_prefillingstays unchanged.4. Whose customer or user needs does this address, and what are those needs?
Mainly users who run the default Qwen3-TTS deployment and use
--log-statsto investigate latency. They need reliable codec TTFT and prefill/decode data to tell which phase the time goes to, and to compare before and after an optimization.Is a PR needed? Yes. This is a real stats bug on the default deployment path, reproduced on an H100, and the fix comes with regression tests, so it should go upstream as a follow-up bugfix to #8575. The PR states the validation scope as it is: the fix has not been re-run on GPU yet, and the histogram part was verified only in unit tests, without scraping
/metrics.Purpose
Follow-up to #8575 (merged as
0d4d09cf7), which added the queue/prefill/decode split toStageRequestStats.Before. With talker first audio (#8163), the codec stage (stage 1) reports
vllm_ttft_msas 0.0 and novllm_prefill_ms/vllm_decode_msfor every request. This is the default Qwen3-TTS deploy:vllm_omni/deploy/qwen3_tts.yamlsetsmodel_runner: v2,async_chunk: trueandtalker_first_audio: true.qwen3_tts_mrv2.yaml,qwen3_omni_moe_mrv2.yamland the threeqwen3_tts_high_concurrency*.yamldeploys also enabletalker_first_audio, so they should take the same path; I did not run them.On one H100 with
Qwen/Qwen3-TTS-12Hz-1.7B-CustomVoice(--async-chunk --log-stats --enforce-eager), this held for 18 of 18 requests over two server boots. With--no-async-chunk, all fields were present (9 of 9). These numbers were measured with an earlier head of #8575 (07dfd2ee8, whosevllm_omni/and docs diff is identical to the merged0d4d09cf7) on base198c21059, which already contains #8163, not on current main.vllm_omni/engine/orchestrator.pychanged by 714 lines between that base and current main2c72dea3a(575 added, 139 removed, including #8675, which made event-driven orchestration the default). This change has not been run on GPU.Cause.
Orchestrator._flush_upstream_first_audiofeeds the talker's first audio frame into the codec stage's output processor withoutIterationStats(orchestrator.pyL1043 on main; the regular path at L938-L943 passes one). That output records no stats, but it still clearsreq_state.is_prefilling: in_process_mm_only_outputs(vllm_omni/outputs/output_processor.pyL794) on the multimodal path, and unconditionally in vLLM'sprocess_outputson the text path._update_stats_from_outputthen readswas_prefillingasFalse(L837) for the codec's first engine-core output and records it as a decode step. Its first-token timestamp is never set,vllm_ttft_mskeeps its 0.0 default, and the split is left out. #8575 reported the missing interval asNoneand had a test expecting that, but the interval is measurable.The
IterationStatspassed to the stat logger for the codec stage have been affected in the same way since #8163: no TTFT sample, an ITL measured from a last-token timestamp of 0, a negative prefill time, and a decode time measured from a first-token timestamp of 0. I checked this only at theIterationStatslevel in the unit tests below, not on a/metricsscrape.After.
OmniRequestStategetsnative_is_prefilling, a prefill flag fornative_text_stats. Only an output recorded with stats clears it, and a streaming update resets it together with the native stats.req_state.is_prefillingkeeps its upstream meaning. When every output carries stats, the two flags are equal at every step, so stages without first audio report the same values as before. The codec stage's split,vllm_ttft_msand histogram samples now start at its own first engine-core output.Contract.
docs/design/metrics.md, section "LLM-stage phase split inStageRequestStats":vllm_prefill_msrow drops "(the first token for an AR stage)".async_chunk, a non-final AR stage (for example the Qwen3-TTS talker) emits one output when it stops, so itsvllm_prefill_msalso covers its decode steps; the first-audio frame does not end the codec stage's prefill.vllm_ttft_msand its histogram sample include that wait, because TTFT is taken when the output processor handles the first output.Field names, the debug table and the Prometheus families are unchanged, so no user guide or example changes.
Codec outputs that arrive before the first-audio frame (second commit, from review). The orchestrator holds such outputs in
pending_first_audio_outputs(orchestrator.pyL1009-L1017) and, after the frame, processes each one in a newOmniEngineCoreOutputs(L1057-L1071). That wrapper had no timestamp, soEngineCoreOutputs.__post_init__stamped it withtime.monotonic()at replay time: in this order the codec'svllm_prefill_msand the wrapped prefill histogram included the wait for the frame, and outputs replayed back to back had ITLs of about 0. Each held output now keeps the engine-core timestamp of the batch it arrived in, and the replay passes it on; the in-order path is unchanged.vllm_ttft_msis still taken at replay: as in vLLM, TTFT runs from the wall-clock arrival time to the time the output processor handles the first output, and engine-core timestamps come fromtime.monotonic().Related.
native_text_statslines inOmniRequestStateand a block at the same place in_update_stats_from_output. Whichever PR lands second gets a three-hunk text conflict inoutput_processor.py; keeping both sides resolves it. Run 5 below tests that resolution.git merge-treeagainst main2c72dea3aand against this commit (all open PRs updated since 2026-10-17, plus [Perf][Sched] Multi-step merged decode for host-bound AR stages #7064, [Perf][Duplex] Model-agnostic duplex serving and stage-output fixes #8405, [Core][Frontend] Add OpenTelemetry tracing for Omni inference #8836, [Bugfix][Frontend] Correct and extend streaming audio continuity metrics #8878 and [Feature / Core] Support Independent Stage Execution & External Routing #6883): this commit adds a conflicting file only for [Core] Split input wait out of vllm_queued_ms in StageRequestStats #9050. [Core] Align vLLM-Omni with vLLM v0.32.0 #9066 (vLLM v0.32.0 alignment), [Model][Core] Add native Lychee-FD support for Unified Duplex and MRV2 #8635, [Model] Add MOSS-TTS Local single-stage fused serving #8864 and [Feature] Support HunyuanImage-3.0 prefill-decode (PD) disaggregation #7217 changeoutput_processor.py, [Core] Route stages through configured pipeline transitions #8745 changestest_orchestrator_first_audio.py, and [Bugfix][Frontend] Correct and extend streaming audio continuity metrics #8878 and [Core][Metrics] Leave unobserved diffusion timings out of per-request metrics #9049 (mine) changedocs/design/metrics.md; none of them conflicts with this commit in those files. [Perf][Sched] Multi-step merged decode for host-bound AR stages #7064 and 13 PRs opened between 2026-07 and 2026-09 already conflict with main inoutput_processor.py,test_output_processor.pyormetrics.md. [Core][Frontend] Add OpenTelemetry tracing for Omni inference #8836, [Perf][Duplex] Model-agnostic duplex serving and stage-output fixes #8405, feat(core): add TransferPayload and ChunkMeta typed contracts for cross-stage execution #9014 and [Feature / Core] Support Independent Stage Execution & External Routing #6883 change other parts oforchestrator.py. [Core] Route stages through configured pipeline transitions #8745 also changes how_flush_upstream_first_audiopicks the codec stage; on a local merge of [Core] Route stages through configured pipeline transitions #8745 (539648e79) and this branch,test_orchestrator_first_audio.py,test_output_processor.pyandtest_orchestrator.pypass (250 passed). None of these PRs changes how the output processor handlesis_prefilling.Test Plan
vLLM Version: v0.31.0 (
db9527a468, theVLLM_VERSIONindocker/Dockerfile.ci), used as a source tree onPYTHONPATHwithout compiled extensions.vLLM-Omni Commit:
74d2606ef(2 commits on upstream/main2c72dea3a)Environment: macOS 26.6 (arm64), Python 3.12.14, torch 2.13.0 CPU, pytest 9.1.1. No GPU; not the CI image. Other dependencies come from an existing local virtualenv and from wheels already unpacked in the local uv cache, put on
PYTHONPATH; nothing was installed for these runs.Runs 1 and 2 were repeated on upstream/main
2c72dea3a, and run 5 on #9050 alone (rebased on2c72dea3a).Lint with the versions pinned in
.pre-commit-config.yaml, on the changed files: ruff 0.14.10checkandformat --check, typos 1.49.0, markdownlint-cli2 0.23.2 ondocs/design/metrics.md, mypy 1.11.1 throughtools/pre_commit/mypy.py 3.10,tools/pre_commit/{check_test_marks,check_spdx_header,check_forbidden_imports,check_torch_cuda}.py, andgit diff --check.Test Result
Errno 63), Model Runner V2 requiring Triton, no Omni AR worker class on this CPU platform (NotImplementedError), and no/bin/trueon macOS.vllm_prefill_msandvllm_decode_msare left out. So do 4 IDs of 3 older tests: they no longer setreq_state.is_prefillingby hand, because it no longer affects the native stats, and they now expect the code under test to clear the flag.test_first_output_without_stats_leaves_prefill_and_decode_out, which expected the split to be missing and is replaced here.SharedMemoryConnectortests intest_chunk_transfer_adapter.pyon both sides, and they also fail on main. They need/dev/shm, which macOS does not have.test_codec_outputs_parked_for_first_audio_keep_engine_core_timestampsalso fails on the first commit alone (1505d9505):vllm_prefill_msis the replay-timetime.monotonic()minus theSCHEDULEDtime, andvllm_decode_msis left out as negative. Run 1 on1505d9505: 517 passed. Run 2 on this branch has the same 44 failing and erroring test IDs as on1505d9505.tools/pre_commitchecks andgit diff --checkare clean, and markdownlint reports 0 issues. mypy reports the same 14 errors on this branch and on main for the first commit's files; for the second commit's files it reports 1 error, at an unchanged line oftest_orchestrator_first_audio.py(L267) that has the same error on main.Not run:
/metricsscrape of the stage-1 histograms.talker_first_audio.AI assistance: I used Claude Code to implement and test this change and to draft this description; I reviewed every change and can explain it.