Skip to content

[mem_cache] Split duplicate insert frees at the SWA eviction floor - #37481

Merged
hnyls2002 merged 3 commits into
mainfrom
lsyin/unified-dup-free-floor
Sep 2, 2026
Merged

hnyls2002 merged 3 commits into
mainfrom
lsyin/unified-dup-free-floor

Conversation

@hnyls2002

@hnyls2002 hnyls2002 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • _insert_walk_step splits the duplicate slice of an insert at the request's swa_evicted_seqlen: FreeDeviceKVFullOnly below it, FreeDeviceKV above it, in both the Python and the Rust tree core
  • Matches what SWARadixCache._insert_helper already does through free_kv_row_segments

Why

  • Below the floor the slice's SWA peers were already released by window eviction, so a both-side free there releases them a second time
  • Today free_swa reads mapping = 0 for those slots and the swa_indices > 0 filter drops them, at the cost of one host sync per release; once that filter becomes a contract ([mem_cache] Make free_swa sync-free on page_size == 1 #36723) the same free trips swa.peer_mapped
  • Both insert producers reach this one emission site: the regular cache_unfinished_req insert and the HiCache load-back insert (floor = span_end - len(swa_dev))
dup-free-floor-split

Tests

  • test_dup_slice_below_eviction_floor_frees_full_only asserts the emitted actions in the shared unified suite, so it runs under both tree-core backends; fails on the current code with no FreeDeviceKVFullOnly emitted
  • The eight components/swa.rs overlap tests whose duplicate slice lies entirely below the floor now expect FreeDeviceKVFullOnly
  • Full test_unified_radix_cache_unittest.py and cargo test --locked pass

CI States

Latest PR Test (Base): 🚫 Run #33573510228
Latest PR Test (Extra): ✅ Run #33573509920
Latest PR Test (AMD ROCm 7.2): 🚫 Run #33573510276

@hnyls2002

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

max(state.params.swa_evicted_seqlen - abs_start, 0), dup.numel()
)
if swa_already_freed > 0:
step_actions.append(FreeDeviceKVFullOnly([dup[:swa_already_freed]]))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you help to confirm if we have async assert inside token_to_kv_pool_allocator.free_full to confirm if there's no swa mapping from the full slots? I feel that could better guardrail the change to avoid leakage.

elif isinstance(action, FreeDeviceKVFullOnly):

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For this PR, the changes should be no-ops. The whole stack is to remove [swa_indices > 0], I added an async assert here, https://github.com/sgl-project/sglang/pull/36723/changes#diff-5e4f4819b718cd7f28b375e1d1729d525290e1f8fa79da7353ccad7c2ad3b56cR368

Which has real behavior changes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your clarification. Yeah, ideally, this change should be no-op.

However, I think that assertion you mentioned verify existence of SWA peer in free_swa, while FreeDeviceKVFullOnly should expect no SWA peer (which should be served as a long term invariant of FreeDeviceKVFullOnly usage. And if we use async assert, the overhead should be small enough). WDYT?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I will add an extra guard of expect(swa.peer_released, mapping[free_index] == 0) in #36723

@hnyls2002
hnyls2002 merged commit 832d029 into main Sep 2, 2026
202 of 249 checks passed
@hnyls2002
hnyls2002 deleted the lsyin/unified-dup-free-floor branch September 2, 2026 06:01
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.

2 participants