Skip to content

Revert DSv4 eager workspace reuse - #52836

Merged
WoosukKwon merged 1 commit into
mainfrom
woosuk/revert-pr-49236
Aug 19, 2026
Merged

Revert DSv4 eager workspace reuse#52836
WoosukKwon merged 1 commit into
mainfrom
woosuk/revert-pr-49236

Conversation

@WoosukKwon

@WoosukKwon WoosukKwon commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Revert #49236 and restore allocator-backed temporary buffers in the DeepSeek V4 attention input-preparation path.

This removes the model-wide DeepseekV4EagerScratchPool, its output-buffer plumbing, and the out-parameter fused op added by #49236. Later changes on main are preserved.

Why

The model-wide scratch pool can reuse storage across layers and CUDA streams without the caching allocator's stream/event lifetime tracking. That allows a producer to overwrite shared workspace while a previous consumer may still be using it. Reverting restores the pre-#49236 allocation lifetime behavior until safe cross-stream synchronization is implemented.

Impact

#51430 moved Q projection/KV insertion, indexer preparation, and compressor preparation out of the narrow eager break and into the captured region. The allocator calls restored by this revert are therefore captured on the Model Runner V2 path, so this revert is not expected to give up the 3.9% TTFT improvement originally reported by #49236 there.

#52401 subsequently restored the wide eager region for Model Runner V1 because the narrow region corrupted V1 output. The allocation overhead remains eager on that path, so its TTFT impact is runner-dependent and has not been measured on the current tree.

Duplicate-work check

This is not duplicating an existing revert. I checked issue #45861 and searched open PRs for references to #49236 and for DeepSeek V4 workspace/eager-scratch reuse. No open PR is dedicated to reverting #49236.

Validation

  • git diff --cached --check — passed before commit
  • git diff --cached --name-only --diff-filter=ACMR -z | xargs -0 .venv/bin/pre-commit run --files — passed
  • .venv/bin/python -m pytest tests/kernels/test_compressor_kv_cache.py tests/kernels/test_fused_deepseek_v4_qnorm_rope_kv_insert.py tests/kernels/test_fused_indexer_q_rope_quant.py -v — 240 passed, 11 skipped

Model evaluation

Not run: the DeepSeek V4 checkpoint is not available in the local Hugging Face cache. This PR remains a draft; DeepSeek-V4-Flash TP4 serving and GSM8K evaluation should be completed before it is marked ready.

AI assistance

OpenAI Codex was used to prepare the revert, resolve conflicts with later main changes, run validation, and draft this description. The human submitter must review every changed line and understand and defend the change end-to-end before marking the PR ready.

This reverts commit df71917 (#49236).

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
@mergify mergify Bot added deepseek Related to DeepSeek models nvidia labels Aug 18, 2026
@WoosukKwon WoosukKwon added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 18, 2026
@WoosukKwon
WoosukKwon marked this pull request as ready for review August 18, 2026 21:43

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@mergify

mergify Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Hi @WoosukKwon, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

@WoosukKwon

Copy link
Copy Markdown
Collaborator Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #84486 for commit 2cbd58585729.

@WoosukKwon
WoosukKwon merged commit f1178f3 into main Aug 19, 2026
254 of 262 checks passed
@WoosukKwon
WoosukKwon deleted the woosuk/revert-pr-49236 branch August 19, 2026 00:03
@github-project-automation github-project-automation Bot moved this to Done in NVIDIA Aug 19, 2026
wenhuach21 pushed a commit to wenhuach21/vllm that referenced this pull request Aug 19, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Wenhua Cheng <wenhua.cheng@intel.com>
randomvariable added a commit to randomvariable/vllm that referenced this pull request Aug 19, 2026
…-applied

Pair B crash-looped on model load with

  AttributeError: 'DeepseekV4Model' object has no attribute
  'eager_scratch_pool'

Upstream f1178f3 ("Revert DSv4 eager workspace reuse", vllm-project#52836) removed the
pool: its construction in DeepseekV4Model, the parameters that threaded it down
to the decoder layer, attention, indexer and compressor, the `*_out` kernel
variants that wrote into it, and eager_scratch.py itself. The rebase took the
half that deletes the construction and the C++ ops, and kept the half that
still reads `self.eager_scratch_pool` and passes it down -- so the very first
`make_layers` lambda dereferenced an attribute nothing assigns.

Nothing in the CPU suites constructs DeepseekV4Model, so the reference only
failed on a GPU worker, i.e. as a crash-looping serving pod.

Completing the revert rather than restoring the pool: the kernels it fed are
gone from csrc (`fused_deepseek_v4_qnorm_rope_kv_rope_quant_insert_out` and the
`output_buffers` paths in `compute_global_topk_indices_and_lens`), so
reinstating the Python side would only fail later, at the op lookup.

Removed the dangling `eager_scratch_pool` parameters and the branches guarded
on them, the orphaned `output_buffers` handling left in
`fused_indexer_q_rope_quant`, and eager_scratch.py, whose last consumer this
was.

Co-authored-by: OMP Agent <noreply@omp.local>
Signed-off-by: Naadir Jeewa <naadir@randomvariable.co.uk>
zyp2014 pushed a commit to zyp2014/vllm that referenced this pull request Aug 21, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Co-authored-by: OpenAI Codex <codex@openai.com>
wyettzeng pushed a commit to wyettzeng/vllm that referenced this pull request Aug 21, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Wyett <wyettzeng@gmail.com>
randomvariable added a commit to randomvariable/vllm that referenced this pull request Aug 23, 2026
…-applied

Pair B crash-looped on model load with

  AttributeError: 'DeepseekV4Model' object has no attribute
  'eager_scratch_pool'

Upstream f1178f3 ("Revert DSv4 eager workspace reuse", vllm-project#52836) removed the
pool: its construction in DeepseekV4Model, the parameters that threaded it down
to the decoder layer, attention, indexer and compressor, the `*_out` kernel
variants that wrote into it, and eager_scratch.py itself. The rebase took the
half that deletes the construction and the C++ ops, and kept the half that
still reads `self.eager_scratch_pool` and passes it down -- so the very first
`make_layers` lambda dereferenced an attribute nothing assigns.

Nothing in the CPU suites constructs DeepseekV4Model, so the reference only
failed on a GPU worker, i.e. as a crash-looping serving pod.

Completing the revert rather than restoring the pool: the kernels it fed are
gone from csrc (`fused_deepseek_v4_qnorm_rope_kv_rope_quant_insert_out` and the
`output_buffers` paths in `compute_global_topk_indices_and_lens`), so
reinstating the Python side would only fail later, at the op lookup.

Removed the dangling `eager_scratch_pool` parameters and the branches guarded
on them, the orphaned `output_buffers` handling left in
`fused_indexer_q_rope_quant`, and eager_scratch.py, whose last consumer this
was.

Co-authored-by: OMP Agent <noreply@omp.local>
Signed-off-by: Naadir Jeewa <naadir@randomvariable.co.uk>
zufangzhu pushed a commit to zufangzhu/vllm that referenced this pull request Aug 24, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Zhu, Zufang <zufang.zhu@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deepseek Related to DeepSeek models nvidia ready ONLY add when PR is ready to merge/full CI is needed

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant