Repository navigation
[Perf][AuxOutput] Do not rescan referenced keys in the routed-experts store - #62392
Merged
Merged
Conversation
aoshen02
force-pushed
the
aux-output-store-rescan
branch
5 times, most recently
from
October 19, 2026 06:53
4732639 to
655233e
Compare
aoshen02
force-pushed
the
aux-output-store-rescan
branch
from
October 19, 2026 07:21
d02a4f5 to
697c1bd
Compare
… store Move referenced keys past the eviction scan. They pile up at the front of the LRU, and every put() on a full store rescanned them (about 9 ms per put with 300k keys). Victims are unchanged. Split from vllm-project#62044. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: faresobeid <111092724+faresobeid@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ao Shen <aoshen@inferact.ai>
aoshen02
force-pushed
the
aux-output-store-rescan
branch
from
October 19, 2026 07:22
697c1bd to
3947149
Compare
Contributor
Author
|
/ci run |
|
✅ Triggered Buildkite CI #95851 for commit ℹ️ This PR is 1 commit behind upstream |
jeejeelee
approved these changes
Oct 19, 2026
bantianyue
pushed a commit
to bantianyue/vllm
that referenced
this pull request
Oct 28, 2026
… store (vllm-project#62392) Signed-off-by: faresobeid <111092724+faresobeid@users.noreply.github.com> Signed-off-by: Ao Shen <aoshen@inferact.ai> Co-authored-by: faresobeid <111092724+faresobeid@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: vllm-agent <claw@inferact.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Split from #62044 (by @faresobeid): only the store eviction fix.
BlockObjectStorekept referenced and unreferenced objects in one LRU, and eviction scanned it from the oldest key, skipping referenced ones. Keys referenced by running requests pile up at the front, so everyput()that had to evict rescanned all of them on the writer thread. #62044 measured about 9 ms perputwith 300k keys, and reports that removing this rescan cut the writer thread from 281 s to 2.3 s over 20 RL steps on GLM-4.5-Air, for +15.6% throughput.The store now follows
BlockPool's free block queue: the LRU holds only unreferenced objects._slotsmaps every stored key to its slot.put(), released keys end up after the batch's objects, in release order, as before.put()unless that call also releases them; they are at the tail by then.With balanced retains and releases, eviction order, victims and errors are the same as before. Releasing a key that is not referenced raises
KeyError, as before; keys released earlier in that call go to the tail of the LRU instead of keeping their old position, so later victims can differ. The writer thread stops after that error.Test Plan
test_store_eviction_does_not_rescan_referenced_keysfills a store with 900 referenced and 100 unreferenced objects, then makes 100put()calls. The referenced keys are astrsubclass that counts its own hashes, so any dict or set lookup of a referenced key is counted, whatever container the store uses. The test also checks that the 100 unreferenced objects are the ones evicted and that every referenced object stays readable. It does not touch store internals.Test Result
test_store.py: 65 passed.main(limit 900). It also fails onmainwith_referencesor_lrurenamed.main(not part of the PR), 5,000 random sequences ofput(duplicate keys, varying payloads, retains, releases) andgeton stores of 1 to 10 slots:KeyError.Benchmark
GLM-4.5-Air, one GB200 node (TP4),
gpu_memory_utilization=0.9(2,434,064 KV tokens, 167,341 store objects),max_model_len=20480, routed experts enabled. Each step is an RL-style rollout: 64 prompts x 8 samples (512 sequences), 1k-16k output tokens withignore_eos, seeded per step so every run generates exactly the same 13,448,609 tokens over 3 steps. Both arms use the same 2026-10-16 nightly image and differ only instore.py; runs are ordered main, PR, PR, main. The store is set to 1.1x the KV-sized default so thatmaindoes not hit the full-KV crash fixed by #62044. Writer-thread time is the sum ofBlockObjectStore.putwall time.puttimeThroughput is +17.7% (9,162 to 10,788 tok/s on average). On
main, the step time follows the writer-thread time: about 420 s at 140 s ofput, about 545 s at 240 s. Withgpu_memory_utilization=0.5(753,552 KV tokens, about 52k objects) the writer was busy for 17% of each step onmainand throughput was unchanged (5,868 vs 5,897 tok/s), so the gain needs a store large enough for the rescan to delay the engine.Reproduce
Run the script below once with
mainand once with this PR, on one node with 4 GPUs (max_bytesis 1.1x the default store size for this model and KV size):A single step with this script gave 459.3 s on
mainand 417.7 s with this PR for the same 4,543,356 tokens.🤖 Generated with Claude Code