Skip to content

perf: avoid fixed-width MTP draft host synchronization - #97

Merged
lukealonso merged 1 commit into
dev/fathomless-firmamentfrom
codex/fathomless-firmament-fixed-width-mtp-hostsync-20260717
Jul 17, 2026
Merged

perf: avoid fixed-width MTP draft host synchronization#97
lukealonso merged 1 commit into
dev/fathomless-firmamentfrom
codex/fathomless-firmament-fixed-width-mtp-hostsync-20260717

Conversation

@voipmonitor

@voipmonitor voipmonitor commented Jul 17, 2026

Copy link
Copy Markdown

Summary

  • classify speculative methods by whether the scheduler needs their real draft token IDs on the host
  • keep fixed-width MTP draft IDs on the worker and give the scheduler fixed-width placeholders
  • avoid the per-target-step draft D2H copy and copy_event.synchronize() for standard MTP
  • preserve real host draft IDs for variable-width DFlash, DSpark, and CausalCascade methods
  • preserve the existing host-copy path for structured-output requests

Root cause

With async scheduling enabled, DraftTokensHandler copied draft token IDs to the host and EngineCore consumed them after every target decode step. This is required for variable-width block speculators because a -1 suffix communicates their actual draft length, but standard MTP always has the configured fixed draft width. Its scheduler does not need the token values; verification continues to use the real IDs retained on the worker.

The unnecessary CUDA-event synchronization reduced GLM-5.2 MTP:3 target-step rate while MTP:0 remained unchanged.

Performance validation

Configuration: GLM-5.2 NVFP4, A16, TP8, DCP1, MTP:3, F8 DMA disabled, CC1, temperature 0. Each result uses four runs of three complete 2,048-token requests. Baseline and patched images were swapped between GPU 0-7 and GPU 8-15, and both servers were fully loaded and settled before measurement.

Metric GPU 0-7 GPU 8-15 Balanced effect
Decode throughput 135.81 -> 142.56 tok/s (+4.97%) 133.02 -> 139.08 tok/s (+4.55%) +4.758%
Target steps/s 47.04 -> 49.16 (+4.52%) 46.33 -> 48.41 (+4.48%) +4.496%

Bootstrap 95% CI for the balanced throughput effect: +3.588% to +5.877%.

Acceptance and mean accepted length did not change significantly:

  • acceptance: +0.337%, 95% CI -1.290% to +1.929%
  • mean accepted length: +0.252%, 95% CI -0.816% to +1.286%

This isolates the gain to target-step execution rather than a favorable acceptance sample.

Checks

  • pytest --noconftest -q tests/v1/spec_decode/test_draft_tokens_handler.py (2 passed)
  • ruff check on all changed Python files
  • ruff format --check on all changed Python files
  • full GLM-5.2 TP8 MTP:3 crossover benchmark on 16 GPUs

Summary by CodeRabbit

  • Improvements
    • Improved speculative decoding support for methods that can return shorter draft-token prefixes.
    • Draft token IDs are now correctly trimmed at negative sentinel values when needed.
    • Fixed handling of fixed-width draft tokens by avoiding unnecessary host-side transfers and providing consistent placeholder values.
    • Updated scheduling and GPU execution paths to use the appropriate draft-token handling behavior.
  • Tests
    • Added coverage for host-copy behavior and negative draft-token suffix trimming.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 39cfe841-5f1d-4a29-b298-b5c9b641e433

📥 Commits

Reviewing files that changed from the base of the PR and between e4879e1 and 138c2f0.

📒 Files selected for processing (5)
  • tests/v1/spec_decode/test_draft_tokens_handler.py
  • vllm/config/speculative.py
  • vllm/v1/engine/core.py
  • vllm/v1/worker/gpu/model_runner.py
  • vllm/v1/worker/gpu/spec_decode/utils.py

📝 Walkthrough

Walkthrough

Host draft-token requirements are derived from speculative methods and propagated through engine scheduling and GPU draft-token handling. Fixed-width modes avoid host copies and return placeholders, while block speculators copy and trim negative-sentinel suffixes.

Changes

Host Draft Token ID Handling

Layer / File(s) Summary
Speculative configuration and engine control
vllm/config/speculative.py, vllm/v1/engine/core.py
Selected speculative methods report whether host draft token IDs are required, and both engine stepping paths use that flag for executor updates.
Draft token copy and normalization
vllm/v1/worker/gpu/spec_decode/utils.py
DraftTokensHandler conditionally skips host copies, emits fixed-width -1 placeholders, and trims copied token lists at negative sentinels.
GPU wiring and behavior tests
vllm/v1/worker/gpu/model_runner.py, tests/v1/spec_decode/test_draft_tokens_handler.py
GPUModelRunner passes the requirement flag to the handler, with tests covering placeholder output and negative-suffix trimming.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: lukealonso, yewentao256

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main performance change: avoiding host synchronization for fixed-width MTP draft tokens.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fathomless-firmament-fixed-width-mtp-hostsync-20260717

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@voipmonitor

Copy link
Copy Markdown
Author

Provenance clarification: an equivalent optimization existed in v17 as direct release-branch commit 83925ac7b6, but that commit had no PR and was never part of canonical dev/fathomless-firmament. A full current-FF source overlay therefore removed it and exposed the MTP regression. This PR is the clean, reviewed canonical replacement against current FF.

@lukealonso
lukealonso merged commit f3c49ff into dev/fathomless-firmament Jul 17, 2026
4 of 5 checks passed
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.

2 participants