Skip to content

Make prefill shared-read completion explicit and default-on - #37958

Open
weireweire wants to merge 4 commits into
sgl-project:mainfrom
weireweire:refactor/default-prefill-shared-read-done
Open

weireweire wants to merge 4 commits into
sgl-project:mainfrom
weireweire:refactor/default-prefill-shared-read-done

Conversation

@weireweire

@weireweire weireweire commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • enable compliant prefill shared-read completion by default
  • add SGLANG_FORCE_PREFILL_COARSE_WAR_BARRIER as a default-false prefill-only opt-out alongside the global SGLANG_FORCE_COARSE_WAR_BARRIER
  • centralize per-batch prefill eligibility in one backend resolver and pass SharedReadEnds explicitly to event publication
  • make DSV4 prepare sparse-read snapshots before returning PRE_REPLAY
  • remove the mutable DSV4 prefill_shared_reads_snapshotted handshake
  • preserve eager prefill publication, conservative capture-stable fallbacks, and HybridAttnBackend delegation

Performance validation

A direct parent/head A/B used the same InferenceX c960 configuration: GB300, disaggregated 4P+4D (prefill DEP8, decode DEP16), DeepSeek-V4-Pro, DSpark K=6, concurrency 960, and a one-hour AIPerf agentic trace. Both runs completed with zero request errors or cancellations.

Metric Parent (15d2cbcc) PR (98713d66) Delta
Request throughput 24.80 req/s 25.31 req/s +2.04%
Input token throughput 3.250M tok/s 3.332M tok/s +2.51%
Output token throughput 24,760 tok/s 25,296 tok/s +2.16%
Mean TTFT 14,673 ms 14,042 ms -4.30%
Median TTFT 9,342 ms 9,148 ms -2.08%
Mean ITL 13.175 ms 13.020 ms -1.17%
Mean request latency 27,324 ms 26,561 ms -2.79%

This validates no performance regression. The approximately 2% aggregate throughput difference is treated as normal run-to-run variation, not as a performance-improvement claim for this refactor. Fixed five-minute windows alternated between the two runs and showed no persistent late-run degradation. The current PR head differs from the benchmarked revision only by a comment.

Validation

  • compiled all changed Python source and test files
  • ran pre-commit run --all-files --show-diff-on-failure

CI States

Latest PR Test (Base): 🚫 Run #34811199006
Latest PR Test (Extra): ❌ Run #34811198808
Latest PR Test (AMD ROCm 10): ❌ Run #34811198969

@weireweire
weireweire force-pushed the refactor/default-prefill-shared-read-done branch 3 times, most recently from 12709ba to df12976 Compare September 7, 2026 09:02
@weireweire weireweire changed the title Enable prefill shared-read completion by default Make prefill shared-read completion explicit and default-on Sep 7, 2026
@weireweire
weireweire force-pushed the refactor/default-prefill-shared-read-done branch 2 times, most recently from a62dbd6 to 95ecc9b Compare September 8, 2026 03:08
Root cause: prefill WAR eligibility and DSV4 snapshot readiness were split between runner-side gates and a mutable metadata flag. Eager and CUDA graph runners had to prepare first and later rediscover whether PRE_REPLAY was safe, while wrapper delegation was incomplete.

Fix: centralize per-batch eligibility in one backend resolver, make preparation return SharedReadEnds explicitly, and pass that result through eager and prefill graph execution. Use default-false global and prefill-only coarse-barrier opt-outs, skip snapshot preparation under either opt-out, preserve capture-stable fallbacks, delegate through HybridAttnBackend, and remove the DSV4 readiness flag.

Validation: compiled all changed Python files and ran pre-commit run --all-files --show-diff-on-failure.
@weireweire
weireweire force-pushed the refactor/default-prefill-shared-read-done branch from 95ecc9b to fb4f006 Compare September 8, 2026 05:18
@weireweire

Copy link
Copy Markdown
Contributor Author

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci label Sep 9, 2026
weireweire and others added 3 commits September 9, 2026 15:57
Root cause: the common prefill shared-read resolver accessed self.model_runner even though attention backends do not uniformly retain ModelRunner. The prefill CUDA graph load_batch return contract also changed without updating one existing caller test.

Fix: pass the speculative algorithm explicitly from eager and prefill graph runners, update existing resolver tests, and unpack the load_batch boundary result in the existing CUDA graph test.

Validation: compiled all changed Python files and ran pre-commit across the full repository.
Root cause: the PR became unmergeable after context-parallel prefill handling changed on main, preventing normal pull-request CI from being created.

Fix: merge current main, retain its all-CP guard, and preserve the unified prefill shared-read boundary resolution. Update the existing replay mock to pass the now-required speculative algorithm.

Validation: Python compileall passed for the modified runtime paths. PR-relative diff checks and formatting/lint hooks passed; the all-files registered-test taxonomy hook reports three unrelated NPU violations already present on current main.
@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

1 similar comment
@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

@nvpohanh

Copy link
Copy Markdown
Collaborator

@weireweire could you fix thie conflicts?

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