[Core] Default prefix_cache_retention_interval to dense for Mamba + EAGLE - #55760
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
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. |
…AGLE PR vllm-project#52216 changed the default of prefix_cache_retention_interval from None to 0. With sparse retention (0), only the latest replay boundary keeps a Mamba state checkpoint, and EAGLE additionally drops the tail block from prefix-cache hits, leaving Mamba checkpoints largely unreachable so prefix caching never hits for Mamba models with EAGLE-style speculative decoding. Keep the public argument unchanged (int | None, default 0) and only change how an unset value is resolved: EngineArgs now defaults the field to a private sentinel (or the deprecated VLLM_PREFIX_CACHE_RETENTION_INTERVAL env var when set) so that create_engine_config can tell an unset value apart from an explicit one and resolve it once the model and speculative-decoding configuration are known: dense (None) for Mamba models with EAGLE-style speculative decoding, 0 otherwise. Explicitly configured values (0 / N / None / env var) are always respected. Co-authored-by: Kimi Code Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
89fa938 to
06c976e
Compare
2 participants
Purpose
#52216 promoted
prefix_cache_retention_intervalto an argument and changed the default fromNone(dense) to0(sparse: keep only the latest replay boundary). This default is unfriendly to Mamba + EAGLE: with sparse retention, only the latest replay boundary keeps a Mamba state checkpoint, and EAGLE additionally drops the tail block from prefix-cache hits, leaving Mamba checkpoints largely unreachable — prefix caching never hits for the Mamba cache group (see #53504 for a production report of 0 hit tokens).This PR keeps the public argument unchanged (
int | None, default0) and only changes how an unset value is resolved:EngineArgs.prefix_cache_retention_intervalnow defaults to a private sentinel (or the deprecatedVLLM_PREFIX_CACHE_RETENTION_INTERVALenv var when set), socreate_engine_configcan tell an unset value apart from an explicit one and resolve it once the model and speculative-decoding configurations are known:has_inner_state) + EAGLE-style speculative decoding (use_eagle()) ->None(dense checkpoints, the pre-Promoteprefix_cache_retention_intervalto an argument and change the default to 0 #52216 behavior)0(unchanged from Promoteprefix_cache_retention_intervalto an argument and change the default to 0 #52216)Explicitly configured values are always respected:
--prefix-cache-retention-interval 0/N/Noneand the deprecated env var all override the resolution.Only
vllm/engine/arg_utils.pyis touched on the product side;CacheConfig,VllmConfig, and the CLI surface are unchanged.Not a duplicate: open PRs in this area (#54713, #53479, #55403) change retention/blocking mechanics for Mamba under EAGLE; none restore the pre-#52216 dense default for this combination.
Test Plan
Test Result
132 passed + 27 passed. New parametrized test
test_prefix_cache_retention_interval_default_resolutioncovers: unset -> dense for Mamba+EAGLE, unset -> 0 for Mamba w/o EAGLE / EAGLE w/o Mamba / plain models, explicit0/None/64respected, CLI default remains the unset sentinel, and the deprecated env var path.pre-commit runpasses on all changed files (ruff, mypy, etc.).tests/v1/engine/test_engine_core_client.pyhas 11 failures on this machine, all from GPU OOM / HF 403s (real-model GPU tests), unrelated to this change.This change was made with AI assistance (Kimi Code); all changed lines were reviewed and the tests above were run by the submitter.