Skip to content

fix(swa): port tombstone-aware eviction + dec_swa_lock_only from upsream #23882#49

Open
yyj6666667 wants to merge 1 commit into
kvcache-ai:mainfrom
yyj6666667:fix/swa-tombstone-eviction
Open

fix(swa): port tombstone-aware eviction + dec_swa_lock_only from upsream #23882#49
yyj6666667 wants to merge 1 commit into
kvcache-ai:mainfrom
yyj6666667:fix/swa-tombstone-eviction

Conversation

@yyj6666667

Copy link
Copy Markdown

SWA-only eviction crashed on leaf nodes with full_lock_ref > 0, swa_lock_ref == 0 (beyond sliding window). Adds tombstone branch to free SWA without asserting, plus dec_swa_lock_only for early SWA lock release during decode. Unifies inc/dec_lock_ref return types across all cache implementations.

…ream sgl-project#23882

SWA-only eviction crashed on leaf nodes with full_lock_ref > 0, swa_lock_ref == 0
(beyond sliding window). Adds tombstone branch to free SWA without asserting, plus
dec_swa_lock_only for early SWA lock release during decode. Unifies inc/dec_lock_ref
return types across all cache implementations.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request optimizes Sliding Window Attention (SWA) cache management by introducing a configurable eviction interval and a mechanism for early release of leaf locks after the window has passed. It also refactors the cache locking API across multiple implementations to use structured result and parameter objects. Feedback identifies an inconsistency in the SWARadixCache._delete_leaf method, where len(node.key) is used instead of len(node.value) to update the evictable size, which could lead to accounting errors if the lengths differ.

Comment on lines +1232 to +1233
if not node.swa_tombstone:
self.swa_evictable_size_ -= len(node.key)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Inconsistency found: _delete_leaf uses len(node.key) to update swa_evictable_size_, while other methods like _insert_helper and evict use len(node.value). For consistency and to avoid potential issues if node.key and node.value differ in length, consider using len(node.value) if node.value is not None.

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.

1 participant