Skip to content

[Core][HiSparse] Keep unpinned blocks last in the free queue - #59283

Draft
MatthewBonanni wants to merge 1 commit into
mainfrom
perf/hisparse-last-resort-copies
Draft

MatthewBonanni wants to merge 1 commit into
mainfrom
perf/hisparse-last-resort-copies

Conversation

@MatthewBonanni

@MatthewBonanni MatthewBonanni commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Purpose

Optional performance improvement, stacked on #59282. #59282 fixes the crash on its own; this PR only affects how many HiSparse GPU prefix copies survive memory pressure, and so how many prefix pages are read from host instead.

BlockPool.unpin_blocks documents unpinned blocks as "last-resort eviction candidates", but it appends them to the free queue like any other block, so blocks freed afterwards are queued behind them. In #59282's scenario, a finished request's cached indexer block is freed after its MLA copies were unpinned, so the resumed request's allocation evicts a copy it was about to adopt while that indexer block survives.

Changes

  • FreeKVCacheBlockQueue keeps a last-resort segment at its tail: append/append_n insert in front of it, the new append_last_resort_n appends behind it, and popleft/popleft_n/remove advance its head when they take it.
  • BlockPool.unpin_blocks uses append_last_resort_n, which makes its documented behavior true.

Only the HiSparse coordinator unpins blocks. Without unpinned blocks the segment is empty and the queue orders blocks exactly as before, at the cost of one pointer comparison per popped or removed block.

This does change HiSparse's eviction policy: unpinned blocks, meaning finished requests' copies and pages a running request released but still reads, now outlast every ordinary cached block rather than aging in LRU order. Evicting either kind forces reads from host.

Copies kept in #59282's scenario:

free_blocks #59282 this PR
7–8 0/3 0–1/3
9 1/3 2/3
10 2/3 3/3
11+ 3/3 3/3

Test Plan

  • test_hisparse_prefix_hit_under_pressure_adopts_surviving_copies[10] now expects all three copies; it keeps two without the last-resort segment.
  • test_free_kv_cache_block_queue_last_resort_stays_behind: later appends stay ahead of the segment after its first block is removed or popped.
  • tests/v1/core/test_prefix_caching.py and tests/v1/core/test_kv_cache_utils.py, with CpuPlatform selected.

Test Result

  • 296 passed. 8 errors are fixture 'caplog_vllm' not found, from running outside the top-level conftest, and are identical on main.
  • Not yet measured: an end-to-end pressure benchmark showing fewer host reads. That should decide whether this is worth a core change.

AI assistance (Claude Code) was used for the investigation, implementation, tests, and this description.

🤖 Generated with Claude Code

BlockPool.unpin_blocks documents its blocks as last-resort eviction
candidates, but blocks freed afterwards were appended behind them, so an
allocation could evict HiSparse's GPU prefix copies before blocks nobody
would reuse. FreeKVCacheBlockQueue now keeps a last-resort segment at its
tail that later appends stay in front of, and unpin_blocks uses it.

Only HiSparse unpins blocks; without them the queue behaves as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant