fix(block_manager): prevent hash_to_block_id unbounded growth on deallocate

[wzgrx]

Summary

When a KV block is freed via _deallocate_block, its hash was never removed from hash_to_block_id. Over long-running deployments (hours/days of continuous TTS inference) this dict grows by one entry per unique KV-block prefix that ever existed, slowly degrading hash-lookup speed and Python GC performance.

Reported in: #61, #58

The Fix

Pop the stale block.hash entry from hash_to_block_id when the block is freed. This is a 1-line semantic change (expanded with a guard + comment for clarity).

Safety Analysis

The operation is safe because a freed block has:

• ref_count == 0 — no sequence references it
• token_ids == [] — cleared by Block.reset()

When allocate() later searches hash_to_block_id for a cache hit, the hit entrys block will have empty token_ids, so the token_ids != token_ids comparison (line 135) will force a cache miss — the same behaviour as before. The only difference is that we now proactively remove the stale dead entry instead of letting it accumulate.

In practice this means a freed blocks prefix can never trigger a future cache hit via its stale hash entry. But since reset() already cleared the content, such a hit would have been a false positive anyway (it would compare empty [] against real token_ids and still miss), so there is zero behavioural change in cache-hit logic.

Context

This is part of a family of long-running stability issues affecting VoxCPM2 + nano-vllm deployments on consumer GPUs. See also #58 (process unresponsive after days — block_id and list accumulation), #61 (progressive audio quality degradation under CUDA graphs + LoRA on Blackwell), and OpenBMB/VoxCPM#269 (cudagraph race on RTX 5090).

Open Questions for Reviewers

• Should we also consider a periodic eviction policy for hash_to_block_id (e.g. LRU capped at N entries) for deployments with very high request volumes?
• Could the same leak class exist elsewhere (e.g. lora_runtime state, scheduler callbacks)?

[wzgrx]

补充分析：CUDA graph 累积的根因（来自现场测试）

精确责任链

层	做了什么	责任
nano-vllm	prefill 不预录制 CUDA graph，fallback 到 self.model(**inputs) 走 torch.compile	🔴 决策问题
PyTorch Inductor	自动为每个新输入尺寸录制动态 CUDA graph	🟡 默认行为（可关）
CUDA Driver	被动分配显存	🟢 无责
Blackwell sm_120	graph 池行为不同更易碎片化	🟢 硬件特性

代码定位

engine/model_runner.py:879-885：

def run_model(self, inputs, is_prefill):
    if (
        is_prefill                     # ← PREFILL 走此分支
        or self.enforce_eager
        or inputs["positions"].size(0) > 512
        or (has_active_lora and not has_lora_graph)
    ):
        return self.model(**inputs)    # ← 触发 PyTorch 动态 CUDA graph 录制
    # decode: 走预录制 graph.replay() ✅

每个 TTS 请求文本长度不同 → 每次 prefill 输入不同 → 每个请求录制一个新 CUDA graph → 累积 1000+ 录制 → GPU 显存碎片 → 推理质量下降

可能的修复方向

1. nano-vllm 为 prefill 也预录制 — 输入 padding 到固定长度集（256/512/1024/2048），对每种长度预录制
2. 关闭动态录制 — torch._inductor.config.triton.cudagraph_skip_dynamic_graphs = True
3. 定期清理 — torch._inductor.cudagraph_trees.reset()（API 不稳定）

已有 #61 完整记录，这里是补充分析。

[a710128]

Thanks for looking into this. I agree that the issue described here is real: hash_to_block_id can retain entries for historical prefix hashes even after the corresponding block has been released, so in a long-running service with many unique prefixes this dictionary can grow beyond the actual number of KV blocks.

However, I think the current fix may change the prefix-cache semantics more than intended.

Today, a freed block is not reset in _deallocate_block(). Its hash and token_ids remain available, and allocate() explicitly handles the case where a hash hit points to a block that is no longer in used_block_ids:

if block_id in self.used_block_ids:
    block = self.blocks[block_id]
    block.ref_count += 1
else:
    block = self._allocate_block(block_id)

So a free block can still be reused as a prefix-cache hit, as long as its KV memory has not been overwritten. Removing the hash entry during deallocation disables that reuse path for sequential repeated prompts. That may be an acceptable policy change, but it should be called out explicitly and covered by tests.

There is also a smaller correctness concern: the pop should probably be conditional, otherwise deallocating one block could remove a mapping that now points to another block with the same hash value:

if block.hash != -1 and self.hash_to_block_id.get(block.hash) == block_id:
    self.hash_to_block_id.pop(block.hash, None)

A possibly safer approach would be to remove a stale hash when a free block is actually reallocated for a cache miss / overwritten, rather than when it is merely released. That would still bound stale entries over time while preserving the current “free blocks may still be prefix-cache candidates” behavior.

Suggested follow-ups before merging:

1. Add a test proving hash_to_block_id does not grow unbounded with overwritten blocks.
2. Add or update a test documenting whether sequential repeated prompts should still hit prefix cache after the previous sequence was deallocated.
3. Guard hash removal so it only deletes the mapping if it still points to the block being cleaned up.

So I agree with the problem statement, but I would not merge this exact change without clarifying the intended cache-retention semantics and adding tests around that behavior.