Skip to content

[Model] Prepare MiniCPM-o 4.5 Code2Wav reference voices before chunk 0 and batch more rows per solve - #9250

Open
chickeyton wants to merge 7 commits into
vllm-project:mainfrom
chickeyton:duplex_perf
Open

chickeyton wants to merge 7 commits into
vllm-project:mainfrom
chickeyton:duplex_perf

Conversation

@chickeyton

@chickeyton chickeyton commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Contributor intent (required human answers)

1. What outcome do you want to achieve?

optimize vllm-omni performance for MiniCPM

2. What is the highest-leverage variable for that outcome?

Code2Wav throughput on one GPU under voice-cloning load. From C=8 up it is the stage that caps served
codec frames/s, and 28–44% of its busy time went to preparing each new request's reference voice inside
the chunk-0 step that every live stream waits on.

3. How will this work move that variable?

It moves reference-voice preparation out of the chunk-0 step: a worker thread does the host half when the
request arrives, and the device half runs on a side stream before chunk 0. It also batches more rows per
Code2Wav solve and merges first chunks into the same solve.

4. Whose customer or user needs does this address, and what are those needs?

Operators serving MiniCPM-o 4.5 voice cloning or TTS to many concurrent users on a single GPU. They need
more concurrent streams per GPU, and lower first-audio latency under load, without losing speech quality.
The benefit is opt-in through a deploy profile, so other deployments are unaffected.


Purpose

Before

On one GPU with Seed-TTS voice cloning (a different reference voice per
request), Code2Wav is the throughput limit from C=8 up: its CUDA stream is
busy 100% of the time, and its codec-token rate equals the served rate.
Measured by per-step timing of each stage's engine core (one L20X, MPS on):

  • New requests cost 28-44% of Code2Wav's busy time. A least-squares fit of
    step time gives 35-48 ms per new request against 0-8 ms per continuing
    stream. Each new request's reference voice is prepared on chunk 0 inside the
    Code2Wav step: normalize, write WAV, read back, resample, S3 tokenizer,
    fbank, CAM++, mel, then host-to-device copies. Every stream in that step
    waits for it.
  • The idle prefetch that exists for this never runs under load.
    run_idle_prefetch starts nothing while any stream is live, or while more
    than token2wav_ref_prefetch_max_waiters (default 1) references wait.
  • First chunks run as a separate decode batch. cfm_onset_merge is off,
    so a step with new and continuing streams runs two solves: 3208 decode
    batches over 2161 steps in our C=1-32 run.

After

  1. Asynchronous reference preparation (token2wav_async_ref_prepare,
    CUDA only, default off; on in the high-concurrency profile). When the async-chunk
    placeholder arrives (on_requests_added), a worker thread runs the host
    half of the prompt. The forward thread later queues the device half on a
    side stream, and chunk 0 waits on a CUDA event. In practice chunk 0 no
    longer prepares its voice inside the step: 0 inline and 1702 prepared
    ahead in the validation run, against 1591 inline without the preparer.
  2. Host/device split of the prompt. StepAudio2Token2WavCore._prepare_prompt
    becomes _prepare_prompt_device(_prepare_prompt_host(path)), so the worker
    can run the host half alone. Values are bit-identical to main's unsplit
    function, including [perf][npu] Remove host sync stalls in the prompt feature setup #9186's pinned copies and sync fixes.
  3. A high-concurrency profile. New minicpmo_4_5_high_concurrency.yaml
    inherits minicpmo_4_5.yaml (including its default cuda_mps) and sets
    the preparer, micro_batch_size: 8 (default 4) and cfm_onset_merge: true,
    so each step runs exactly one decode batch (2194 batches over 2194 steps in
    the validation run).

Not in this PR: batching the setup solves of several new voices. That's open
PR #9221. A local version of it was built, measured and removed in favor of
#9221; see "Relationship to other open PRs".

