Conversation
…instead of model window
There was a problem hiding this comment.
Code Review
This pull request updates the PoolConfigurator to size the c4 state pool using the SWA storage page size (server_args.page_size) instead of the model's SWA window size (cfg.window_size), preventing over-allocation. Feedback suggests accessing mr.page_size directly instead of mr.server_args.page_size for consistency, and using the passed page_size parameter inside _compute_dsv4_sizes instead of self.swa_storage_page_size.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| c4_max_total_num_tokens=full_token // (4 * self.c4_shrink_factor), | ||
| c128_max_total_num_tokens=full_token // 128, | ||
| c4_state_pool_size=swa_tokens // self.swa_page_size * self.c4_ring_size, | ||
| c4_state_pool_size=swa_tokens // self.swa_storage_page_size * self.c4_ring_size, |
There was a problem hiding this comment.
In _compute_dsv4_sizes, the page_size is already passed as an argument to the function. To maintain consistency and ensure correctness if a different page size is used (e.g., during profiling or testing), you should use the passed page_size parameter instead of self.swa_storage_page_size for calculating c4_state_pool_size.
| c4_state_pool_size=swa_tokens // self.swa_storage_page_size * self.c4_ring_size, | |
| c4_state_pool_size=swa_tokens // page_size * self.c4_ring_size, |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…instead of model window
# Conflicts: # python/sglang/srt/model_executor/pool_configurator.py
…project#32035 sgl-project#33656 sgl-project#32183 sgl-project#33145) Applied PRs (latest from GitHub): sgl-project#33288 Indexer logits OOM fix sgl-project#30393 HiCache packed/sidecar draft caches sgl-project#31170 DPA prefix_affinity load balancing sgl-project#33795 DSpark compact ragged-verify CUDA graph JIT race sgl-project#32467 C128 plan-kernel warp barrier sgl-project#33865 DSpark x prefill CP unblock sgl-project#30371 SWA state pool sizing (storage page) sgl-project#33358 FlashMLA norm-rope K-tokens-per-block ILP sgl-project#33872 num_draft_tokens clamp + extend_len==0 skip (supersede sgl-project#32183) sgl-project#34002 Sidecar backup vacuously-successful fix (replaces sgl-project#33656, with tests) sgl-project#33862 Reclaim redundant host mirrors after storage backup sgl-project#31315 Avoid repeated Mooncake gets after stale hits sgl-project#32327 Q8KV8 sparse MLA prefill backend (flashmla_sparse_q8) sgl-project#31668 Fix sidecar pool life-time (use-after-free on prefetch abort) sgl-project#31195 TP0 verify-token-budget broadcast (adapted to get_schedule() API) Dropped (per user request or superseded): sgl-project#32771 IndexCache C4 top-k reuse — has bug sgl-project#32035 DSpark C128 online compressor — has bug sgl-project#33656 Superseded by sgl-project#34002 (same fix + unit tests) sgl-project#32183 Superseded by sgl-project#33872 (included in supersede PR) sgl-project#33145 Base f01f706 already has superior reasoning-effort profile system Conflicts resolved: sgl-project#31195: adapted to base get_schedule().disable_overlap_schedule API sgl-project#32327: path remapped jit_kernel/ -> kernels/jit/ and kernels/ops/attention/ sgl-project#31668: applied cleanly on top of sgl-project#30393+sgl-project#34002+sgl-project#33862 modifications
|
@mitu626 please fix the lint issue |
ispobock
left a comment
There was a problem hiding this comment.
can merge after lint fixed and ci passed
|
/rerun-failed-ci |
There was a problem hiding this comment.
This is a CI regression: https://github.com/sgl-project/sglang/actions/runs/32143584914/job/96140894881
@mitu626 could you take a look?
Motivation
For DeepSeek-V4 models, DSV4PoolConfigurator sizes the c4 compressed-state pool using the model's SWA window (cfg.window_size = 128), while the runtime addresses that same pool
using the pool's SWA storage page size (server_args.page_size = 256). These two values differ by 2x, causing capacity waste.
Modifications
Align the state-pool paging to the SWA storage page size (server_args.page_size), consistent with how DeepSeekV4TokenToKVPool and CompressStatePool address the pool at runtime:
self.swa_storage_page_size = mr.server_args.page_size # 256
c4_state_ratio = self.c4_ring_size / self.swa_storage_page_size
c4_state_pool_size = swa_tokens // self.swa_storage_page_size * self.c4_ring_size
A comment is added to disambiguate this storage page size from the model's SWA window (cfg.window_size / SWA_WINDOW = 128). This is not a correctness bug — the runtime already
divides by 256, so the smaller pool stays within bounds; it purely recovers wasted capacity.
Accuracy Tests
Speed Tests and Profiling
Validated on GB200 with DeepSeek-V4-Flash (TP4 / DP4, default config, mem-fraction-static unconstrained). Expected improvements from the memory-pool sizing logs:
Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ❌ Run #32143584914
Latest PR Test (Extra): ✅ Run #32143584743