Move torch.compile configs into cuda_graph_runner.py by Ying1123 · Pull Request #993 · sgl-project/sglang

Ying1123 · 2024-08-08T19:48:09Z

We should set the environment variables in the tp worker process instead of setting them before launching the processes.

fix 0713

THE DEFECT, one line. Every draft page the TP phase backs up is restored correctly by `_page_transfer` -> `_draft_page_get` and then zeroed again one funnel later, because `arm_draft_cold_for_admission` scrubs a seam-re-admitted request's WHOLE cached prefix unconditionally -- the trigger asks WHERE the request came from, never WHETHER its draft rows arrived. Nothing rewrites a prefix's draft rows after admission, so such a request decodes over an all-zero draft chain for its entire life. That is the "no spec win in decode" the user named, and the 50504 draft pages in /tmp/hicache_783 are the cost already paid for it. DESK-PROVEN only (no cards in this session, no boot). Not item (d) of TASK_861. The draft key suffix does NOT move across a cutover: `config_suffix` is built once in `HiCacheFile.__init__` (hicache_storage.py: 605-637) from a `storage_config` that only `attach_storage_backend` (cache_controller.py:795) produces, and the cutover path (`hicache_phase_binding._stamp`, :267-300) re-points pool attributes and the binding generation and nothing else. Live store confirms it: every draft page carries `_0_1_3_{0,1,2}`, the BOOT geometry, and rank i therefore reads back exactly the pages rank i wrote. A canonical draft-page form buys nothing here; the blocker was the read path. TWO LINKS, both small, both default-inert without a flip: (1) `_draft_page_get_generic` writes ZEROS on a miss instead of leaving the row alone. The row is a recycled host slot holding the previous occupant's draft bytes, and leaving it made "restored" and "stale" indistinguishable downstream -- which is precisely why the scrub had to be unconditional. `get_dummy_flat_data_page()` is already `torch.zeros`, so the miss path writes the same value `scrub_draft_kv` writes, at the one site that knows which pages missed. Also removes the fp8 NaN/Inf path into the draft softmax that arbitrary recycled bytes carry. (2) The admission scrub is bounded to `not tier_armed`. With the tier armed the prefix came back THROUGH the draft read path, so its rows are real where the page existed and zero where it did not. The MARK is deliberately kept for both: splitting the scrub from the seed is sgl-project#631's own decomposition, and the seed is the cheap half (one non-drafting round whose FULL-captured hidden states start the real chain). Correctness is untouched in both directions -- the target verifies every proposed token, so this buys acceptance, not answers. INSTRUMENTS (speed mode: one mandatory line per chain link, and both are written so silence is a finding rather than success): * `sgl-project#993 draft L3 READ: N page(s) requested, H hit / M miss` -- fires on the first batch, then per doubling. A process that never reads a draft page emits nothing, and that absence is the upstream finding. * the ADMISSION line gains `%d prefix(es) KEPT (draft tier armed=%s)`. kept=0 with armed=True means the read path is not delivering and the fault is above this funnel -- a distinction the old line could not make. VERIFICATION (desk, hermetic, CUDA_VISIBLE_DEVICES=""): * import smoke on the worktree, patch confirmed applied by introspection. * test_draft_cold_admission_861 / test_draft_tier_gate_861 / test_draft_hicache_binding_861: 46 passed. * Two new pins, one per direction, and CAN-FAIL proven: reverting the `not tier_armed` bound fails the armed pin with `assert (5 == 0)` -- the five scrubbed rows are the defect itself. Cold rather than boot-observed on purpose: on metal the two behaviours differ only in an acceptance number no single boot can separate from model, traffic and flip cadence. * ruff check and ruff format: zero delta against HEAD on all three files (the 13 findings and the reformat are pre-existing, measured against `git show HEAD:` copies). SIBLING SWEEP (analysis). `arm_draft_bootstrap`, the sgl-project#631 cutover leg, scrubs too, but its input is carried DEVICE rows no read path restored, so its scrub is correct and it is de facto dead since sgl-project#856 retracts the residents anyway. The v2 draft route stays unreachable and unchanged. The class to watch: a restore path that leaves a recycled row untouched on a miss forces every downstream consumer to assume the worst about ALL rows. The component pools avoid it a different way -- `batch_exists_v2` takes the MIN and truncates the prefix rather than scrubbing behind itself.