Unchanged:

  • the default profile minicpmo_4_5.yaml;
  • duplex sessions (they send no prewarm payload, so the preparer never runs);
  • the MRv2 generation runner (it doesn't call the hooks);
  • NPU (the preparer is CUDA-only, so the idle prefetch stays);
  • prompt feature values (bit-identical on the real model).

Affected contract: docs/design/minicpm_o45_mrv2_performance.md gains a
section, "Asynchronous reference preparation". It sits next to "Reference
prefetch at higher concurrency", which this option replaces while active.
User guide: recipes/OpenBMB/MiniCPM-o-4_5.md (a paragraph on the new
profile).


Design

Asynchronous reference preparation

sequenceDiagram
    participant R as Generation runner
    participant M as Code2Wav (forward thread)
    participant W as Worker thread (new)
    participant S as Side CUDA stream (new)
    participant G as Main CUDA stream
    R->>M: on_requests_added(prewarm ref_audio, ref_audio_sr)
    M->>W: submit _prepare_reference_job (bounded at 32 jobs)
    W->>W: normalize + sha256, write PCM_16 WAV, host half of the prompt
    Note over M,G: later forward or idle step
    M->>M: _pump_reference_jobs(): pin the prompt to the request
    M->>S: install_prompt(): pinned copies, S3 quantize, CAM++
    S-->>M: event recorded after the device half
    R->>M: forward with chunk 0
    M->>G: wait_event(event), then prepare_prompt() hits the cache
Loading

Worker thread (_prepare_reference_job, ThreadPoolExecutor, two
workers by default via token2wav_async_ref_workers). It makes no CUDA calls
and doesn't touch model state:

  • _runtime_prompt_identity normalizes the reference onto the 24 kHz, 6 s
    grid and names it by sha256, exactly as the inline path does.
  • _write_runtime_prompt writes the PCM_16 WAV atomically (temp file plus
    os.replace), as before.
  • BatchedToken2Wav.prepare_prompt_host runs the host half: load, resample to
    16/24 kHz, S3 log-mel, fbank, mel. It returns PromptHostFeatures.

Keeping CUDA off the worker is what makes threading safe:

  • the per-step TF32 toggle (_stage_matmul_policy) still applies, because
    every CUDA call runs on the forward thread under it;
  • no CUDA graph capture can be disturbed;
  • the default-dtype switch stays on one thread. The preparer is enabled only
    while the process default dtype is fp32, so the worker never sees another
    value.

Forward thread, which calls _pump_reference_jobs at the start of
forward and from run_idle_prefetch (under the stage matmul policy):

  • takes finished jobs;
  • pins each prompt to its request, reusing _prefetch_pins, until chunk 0
    commits it or the request finishes;
  • calls BatchedToken2Wav.install_prompt on a side stream: the device half
    with pinned non-blocking copies, then record_stream(main stream) on the
    published tensors so the allocator can't hand their memory back early;
  • records one CUDA event per pump.

With the CPU-reference TensorRT speaker that minicpmo_4_5.yaml now enables
(#9188), CAM++ enqueues on the current stream, which is the side stream here,
and then synchronizes that stream. The forward thread therefore waits for the
device half queued so far, but not for the main stream. The bit-identity check
covers this backend.

Chunk 0. _prepare_decode_batch (and the initial-segment-marker path)
calls _wait_prepared_prompt before prepare_prompt. The main stream waits
on the event (a device-side wait, no host sync), and prepare_prompt hits the
feature cache.

Fallbacks. Each falls back to today's inline path:

Case Behavior
Job not finished at chunk 0 Chunk 0 prepares inline. The late result is dropped (async_late) and leaves no pin.
Job failed Logged; chunk 0 prepares inline (async_failed). A WAV it already wrote joins the runtime-prompt cache as the next eviction candidate.
Request finishes first The job is cancelled and its pin released. A job already running or done can't be cancelled: it is kept until it settles, then its WAV joins the runtime-prompt cache as the next eviction candidate, unless another request already tracks that prompt (async_abandoned).
Queue full (32 jobs) No job (async_queue_full); chunk 0 prepares inline.
Not CUDA, non-fp32 default dtype, or flag off The idle prefetch runs as before (async_unavailable is counted when the flag is on).

Counters. prewarm_accepted/rejected, async_*,
chunk0_prepared_async/cached/inline, compute_steps and decode_batches. They're logged at info level at most every 30 s, and only
when they change.

Host/device split of _prepare_prompt

StepAudio2Token2WavCore._prepare_prompt(path) is now
_prepare_prompt_device(_prepare_prompt_host(path)):

  • _prepare_prompt_host touches no device and no lazily loaded model: load,
    resample to 16/24 kHz, S3 log-mel, fbank, mel.
  • _prepare_prompt_device keeps main's device code from [perf][npu] Remove host sync stalls in the prompt feature setup #9186 unchanged:
    pinned _h2d_async copies, the short-audio S3 quantize without the
    .any() sync (an NPU graph on NPU), CAM++ and the NPU CAM++ stream join.
  • Step-Audio2 and the inline MiniCPM-o path call _prepare_prompt as before.

MiniCPMO45Token2wav delegates both halves; BatchedToken2Wav.install_prompt
calls the device half on the side stream.

Profile batching options

micro_batch_size: 8 lets up to 8 rows share one Whole-Euler graph replay
(graphs exist for 1, 2, 4 and 8 rows). cfm_onset_merge: true lets first
chunks join continuing rows in one ragged solve. Ragged cross-reference
batching was already active (CUDA ragged Whole-Euler), which the counters
confirm: one decode batch per step. micro_batch_size: 16 gave no further
gain.

What remains on the chunk-0 path

The CFM setup solve (setup_batch -> _create_initial_states) still runs in
the chunk-0 step: about 25 ms per new request at C=32, or 31% of Code2Wav
busy time. It can't move to a side stream, because it replays the shared
chunk-encoder and Whole-Euler graph arenas. Batching it across new voices is
#9221's scope.


Relationship to other open PRs (scanned 2026-10-22, rechecked 2026-10-26)

This PR depends on no open PR, and no open PR depends on it. It builds
only on merged code. Based on a scan of the open PRs on 2026-10-22 and on
test-applying the overlapping PRs' diffs to this branch and to main then;
the states of the PRs named below were rechecked on 2026-10-26.

1. Already merged; this PR builds on it.

2. Same work, so it was removed from this PR: #9221 (natureofnature, draft, approved).

3. Same files, independent changes. No functional overlap. Test-applying
their diffs to this PR's tree: #9207, #9231 and #9164 apply cleanly. #9213's
minicpmo_4_5_code2wav.py part applies cleanly; its batched_token2wav.py
part fails on main too (base drift, not this PR).

4. Next bottleneck, not in this PR. The Talker becomes the limit at about
2000 fps:

This PR doesn't touch the Talker.

5. Textual only. About 15 PRs add tests to tests/config/test_config_factory.py.
#8428, #8286 and #7294 add rows to test_duplex_deploy_config.py, and #6520
edits the MiniCPM-o recipe. Different functions and rows; trivial to resolve.

Test Plan

vLLM Version: 0.32.0 (torch 2.13.0+cu129, TensorRT 11.4.0.106, driver
570.133.20).

vLLM-Omni Commit: this branch on main at c0d14249a. Unit tests, the GPU
bit-identity check, chat throughput/VRAM, Seed-TTS WER and duplex latency ran
on the code of 87115b1dd, against minicpmo_4_5.yaml. The per-option WER and
duplex ablations ran on main at 6a56252e6 with the same Code2Wav changes,
against the default profile plus cuda_mps. Component attributions were
measured on 524f04c56 plus the same changes. Model revision d59117d19.

Hardware: one NVIDIA L20X (144 GB), shared host. All three stages on one
GPU, with MPS on in both arms.

Unit tests:

pytest -q tests/config/test_config_factory.py tests/engine/duplex \
  tests/model_executor/models/minicpmo_4_5 tests/model_executor/models/step_audio2 \
  tests/model_executor/models/test_minicpmo_handoff.py \
  tests/model_executor/stage_input_processors tests/worker/test_generation_runner_prewarm_hooks.py

New tests in tests/model_executor/models/minicpmo_4_5/test_code2wav_async_ref_prepare.py:

  • the host half runs off-thread, and chunk 0 is bit-exact with the inline path;
  • the idle step also queues the device half;
  • a late result is dropped without leaking a pin;
  • a failed job falls back to inline;
  • finishing before chunk 0 cancels the job and releases its pin;
  • finishing after the job started (running, done but not yet pumped, or
    failed) keeps the job until it settles, then its WAV is evicted and no pin
    is left; a reference shared with a live request keeps its entry and file;
  • a shared reference installs once and pins per request;
  • known requests and invalid payloads are not submitted;
  • the flag off, non-CUDA, and non-fp32 default dtype keep the idle prefetch;
  • install_prompt equals prepare_prompt.

test_minicpmo_high_concurrency_profile_adds_code2wav_options checks that the
profile adds exactly the three connector options and inherits cuda_mps and
everything else from the default profile.

GPU bit-identity check (real model, 3 references including 16 kHz and
22.05 kHz with noise, for both CAM++ backends: Torch and the CPU-reference
TensorRT engine): main's unsplit _prepare_prompt from c0d14249a vs
device(host(...)), and vs the host half on a worker thread with the device
half on a side stream.

Serving benchmark, with the dataset and request options of
tests/dfx/perf/tests/test_minicpmo_4_5.json (Seed-TTS en voice cloning) but
our own request counts:

python -m vllm_omni.entrypoints.cli.main serve openbmb/MiniCPM-o-4_5 --omni \
  --deploy-config vllm_omni/deploy/minicpmo_4_5_high_concurrency.yaml --trust-remote-code   # vs minicpmo_4_5.yaml

vllm bench serve --omni --endpoint /v1/chat/completions --backend openai-chat-omni \
  --model openbmb/MiniCPM-o-4_5 --dataset-name seed-tts --dataset-path zhaochenyang20/seed-tts-eval \
  --seed-tts-locale en --no-oversample --disable-shuffle --trust-remote-code \
  --num-prompts $(( C < 2 ? 32 : 16*C )) --max-concurrency $C \
  --extra-body '{"modalities":["text","audio"],"chat_template_kwargs":{"enable_thinking":false,"use_tts_template":true}}' \
  --percentile-metrics ttft,e2el,audio_ttfp,audio_rtf,audio_duration --metric-percentiles 50,95
# One discarded warmup (C=8, 32 prompts), then C=1/8/16/32 three times on the same server.
# GPU memory: nvidia-smi --query-gpu=memory.used -l 1 on the reserved GPU.
# Quality: the same with --seed-tts-wer-eval --seed-tts-wer-save-items at C=8, --num-prompts 1088 (full en set).
# Duplex: tests/dfx/perf/tests/test_minicpmo_4_5_duplex_seed_tts.json (/v1/realtime, 4 turns, C=1/4/8),
# then C=8 twice more on the same server.

Test Result

Unit tests at 4222d053b, the command above: 4112 passed, 25 skipped, 269 deselected.
GPU bit-identity: all identical for both CAM++ backends. All pre-commit
hooks pass on the changed files (ruff, ruff format, typos, markdownlint,
mypy-3.10, SPDX, test marks, forbidden imports, the torch.cuda check).

Chat throughput, codec frames/s, on the rebased tree. Baseline is
minicpmo_4_5.yaml (MPS, the TensorRT speaker and Thinker prefix caching on).
Three runs per point on one server per config, 0 failed requests:

C Baseline runs Mean This PR's profile runs Mean Change
1 200 / 202 / 202 201 201 / 190 / 200 197 -2% (within spread)
8 787 / 774 / 760 774 844 / 872 / 859 858 +11%
16 830 / 940 / 926 899 964 / 1120 / 1122 1069 +19%
32 1086 / 1081 / 1100 1089 1308 / 1338 / 1333 1326 +22%

Mean first audio at C=32 went 1281 → 1056 ms, and at C=16 820 → 733 ms. With
the profile, every new request's reference was ready before chunk 0
(chunk0_prepared_async 1702, no inline preparation); the baseline prepared
1591 inline.

GPU memory (whole GPU, exclusive reservation): 87,665 → 90,377 MiB after
startup (+2.6 GiB, the Whole-Euler arena and graphs for 8 rows), and
107,177 → 108,398 MiB at peak over the C=1-32 sweeps (+1.2 GiB).

Known limitations

  • The per-option WER and duplex ablations were measured one main revision
    earlier, before [Model] Add CPU-reference TensorRT speaker extraction for MiniCPM-o #9188 switched the default CAM++ backend to TensorRT. The
    shipped profile was re-measured against the new default on the rebased tree.
  • Numbers come from one shared host, with the chat, WER and duplex runs on
    three GPUs at the same time; three runs per point, ranges shown.
  • Duplex C=4 was measured once per config (a warmup stage); the one run was 5%
    slower with the profile.
  • SIM and UTMOS are not reported by this build's WER evaluation.

🤖 Generated with Claude Code

@natureofnature

Copy link
Copy Markdown
Collaborator

@natureofnature

@natureofnature natureofnature left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the changed code, reference lifecycle, host/device split, profile and tests at 19a08b7. The reported real-model bit-identity check and full-set WER are useful evidence; the last commit only changes documentation.

I suggest exercising the asynchronous lifecycle on real CUDA before merging: at this head or the fix commit, delay side-stream completion while mixing shared/distinct references, inline fallback, cancellation and cache eviction, then repeat the cycle. Surviving requests should retain the correct reference features/audio, complete without CUDA errors, and release cancelled work without continuing growth in temporary files or retained memory after caches warm. The new CPU-fake tests do not exercise that stream/lifetime interaction (installation path); the reported parity check covers a different condition. I have not run repository code.

【Conclusion: Found blocking code issues and validation gaps in commit 19a08b7.】
【Action recommended: Clean up references cancelled before installation, verify cancellation leaves no orphan files, and exercise CUDA cancellation/eviction with stable outputs and bounded resource use before merging.】

Comment on lines +1397 to +1399
job = self._ref_jobs.pop(state_id, None)
if job is not None:
job.cancel()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Keep cleanup ownership until a cancelled reference job settles. If a request is cancelled after its worker starts (or finishes) but before the pump installs it, Future.cancel() returns false and this pop loses the only tracked job. The worker writes the WAV first, whereas the runtime-cache entry is created only by the pump, so the file never reaches normal eviction. Repeated cancelled voice-cloning requests with distinct references accumulate files until process exit and can exhaust temporary storage. Retain/drain cancelled jobs and reclaim their unowned files safely, without deleting a shared reference still in use. A regression should cancel both a running job and a completed-but-unpumped job, wait for completion, and check that unowned WAVs and pins are gone; the current cancellation test pumps before cancelling.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4222d05. on_requests_finished now keeps a job whose cancel() fails in _abandoned_ref_jobs. Once it settles, the pump registers its WAV as the cold end of the runtime-prompt cache, so normal eviction deletes it. That registration runs after the pump's own pins, so a reference shared with a live request keeps its entry and file. A job that fails after writing its WAV (_ReferenceJobError) is handled the same way. Counted as async_abandoned.

Regressions in test_code2wav_async_ref_prepare.py:

  • test_finishing_after_the_job_started_reclaims_its_wav[running|done|failed] cancels while the worker is blocked after writing the WAV, after it finished but before any pump, and after it failed. It then waits, pumps, and asserts the WAV is deleted, the cache is empty and no pin is left.
  • test_reclaiming_an_abandoned_job_keeps_a_shared_reference checks the shared case.

The new tests fail on the previous head.

chickeyton and others added 4 commits October 26, 2026 09:39
…0 and batch more rows per solve

Code2Wav limits MiniCPM-o 4.5 throughput from concurrency 8 up on one GPU,
and a new request's reference voice was prepared inside its chunk-0 step,
which every live stream waits on.

- token2wav_async_ref_prepare (CUDA only, default off): on the prewarm hook a
  worker thread runs the host half of prompt preparation (no device calls);
  the forward thread queues the device half on a side stream, and chunk 0
  waits on an event. Late or failed jobs fall back to inline preparation.
- Split StepAudio2Token2WavCore._prepare_prompt into host and device halves;
  the inline path stages its copies through pinned memory. Values are
  unchanged.
- minicpmo_4_5_mps.yaml also sets token2wav_async_ref_prepare,
  micro_batch_size: 8 and cfm_onset_merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: chickeyton <ngton2014@gmail.com>
The MPS profile's Code2Wav options were checked on the full Seed-TTS en set
(1088 utterances, C=8): 2.18% WER against 2.24% with only cuda_mps, with
utterance-level changes balanced (19 worse, 19 better). Update the profile
header and the recipe, which described the question as open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: chickeyton <ngton2014@gmail.com>
… 4.5 MPS profile

Three C=8 duplex Seed-TTS runs per config on one warmed server show the
profile's first-audio p95 is not higher than the baseline's (mean of p95
513 vs 580 ms; the baseline alone spans 505-625 ms). The earlier two-run
gap was within that spread. Comment and doc text only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: chickeyton <ngton2014@gmail.com>
…r MPS became the default

minicpmo_4_5.yaml now enables cuda_mps (vllm-project#9188), so the profile no longer adds
MPS; it only tunes Code2Wav for many concurrent voices. Rename it to
minicpmo_4_5_high_concurrency.yaml, drop its redundant platforms.cuda block,
and record numbers measured on the rebased tree (c0d1424) against the new
default, three runs per point: C=32 1089 -> 1326 codec frames/s, C=16
899 -> 1069, C=8 774 -> 858, C=1 unchanged; full Seed-TTS en WER 2.73% vs
2.68%; duplex C=8 p95 556 vs 555 ms; about 2.6 GiB more GPU memory after
startup.

After vllm-project#9186, StepAudio2Token2WavCore._prepare_prompt already stages its
copies through pinned memory, so the device half keeps that behavior and
drops its non_blocking flag. BatchedToken2Wav.prepare_prompt calls
_prepare_prompt directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: chickeyton <ngton2014@gmail.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-26T02:54:33.682051Z
We couldn’t complete this review. Comment @codex review to try again.
87115b1 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR appears to belong to: docs/design/module/ar_runtime.md, docs/design/module/model_integration.md.

Module owners: @tzhouam @fake0fan @Gaohan123

Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 via module of the changed files

@chickeyton, 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 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the async ref-prepare path on tip 87115b1 (host/device split, side-stream install + event wait, pin lifecycle, late/fail fallback, and the new CPU tests).

The split looks sound for the happy path: the worker stays off-device, _pump_reference_jobs queues the device half on a side stream with record_stream on the consumer, and chunk 0 orders via _wait_prepared_prompt before prepare_prompt. Shared-reference install-once + per-request pins and the inline/late/fail fallbacks match the stated design. Profile rename / numbers are fine as docs.

One issue from the earlier review is still present on this tip (inline below): cancelling a job that has already started or finished drops the only tracker before the pump can install or reclaim the WAV the worker writes. The current cancellation test pumps before finishing, so it does not cover that orphan path. Worth fixing (or at least draining cancelled/unowned files) before relying on this under high churn with distinct references.

The CPU-fake suite still cannot exercise real CUDA stream/lifetime interaction; a short mixed cancel/eviction run on GPU would help confirm bounded temp-file and memory use after warm-up.

Comment on lines +1482 to +1484
job = self._ref_jobs.pop(state_id, None)
if job is not None:
job.cancel()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the request finishes after the worker has started (or already finished) but before _pump_reference_jobs installs the result, Future.cancel() returns false and this pop drops the only tracked handle. _prepare_reference_job writes the WAV before returning, while _runtime_prompts / pins are only created in the pump — so the file never reaches normal eviction via _evict_runtime_prompt. Distinct cancelled voice-clone refs can accumulate under the runtime prompt dir until process exit.

Please retain/drain cancelled jobs (and reclaim unowned WAVs safely when no other pin/owner shares the path). A regression that cancels both a running job and a completed-but-unpumped job, waits for completion, then asserts unowned WAVs/pins are gone would cover this; the existing finish test pumps first.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4222d05. on_requests_finished now keeps a job whose cancel() fails in _abandoned_ref_jobs. Once it settles, the pump registers its WAV as the cold end of the runtime-prompt cache, so normal eviction deletes it. That registration runs after the pump's own pins, so a reference shared with a live request keeps its entry and file. A job that fails after writing its WAV (_ReferenceJobError) is handled the same way. Counted as async_abandoned.

Regressions in test_code2wav_async_ref_prepare.py:

  • test_finishing_after_the_job_started_reclaims_its_wav[running|done|failed] cancels while the worker is blocked after writing the WAV, after it finished but before any pump, and after it failed. It then waits, pumps, and asserts the WAV is deleted, the cache is empty and no pin is left.
  • test_reclaiming_an_abandoned_job_keeps_a_shared_reference checks the shared case.

The new tests fail on the previous head.

@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot: three questions on the performance claim

@chickeyton this PR reads as a performance or value claim:

  • claim: On one L20X this raises throughput by 22% at

Before the full evidence checklist, three short questions:

  1. Bottleneck — what is the current bottleneck, and which profile, trace or per-stage measurement shows it?
  2. Value — what does the change buy the user or the system (latency, throughput, memory, cost), and at which workload?
  3. A/B or ablation — is there a same-workload, same-head/config comparison that isolates each main claim on its own? For stacked optimizations, one number per item rather than a blended delta.

When you answer, the evidence that settles it is: base and head SHA, hardware, model, workload, warm-up and repeat count, mean or percentiles with their spread, and a correctness/quality-equivalence signal; an end-to-end claim also needs stage attribution.

…led after they started

When a request finished after its reference job's worker had started (or
finished) but before the forward thread pumped it, Future.cancel() returned
false and the job was dropped. The worker had already written the prompt
WAV, and only the pump registers it with the runtime-prompt cache, so the
file was never evicted. A job that failed after writing its WAV leaked the
same way.

Keep such jobs until they settle, then register their WAV as the next
eviction candidate of the runtime-prompt cache, after the pump's own pins so
a reference shared with a live request keeps its entry. Counted as
async_abandoned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: chickeyton <ngton2014@gmail.com>
@chickeyton

Copy link
Copy Markdown
Contributor Author

Self-review at 4222d05. Thanks @natureofnature and @amy-why-3459. The cancellation leak you both flagged is fixed; details are in the inline thread.

What I checked:

  • Rebase onto c0d14249a. [perf][npu] Remove host sync stalls in the prompt feature setup #9186's device-side code sits unchanged inside _prepare_prompt_device, and the non_blocking flag and its fallback are gone. A real-model GPU check compares main's unsplit _prepare_prompt with the split inline path and with the worker-thread plus side-stream path. All are bit-identical for both CAM++ backends (Torch, CPU-reference TensorRT).
  • Unit tests (the PR's command, CPU): 4112 passed, 25 skipped. Pre-commit passes.
  • CUDA lifecycle, as you suggested: a stress run on the profile server, with and without the fix.
    • Setup: 6 cycles of 48 voice-cloning requests at C=16. A third share one reference and the rest are new, so the 4-entry cache keeps evicting. A third are cancelled early, a third mid-stream, and a third complete.
    • With the fix: runtime-prompt WAVs stayed at 4 after every cycle. GPU memory and Code2Wav RSS stayed flat, there were 0 CUDA errors, and every completed request returned audio.
    • Without the fix: WAVs rose to 5 and stayed there.
    • End to end the race is rare (async_abandoned 3 and 0 in two runs), because the pump runs every step. The unit tests cover the orderings deterministically.
  • Not done: a run that artificially delays side-stream completion, which needs a test hook in the stream path. I haven't added one.
  • Numbers on the rebased tree (three runs per point, against minicpmo_4_5.yaml): +22% codec frames/s at C=32, +19% at C=16, +11% at C=8. Full Seed-TTS WER is 2.73% vs 2.68%, and duplex C=8 p95 is 556 vs 555 ms. The description has the ranges.

@amy-why-3459 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the abandoned-job reclaim path introduced in 4222d053b (present on current tip 371e929, which also merges main).

The earlier cancel/orphan concern looks addressed:

  • on_requests_finished keeps the Future in _abandoned_ref_jobs when cancel() returns false.
  • _prepare_reference_job wraps post-write failures in _ReferenceJobError so the WAV path survives.
  • _pump_reference_jobs drains settled abandoned jobs after its own pins, then _adopt_unowned_reference + _trim_runtime_prompts evict unowned files; a shared key already pinned by another request is left alone.
  • Late and failed live-job paths also adopt the written WAV instead of dropping the only handle.

The new CPU tests cover the important cases: finish while running / already done / failed-after-write, plus the shared-reference keep path. That matches what was missing before.

One residual (non-blocking) note: reclaim still only runs when the forward/idle pump runs, so a settled abandoned job can hold its WAV until the next step. Under normal serving that is fine; just worth knowing if someone expects finish alone to delete the file immediately. The suite is still CPU-fake, so a short GPU cancel/eviction churn check would still be useful before leaning on this under high distinct-voice churn, but the lifecycle leak itself looks closed.

@amy-why-3459

Copy link
Copy Markdown
Collaborator

Could we apply the settings responsible for these performance gains directly to the default minicpmo_4_5.yaml, instead of adding a separate minicpmo_4_5_high_concurrency.yaml? This would let users benefit from the improvements by default. Are there trade-offs that require keeping these settings opt-in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants