Simplify mem state - #623
Merged
Merged
Conversation
Ying1123
approved these changes
Jul 15, 2024
timethink
pushed a commit
to timethink/sglang
that referenced
this pull request
Mar 9, 2025
efschu
added a commit
to efschu/htsglang
that referenced
this pull request
Aug 7, 2026
…t-sum wiring at all DCP index-build sites
efschu
added a commit
to efschu/htsglang
that referenced
this pull request
Aug 8, 2026
… decidable, and refute the collective-stream fix The sgl-project#622/sgl-project#649 family is an ordering hazard, not a transport fault. Five production hangs on 2026-08-07 park all three ranks on the identical host line, GPUs at 100 %, never a divergence -- but on three DIFFERENT lines across the specimens (dcp/owner.py:566 twice, the draft kv_indptr .cpu() twice, a host-path abort once). What they share is the shape: a blocking host sync in the out-of-graph metadata-prep phase is ordered behind a barlink collective from the previous step. CUDA streams are in-order, so a collective that stalls for D blocks the host thread for D, and a host thread that is blocked cannot enqueue, cannot service the abort gate, and cannot be the rank that unwedges its peers. A bounded stall becomes an unbounded cluster hang. Enumerating the syncs does not close it, and that is now measured rather than argued. sgl-project#623 removed the .item() at owner.py:548 by threading total_tokens through all five call sites; the 15:55 specimen wedges at owner.py:566, five lines later in the same function, on boolean-mask indexing that has no host-derivable form. The callsite at flashinfer_backend.py:7237 does pass total_tokens, so :548 was correctly skipped and the wedge relocated. That is NOTE_622 section 3's prediction observed in production, before and after on one line pair, the same day. This commit adds the seam that makes the property decidable, and reports what it decides. barlink_stream_policy states the placement of forward work -- stream role plus the cross-stream ordering edges it implies -- as inspectable data instead of a scatter of torch.cuda.stream contexts across the attention backends, the graph runners and the transport. It holds no torch import and allocates nothing, so the decision is testable on a host with no CUDA device. The falsifier builds the ordering graph a production step would produce under a given policy and asks whether any collective is reachable backwards from a prep sync. It is a property test, not a call-site test: a test built around any one of the five specimens would pass while the class stayed open. THE RESULT, WHICH IS NEGATIVE FOR THE PROPOSED FIX Giving the collectives their own stream does NOT satisfy the property. A collective whose result the model consumes must be joined back onto the compute stream, and that join is itself a compute-stream node that the next step's sync waits for. Forking without also isolating the sync side moves the kernel and keeps the hazard. This is asserted explicitly (test_collective_stream_alone_violates_the_property) so the limitation cannot be forgotten and re-proposed. The placement that does satisfy it is ISOLATED_PREP: the prep phase on its own stream, ordered after the host-driven input copies and nothing else. Its soundness obligation -- that prep inputs are host-written -- is encoded rather than documented: a DEVICE_INPUT is joined into prep and breaks the property check, so adding a device-produced prep input fails a test instead of hanging production. Worth noting for whoever wires this up: the fork/join pattern at barlink.py:1028-1067 is the gloo host-staged fallback, which contains ev.synchronize() and a host dist.all_reduce and therefore cannot be captured. The in-graph BAR1 path (barlink.py:1007 -> barlink_all_reduce) has no stream context at all and lands on whatever is current. Today's placement is exactly LEGACY. TESTS test_collective_stream_sync_isolation_622.py: 12 passed, 202 subtests, on CUDA_VISIBLE_DEVICES=99. Can-fail proof by mutation, both executed: - PREP_SYNC moved back onto the compute stream in ISOLATED_PREP -> 107 failed, 11 passed. - the join modelled as free (no node on the joined stream) -> 8 failed, 9 passed, killing both the collective-stream refutation and the consumer-ordering control. Full test/registered/unit/distributed: 2645 passed, 12 skipped, 0 failed. Baseline before this commit was 2633 passed / 12 skipped / 0 failed; the 12 new tests are the entire delta. The 18 pre-existing failures attributed to sgl-project#627 in the briefing did not reproduce on this tree -- the named 615/580/603 files are 82/82 green. ruff and codespell clean. NOT VALIDATED ON A GPU. This branch was produced on a host with no CUDA device. ACTIVE is pinned to LEGACY and the module has zero production importers, so merging changes no runtime behaviour; flipping the default is a separate, GPU-evidenced change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.