…request Boot 12 (7b855f6) died on PP0 at 21:46:06 with `AssertionError: reusing request must be chunked or have committed KV` (memory_pool.py:395), via get_new_batch_prefill -> prepare_for_extend -> alloc_for_extend -> alloc_req_slots -> HybridReqToTokenPool.alloc. ROOT, from the ledger and not from inference. One second earlier the same rank logged, for the same rid: sgl-project#969 voided-request release failed for 5708abdd579e4f7097ab97ed796481ea: Committed KV cache already freed (self.kv_committed_len=4096) `_release_voided_request`'s never-raise contract caught that, logged it, and fell through to `reset_for_retract`. The contract is right -- an instrument that raises while cleaning up after a divergence turns one defect into two -- but it only decided that this frame does not PROPAGATE the failure. It never decided what the half-released request IS afterwards. The answer was: a request still holding `req_pool_idx`, whose `inflight_middle_chunks` and `kv_committed_len` the reset then zeroes -- and those two fields are the allocator's only evidence that a retained row is legitimate. It was re-queued, re-admitted one second later, and killed rank 0. The assert is correct and is the only thing that caught this. The defect is upstream of it. CLASS: warn-then-continue. The compensator converts a partial failure into a live, corrupt, re-admissible object. This is the catalogued `warn-then-continue swallow` shape, and the cure is not to remove the swallow but to make the post-failure STATE explicit. FIX, at the junction: on a failed release, hand the row back -- mamba slot first, then the req-pool row, the ordering `_release_dynamic_chunk_probe` already documents. The reset below drops prefix, pages and geometry anyway, so the request re-prefills regardless; the row it keeps buys nothing and justifies nothing. `free_slot` carries its own membership scan and REFUSES an already-free row (sgl-project#616), so a release that failed AFTER returning the row is caught by name and logged instead of corrupting the free list -- both outcomes are reported with the rid. NOT CLOSED, and deliberately not guessed at: why `pop_committed_kv_cache` found `kv_committed_freed` already True at `kv_committed_len=4096`. That producer is a second defect. This change contains it and names it in the log (`sgl-project#993 INCOMPLETE RELEASE DISOWNED`), so the next boot measures how often a release fails at all -- a number no boot of this family has ever had. FUTURE CHECK: any new raise inside `release_req` now ends in a disowned row plus a named line, not in an allocator assert three passes later. Evidence: desk. py_compile + import smoke + wiring assertion on `_release_voided_request`. Belegstufe: DESK-BEWIESEN.

Boot 13 (4fde6e5) did not crash: it WEDGED. 5 queued, 0 running, no first token, on all three ranks, behind 239 identical refusals of one shape: sgl-project#791 FORWARDED SCHEDULE UNEXECUTABLE for rid=901a7d29…: the decision names prefix_len=0, this rank holds 7938. ROOT, and it is the second half of the exemption sgl-project#992 found. `sgl-project#791 PP ADMISSION UNIFORMITY` has two halves -- MEMBERSHIP ("admit only what the decision names") and GEOMETRY ("with exactly the prefix it named") -- and both are implemented inside the waiting-queue loop, applied strictly before `adder.add_one_req`. The chunked continuation does not travel that loop: it reaches `can_run_list` through `add_chunked_req`, ~250 lines earlier, and was therefore exempt from BOTH. sgl-project#992 closed membership. Geometry stayed open, so a NAMED continuation entered the batch carrying its own prefix while the decision named another -- and that is a SHAPE disagreement, because `prepare_for_extend` sizes the cross-stage tensor directly off `len(req.prefix_indices)`. The follower refused, correctly, every pass. The decision said 0 because PP0 had spent its `sgl-project#946 PREMISE RECOMPUTE` terminator on that rid. That is PP0's call to make and the follower's to execute; second-guessing it is what the refusal exists to prevent. FIX: apply the same `truncate_prefix_to(told)` the queue loop applies, at the same point in the same order -- immediately before the re-derivation. sgl-project#930: the helper moves `prefix_indices` and `cache_protected_len` together. sgl-project#961: the mover must be followed by the re-derivation it invalidates; in the queue loop that is `add_one_req`, here it is `add_chunked_req` on the next line. Nothing is re-derived by hand and no new helper is introduced. Logged with rid, local prefix and told, counted, so the adoption is affirmative rather than inferred from the absence of refusals. SIBLING SWEEP: the two entries into `can_run_list` under a forwarded schedule are the queue loop and this one. Both now carry both halves of uniformity. PP0 has no incoming decision (`incoming is None`), so its path is byte-identical. BOOT 13 ALSO BOUGHT: sgl-project#993's containment fired and held -- no repeat of boot 12's allocator assert. sgl-project#992 held -- no repeat of boot 11's sgl-project#801-spin. Batch lines were symmetric 5/5/5 across ranks, i.e. no rank is structurally excluded. Evidence: desk. py_compile + import smoke + wiring assertion. Belegstufe: DESK-BEWIESEN.

…y what it owns Boot 18 (ecedf3f) is the specimen and it is the important one: plain PP=3 with the flip PROVABLY off (`PHASE-POLICY arming` 0, `PHASE-FLIP DONE` 0, chain-breaker check NONE, `command not found` 0), 31 batches, then PP0 died on AssertionError: Double-free detected: slots not currently allocated: [60934, 60935, ... 60971+] reached with no flip link in the chain at all: _event_loop_pp_body -> get_next_batch_to_run -> get_new_batch_prefill -> check_hicache_events -> drain_storage_control_queues -> _drain_release -> mem_pool_host.free. CLASS, and it is the FOURTH instance today: many producers, one give-back, no provenance. sgl-project#990 (lock_ref), sgl-project#991 (mamba slot), sgl-project#993 (req-pool row), now the host region. `cc.host_mem_release_queue` has FIFTEEN producers -- eight in unified_radix_cache.py, plus swa_component, mamba_component, hiradix_cache (x3), hi_mamba_radix_cache (x2) and cache_controller itself -- and `_drain_release` freed whatever arrived, with no dedup and no ownership test. TWO CHANGES, and the first matters more than the second. 1. PROVENANCE AT THE ENQUEUE. `append_host_mem_release` stamps the caller's `module:lineno` per page into `host_release_provenance`. Taken from the caller frame rather than added as a parameter to all fifteen sites, so a producer added LATER is stamped without anyone remembering to. This is what makes the next 90-second boot identify the producer PAIR itself instead of us picking the likeliest of fifteen. 2. OWNERSHIP AT THE GIVE-BACK. `_drain_release` now frees the unique, owned subset and reports the rest with the site that queued it. Two shapes are covered: the same span queued twice, and a span queued again after it was already freed. Freeing an unallocated slot is never correct, so dropping those is not a heuristic -- it is the give-back doing what it always should have done. WHY THE W35 GUARD DID NOT COVER IT, which is a coverage answer and not a retrofit. `append_host_mem_release` already carries a sgl-project#719/W35 guard whose own text says "freeing them against the current pool is the W35 double-free". That guard routes by BINDING GENERATION -- it catches a producer naming slots from the pool bound before a cutover. Two producers queueing the same span WITHIN one generation are, to it, two ordinary current-generation batches: structurally invisible, not merely missed. The guard names the hazard, not the gap. SCOPE, stated plainly: this is an INSTANCE fix. It closes the host-region give-back. The CLASS -- an ownership authority over every pooled resource, so a give-back can always ask "is this mine" -- stays open, and its docking points are sgl-project#902 (ownership authority beyond KV rows, where host regions are already listed as a sibling) and #888b's seed "every participant declares the paths of its release sites". Four instances in one day is the argument for doing it once, centrally, rather than a fifth time. FUTURE CHECK: a sixteenth producer needs no edit to be stamped, and a slot it double-queues is named with its own file and line on the first occurrence instead of killing rank 0. Evidence: desk. py_compile on both files; enqueue stamp, accessor and drain check verified in the loaded modules; behavioural probe of the drain algebra on the boot-18 shape (span 60934/5 queued twice + one already-free slot) showing dupes/unowned/freed split correctly AND that the unfixed form would have called free() on the unowned slot -- i.e. the probe can fail. Belegstufe: DESK-BEWIESEN. Boot 19 on the 90-second control arm is the test.

Boot 22 -- the only configuration of this window that reaches decode (27 prefill / 157 decode) -- died after 55 s on PP1: sgl-project#631 PP proxy/batch mismatch: received hidden_states with 253 row(s) for a batch of 302 token(s) (bs=1) READING PROBE FIRST, because four of five suspicious number pairs today turned out to be unit confusions. This one is NOT: `_hs.shape[0]` is the leading dimension of the hidden-state tensor (one row per token) and `_want` is `forward_batch.input_ids.shape[0]` (one entry per token). Same axis, two vocabularies. The guard is sound and it is EARNED -- its own comment records the failure it replaced: "a decode batch of one request was computed on a 2048-row chunked-prefill hidden state, and the mismatched pair reached causal_conv1d_update -- an out-of-bounds write into another request's conv state." Loosening it is not on the table. WHAT WAS MISSING. The sender has declared its width since sgl-project#631 Variant B: `_pp_proxy_stamp` builds (mb_id, seqno, ROWS, epoch). Nobody reads element 2. Every stamp consumer compares only pass identity -- which the #791c note in this very file already calls out as insufficient: "Every identity above answers 'which PASS is this from'; none answers 'which BATCH is it of'." And `_pp_recv_proxy_tensors` POPS the stamp before the payload reaches the one check that does answer the batch question, so that check raises with an anonymous counterparty. This change carries the sender's stamp to that check and prints it. It splits two failures the old text could not: sender_rows == received rows -> payload intact, the PAIRING is wrong (a leftover from another pass); sender_rows != received rows -> the payload itself is wrong (assembled from the wrong result, or altered in transit). An absent stamp prints as "sender unstamped", itself a fact. CARRIED AS AN ATTRIBUTE, NEVER A DICT ENTRY. The pop site is explicit about why: PPProxyTensors' slice path maps `v[key]` over every dict entry and cuda-graph buffer copies iterate the dict, so a tuple left there "would slice to nonsense rather than raise -- the worst available outcome". An attribute is invisible to both, and the smoke asserts it stays out of `.tensors`. PURELY ADDITIVE, DELIBERATELY. No comparison is added at the receive site and nothing is refused that was not refused before. Deriving this rank's expected width there would be a second independent derivation of the very quantity whose disagreement IS the defect, and a wrong derivation would manufacture false refusals -- the mistake sgl-project#995's first version made, which boot 15 punished with a 175-refusal livelock on one rid. The one correct comparison already exists downstream; this only lets it name its counterparty. CLASS NOTE. Three sites were found independently today with one shape: a rank-local predicate deciding a group-uniform quantity (schedule_policy's admission arithmetic, the PP hold actuator, and this send/recv boundary -- where the code writes it out itself: "the recv is guarded by THIS rank's cur_batch while the upstream's send is guarded by the UPSTREAM's, and nothing enforces that the two agree"). The durable fix direction is that the DECISION travels with the PAYLOAD. Here it already does; it was simply never read. This commit is the diagnosis half; the enforcement half needs the next specimen to say which of the two failures above it is. Evidence: desk. py_compile on both files; smoke asserting PPProxyTensors accepts the attribute (not slotted/frozen), that it does NOT leak into `.tensors`, and that both message arms format. Belegstufe: DESK-BEWIESEN. Boot on the Boot-22 configuration is the test -- sgl-project#993 is reachable nowhere else, because no other cell serves long enough to reach the transition.

… re-home by subject Both pins lived in files whose subject was the deleted void relay and went down with it in sgl-project#1072, although neither pin was about the relay: the disciplined release path and `pp_rehome_displaced_chunked_req` both still have a live caller in `_pp_void_own_batch`. Re-pinned here in a file named for the discipline, so the next deletion sweep on the PP ring family runs straight through it. RETRACTION, AND IT IS THE REASON THIS FILE READS THE WAY IT DOES. The predecessor pin `test_the_discipline_has_exactly_one_expression` was RED when its file was deleted and was handed over as a surviving defect. It is not one. It asserted that `free_mamba_cache(` appears NOWHERE in `_release_voided_request`; sgl-project#993 later added a post-failure give-back inside the `except` handler, with a measured justification (boot 12, 7b855f6, 2026-08-28 21:46:05: `release_req` raised, the swallow left a re-admissible request holding a req-pool row, and the memory_pool.py:395 assert killed rank 0 one second later). The stale half was the ASSERTION: its subject was "the normal path delegates", its implementation was "this string never appears in this function", and deliberate containment added later violated the letter while keeping the intent. Measured split: try body release_req( yes; free_mamba_cache( no; token_to_kv_pool_allocator.free( no; dec_lock_ref( no except handlers free_mamba_cache( yes; pool.free( yes; release_req( no So the discipline is now asserted where it is claimed -- on the normal path -- and the containment is asserted to STAY containment: reachable only from a failed release, never raising out of it, mamba given back before the req-pool row it is keyed on. The re-home pins cover the #968b-2 junction that boot 5 died on (the park-shape equality refused the un-parked mid-plan occupant, returned None, and the caller's next statement nulled it: a drop), plus the deliberate log-only reset-shape decline (sgl-project#987) and the queue junction's refusal path. Both pin classes are proven can-fail, not merely green: a mutant that moves the give-back onto the normal path is CAUGHT, and a re-home that stops parking the un-parked occupant is CAUGHT.

…ce probe, ring buffers and one indexed host write per batch for KV and draft pages, preadv Boots xsn129-132: the D group read a store prefix at 250-430 pages/s per rank (11k pages -> 13 s, past the 13.5 s reap budget: completed=2680 of 11806 on xsn132), so every leg-2 request parked, the box idled, and the W1b abort had to end the drain. py-spy on D TP0 during the in-flip read of xsn132 (profile in the job dir, 2663 samples): prefetch issuer thread: 691/710 samples in batch_exists_v2, 533 of them in os.path.exists -- the probe of one stem was exists(sharded) + exists(flat) + exists(sharded) + getsize, and an 11k prefix probes 33k stems (KV, mamba, draft). Measured on the store: 11.5 us a stem as written, 2.7 us as one os.stat. prefetch IO thread: ~half in set_from_flat_data_page (2*layer_num strided slivers per 32 KiB page), the rest split between a fresh pinned get_dummy_flat_data_page() per page (the sgl-project#720 ring served only the extra-pool route, never the KV/draft pages), read_extents' pread alloc+copy, _existing_path's second exists, and the evictor touch. The disk itself: 6-7 us a warm 32 KiB page (readinto/preadv measured). Changes, all on the same read path: hicache_storage._stat_stem: ONE os.stat answers presence, path and size; the legacy flat layout is consulted only when the top directory holds flat .bin files (decided once per backend). _stem_exists, _existing_path and _stem_readable are its three views. cache_controller: _generic_page_get and _draft_page_get_generic borrow their targets from the backend's read ring (_borrow_read_pages, module level so the harness doubles that bind curated methods keep working); a draft miss zeroes its borrowed buffer (sgl-project#993 stays: the zero page is the miss); the served prefix of a batch is written with ONE indexed copy (_set_host_pages -> set_from_flat_data_pages). pool_host/mha.py: set_from_flat_data_pages for layer_first/page_first (one index assignment over the token axis); base.py default loop; memory_pool_host's bound-tier wrapper keeps the per-index stray check. canonical_page_store.read_extents: preadv straight into the target. Tests: test_read_ring_prefix_routes_1402 (6: ring borrow/return, dirty buffer -> zero page, no-ring fallback, raised read returns buffers, batched write == per-page loop for both layouts x page_size 1/2, served prefix written once and stops at the miss) + sharding_558, canonical 1233/706, mem_pool_host, stray_718, draftkv, reissue_939, 1157, 869b, 905, 937, 966, 1063, 861, 0828, 1324, 1401: 221 passed, 7 failed -- all seven red at the tip without this change (3 mem_pool_host + 1 in 905 + 2 in 1063 + the 0828 text scan that trips on a 2026-09-07 comment).

move torch.compile configs into cuda_graph_runner.py

15ce02a

Ying1123 requested a review from hnyls2002 August 8, 2024 19:48

Update server.py

8f37893

Ying1123 commented Aug 8, 2024

View reviewed changes

Comment thread python/sglang/srt/model_executor/cuda_graph_runner.py Outdated

Update python/sglang/srt/model_executor/cuda_graph_runner.py

55936d7

Ying1123 merged commit 9f66250 into main Aug 8, 2024

Ying1123 deleted the fix-torch-compile branch August 8, 2024 20:20

timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025

Move torch.compile configs into cuda_graph_runner.py (sgl-project#993)

b8d79b3

qybnb pushed a commit to qybnb/sglang that referenced this pull request Jul 14, 2026

Merge pull request sgl-project#993 from hhhh1252023/patch-32

855576b

fix 0713

Navigation Menu

Uh oh!

Move torch.compile configs into cuda_graph_runner.py - #993

Move torch.compile configs into cuda_graph_runner.py#993
Ying1123 merged 3 commits into
mainfrom
fix-torch-compile

Ying1123 commented Aug 8, 2024 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Ying1123 commented Aug 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Ying1123 commented Aug 8, 2024 •

edited

Loading