Repository navigation
perf(vllm): bypass OpenAI logprob objects for raw captured responses - #4634
Conversation
|
/ok to test ff5c044 |
ff5c044 to
1cfe1e3
Compare
|
/ok to test 1cfe1e3 |
|
/ok to test 64de786 |
Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…them The L0 Vllm shards' base run globs unit/models/generation/test_vllm*.py, and the Models/Other shards ignore unit/models/generation/, so only the vllm-marked test in this file ran in CI. Renaming brings the scope-isolation and reset tests into the base run. Also add the NVIDIA copyright header the copyright check requires. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
64de786 to
5b6fa89
Compare
|
/ok to test 5b6fa89 |
terrykong
left a comment
There was a problem hiding this comment.
Thanks for this. The change is small, well-scoped, and safe. We found no correctness issues.
When the bypass is on, nothing reads the objects it skips:
- the raw arrays come from the engine output (utils.py#L470);
- Gym's capture adapter reads the message fields first (adapters/vllm.py#L34-L42);
_finish_request_capturestripschoice.logprobsanyway (vllm_worker_async.py#L880);- vLLM's
ChatCompletionResponseChoice.logprobsis Optional, so leaving it unset is allowed (protocol.py#L106).
The scope design is good. The separate module imports without vLLM, so the scope tests run in the CPU lane. Each part of the (instance, task, thread) check has its own test: we planted 9 mutants and every one was caught. And when the bypass doesn't apply, it falls back to vLLM's normal path.
One low-severity inline comment: add a test that fails if a vLLM bump stops calling _create_chat_logprobs from the full generator. Today that would turn off the speedup without any test failing.
No ask on perf numbers. They are from an earlier run, but the reason for the speedup is clear: it stops building per-token objects that get thrown away.
We did not run anything on GPU or with vLLM installed. The CPU tests passed locally (18 passed, 1 skipped because vLLM is missing).
Visual explainer: https://terrykong.github.io/gh-pages-poc/terryk/pr-4634-logprob-bypass.html
Generated by Claude Code
…_chat_logprobs The captured-logprobs bypass only takes effect because vLLM's chat_completion_full_generator calls self._create_chat_logprobs. Assert on that call in the pinned source so a vLLM bump that moves it fails the test instead of silently disabling the speedup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: root <root@pool0-01908.cm.cluster>
|
/ok to test 7a3b6e1 |
What does this PR do?
Captured chat responses currently build and serialize OpenAI logprob objects for every generated token, then discard those objects after staging the raw token IDs and selected-token logprobs to TQ. This PR skips that redundant object construction for admitted, non-streaming captures that use the raw-array response path.
Targets
maindirectly. Main already stages captured raw arrays and strips standard OpenAI logprobs after terminal staging. This optimization requires no prefix-recovery or checkpoint-stack changes.Changes
return_tokens_as_token_ids=true, and has an admitted capture state._create_chat_logprobsat the serving MRO boundary. Keep the original request and engine sampling parameters intact. Continue attaching and validating the engine-native raw arrays through the existing helper.Observed performance gains
Historical CPU A/B from
tq-chat-response-v2-768-20261007-085403, benchmark revisionreused-engine-inputs-bounded-drain-v2:That is about 80× lower response-construction p95, 94% lower terminal p95, and 7.9× more steady terminal completions in this workload. The full path spent 307.4 ms of CPU time on average in
_create_chat_logprobs; the bypass omits that work. Raw-array attachment remained about 2 ms CPU per response in both modes.Setup: one simulated generation worker, 768 active sequences, token lengths 2,048 / 8,192 / 32,768 / 65,536, four Simple TQ storage actors, 32 terminal writers, pending-terminal cap 64, cached physical-core counts, audit disabled, and checkpoint attempts every 30 s. The read-only synthetic engine inputs were reused, avoiding repeated fixture construction as a confounder. Measurements above use
steady_*distributions and exclude initialization and final drain.The three fence-drain measurements were 4.57 / 8.32 / 1.55 s with full logprobs versus 0.58 / 0.26 / 0.48 s with the bypass. Prefix cut time did not improve consistently: the benchmark's
cut_and_verify_secondswere 29.62 / 18.45 / 13.77 s versus 23.13 / 20.60 / 20.22 s. This change removes response-formatting contention; it does not remove all prefix-processing work.These measurements come from the earlier prefix-checkpointing experiment; its fence/cut phases are context for the gains, not functionality added by this PR. These are historical results from the benchmark's
fullversusskip-logprobsmodes, not a GPU measurement of this new commit or a 131k-rollout experiment. The benchmark exercised production cut/fence/terminal methods, Gym capture, and installed vLLM/RL response builders, with a simulated engine and shared serving loop. It excluded text/tool parsing, GPU generation, Gym HTTP, training, and checkpoint export. Aggregate terminal timings include queueing and other work; they are not isolated storage PUT latencies. Each mode was measured once.Internal artifact root:
/lustre/fsw/portfolios/nemotron/projects/nemotron_sw_post/users/amahishi/projects/pipeline/artifacts/tq-chat-response-v2-768-20261007-085403Results:
full/seqs-768-audit-off/result.jsonandskip-logprobs/seqs-768-audit-off/result.json.Usage
Automatic for eligible captured requests; no new flag is needed. Keep engine logprobs enabled. Ordinary API responses, streaming requests, unadmitted capture markers, and requests without the raw-array path retain their normal formatting behavior.
Testing
test_captured_chat_logprobs.py,test_vllm_chat_template_wiring.py,test_vllm_utils.py, andtest_vllm_token_capture_hosting.py.git diff --checkpassed.Local validation used a Python 3.12 CPU environment with the Gym revision pinned by main (
17abf2ddc76f74e40d0ba5a15b0401710a46db26). The four skipped tests require vLLM, which is not installed locally; one is a new engine-free integration test against the real vLLM formatter. GPU generation and end-to-end Gym HTTP validation remain for CI or a container smoke run.Before your PR is "Ready for review"