Skip to content

[HiCache] Fix memory host free logic when share_indices_with_anchor enabled#22767

Merged
hzh0425 merged 1 commit intosgl-project:mainfrom
antgroup:fix_hybrid_dsa
Apr 15, 2026
Merged

[HiCache] Fix memory host free logic when share_indices_with_anchor enabled#22767
hzh0425 merged 1 commit intosgl-project:mainfrom
antgroup:fix_hybrid_dsa

Conversation

@huangtingwei9988
Copy link
Copy Markdown
Collaborator

@huangtingwei9988 huangtingwei9988 commented Apr 14, 2026

Motivation

If share_indices_with_anchor is set to true, the entry host pool does not require explicit alloc or free. The redundant free operation occurring here causes the memory consumed by free_slots to continuously increase.

Modifications

Accuracy Tests

Speed Tests and Profiling

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

Copy link
Copy Markdown
Contributor

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

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 simplifies the free method in memory_pool_host.py by removing the logic that mirrored index freeing across multiple pool entries. Feedback indicates that this change makes the documentation for the share_indices_with_anchor flag stale, so the corresponding comments in the PoolEntry dataclass should be updated to maintain accuracy.

if getattr(entry, "share_indices_with_anchor", False):
entry.host_pool.free(indices)
return n
return self.anchor_entry.host_pool.free(indices)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The removal of the mirroring logic in free makes the documentation for share_indices_with_anchor in the PoolEntry dataclass (line 1666) stale and incorrect. It still claims that HostPoolGroup.free mirrors frees to the pool when this flag is enabled. Please update the comment in the PoolEntry definition to reflect that mirroring is no longer performed.

@huangtingwei9988
Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

Co-authored-by: hzh0425 <hzh0425@apache.org>
@huangtingwei9988
Copy link
Copy Markdown
Collaborator Author

/rerun-failed-ci

2 similar comments
@huangtingwei9988
Copy link
Copy Markdown
Collaborator Author

/rerun-failed-ci

@huangtingwei9988
Copy link
Copy Markdown
Collaborator Author

/rerun-failed-ci

@hzh0425 hzh0425 merged commit 3511c2d into sgl-project:main Apr 15, 2026
593 of 691 checks passed
yhyang201 pushed a commit to yhyang201/sglang that referenced this pull request Apr 22, 2026
whybeyoung pushed a commit to whybeyoung/sglang that referenced this pull request Apr 26, 2026
kyx1999 pushed a commit to KMSorSMS/sglang that referenced this pull request Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants