fix(profile): stop the per-batch-size graph-capture exporter taking the server down - #1399
fix(profile): stop the per-batch-size graph-capture exporter taking the server down#1399fengshaoyi-amd wants to merge 1 commit into
Conversation
fengshaoyi-amd
changed the title
CI E2E report — ❌ Failed
|
|
@fengshaoyi-amd Can you please tell what exact issue are you facing and for which workload. We intentionally store one capture trace per batch size since that is required for downstream analysis flow and this change might break that. |
…he server down SGLANG_ENABLE_CUDA_GRAPH_CAPTURE_TRACE replaces SGLANG_GRAPH_BATCH_CAPTURE. The per-bs exporter indexes a list built from capture_bs once per profiler callback, but capture() fires one callback per (bs, lora, dsa) combination, so a DSA dual-graph model -- dense and sparse for every batch size -- walks off the end of the list and takes the server down during graph capture, before it is ready to serve. Upstream gives the single-trace flag precedence when both are set (decode_cuda_graph_runner.py::_graph_batch_capture_active, from sgl-project/sglang#24370), so an inherited SGLANG_GRAPH_BATCH_CAPTURE=1 cannot re-select the crashing path and does not need to be pinned to "0". The key also moves to the graph-capture section it belongs to. The layout it writes is not a drop-in, and this commit stops that being silent. One combined file per rank holds every batch size at once and carries no variant identity in its name, so the shape-manifest classifier rejects it -- deliberately, since admitting it mints one shape-identical variant per rank and defeats the dedup behind variant_count. The identity is not inside the file either: the capture-phase record_function spans are not emitted on this path, measured on a GLM-5.2 MI355X capture at 15166 user_annotation events with not one capture span among them. Nor is the sibling execution_details.json a way out, though it carries the right schema: on this layout it records a single batch_size for a file holding every batch size (256 for all eight decode ranks, 8 for all eight EAGLE draft ranks in that same capture), so admitting the file on its strength would hand Forge a dense shape that is wrong rather than missing. So the manifest degrades to eager -- which is also what an eager profile produces legitimately, leaving the two indistinguishable downstream. It now emits shape_manifest_capture_not_indexable when capture ran and only its layout was unreadable, naming the files. A server that starts takes precedence over a manifest that indexes, but the trade is recorded rather than inferred. Three comments described these layouts as evidence of an unpatched SGLang. All three are upstream exporters selected by env var, and after this change the combined one is the default, so the misreading would now point at the healthy configuration. Corrected, along with the capture_traces/bs_* paths in the diagnostics, which name the pre-#24370 layout. Based on #1399 by Shaofeng Feng <shafeng@amd.com>. Co-authored-by: Cursor <cursoragent@cursor.com>
3c7c493 to
c52c929
Compare
…he server down SGLANG_ENABLE_CUDA_GRAPH_CAPTURE_TRACE replaces SGLANG_GRAPH_BATCH_CAPTURE. The per-bs exporter indexes a list built from capture_bs once per profiler callback, but capture() fires one callback per (bs, lora, dsa) combination, so a DSA dual-graph model -- dense and sparse for every batch size -- walks off the end of the list and takes the server down during graph capture, before it is ready to serve. Upstream gives the single-trace flag precedence when both are set (decode_cuda_graph_runner.py::_graph_batch_capture_active, from sgl-project/sglang#24370), so an inherited SGLANG_GRAPH_BATCH_CAPTURE=1 cannot re-select the crashing path and does not need to be pinned to "0". The key also moves to the graph-capture section it belongs to. The layout it writes is not a drop-in, and this commit stops that being silent. One combined file per rank holds every batch size at once and carries no variant identity in its name, so the shape-manifest classifier rejects it -- deliberately, since admitting it mints one shape-identical variant per rank and defeats the dedup behind variant_count. The identity is not inside the file either: the capture-phase record_function spans are not emitted on this path, measured on a GLM-5.2 MI355X capture at 15166 user_annotation events with not one capture span among them. Nor is the sibling execution_details.json a way out, though it carries the right schema: on this layout it records a single batch_size for a file holding every batch size (256 for all eight decode ranks, 8 for all eight EAGLE draft ranks in that same capture), so admitting the file on its strength would hand Forge a dense shape that is wrong rather than missing. So the manifest degrades to eager -- which is also what an eager profile produces legitimately, leaving the two indistinguishable downstream. It now emits shape_manifest_capture_not_indexable when capture ran and only its layout was unreadable, naming the files. A server that starts takes precedence over a manifest that indexes, but the trade is recorded rather than inferred. Three comments described these layouts as evidence of an unpatched SGLang. All three are upstream exporters selected by env var, and after this change the combined one is the default, so the misreading would now point at the healthy configuration. Corrected, along with the capture_traces/bs_* paths in the diagnostics, which name the pre-#24370 layout. Based on #1399 by Shaofeng Feng <shafeng@amd.com>. Co-authored-by: Cursor <cursoragent@cursor.com>
c52c929 to
d056abf
Compare
|
@mohbasit Thanks — and you are right that this change breaks the per-batch-size Workload and exact failureGLM-5.2 (MXFP4) on MI355X, TP=8, DSA dual-graph capture enabled. The server
# _init_profile_context_and_memory_record, lines 781-791
self._profile_bs_list = list(reversed(self.capture_bs))
self._profile_bs_idx = 0
def on_trace_ready(prof):
bs = self._profile_bs_list[self._profile_bs_idx] # <-- unguarded index
trace_file = os.path.join(trace_dir, f"{runner_name}_bs_{bs}_rank{rank}.json.gz")
prof.export_chrome_trace(trace_file)
self._profile_bs_idx += 1But for bs in capture_range:
for variant_label, _variant_has_lora in lora_variants:
for dsa_variant in dsa_variants:
self.capture_one_shape(bs, ...)With 8 capture_bs and DSA dual-graph (dense + sparse per bs) that is 16 The part I think matters more to youThe crash is not the worst of it. The first 8 callbacks succeed, and 7 of So on a multi-variant model the per-bs layout does not deliver the guarantee On the combined trace — your concern is correctWe measured it, and it is worse than "might break": the combined layout carries We switched only because a degraded manifest is recoverable and a server that What I would rather doName the trace after the shape actually being captured, instead of indexing a self._set_profile_capture_tag(bs, variant_label, dsa_variant) # before capture_one_shape
# -> DecodeCudaGraphRunner_bs_8_dense_rank0.json.gz
# DecodeCudaGraphRunner_bs_8_sparse_rank0.json.gzReplayed against four capture shapes:
Single-variant models produce byte-identical filenames, so nothing in your Applies cleanly to v0.5.18 and to upstream main (the bug is still there on main). |
3 participants
Why
profile_sglang.yamlselects SGLang's per-batch-size graph-capture exporter.That exporter cannot survive a DSA dual-graph model: it names each trace after a
batch size, tracked by one index advanced once per profiler callback.
But
capture()fires one callback per(bs, lora, dsa)combination. A modelthat captures a dense and a sparse graph for each batch size therefore raises
the index past the end of the list, and the server goes down during graph
capture — before it is ready to serve a single request.
Switching to the single-trace exporter takes that path out of play. Upstream
gates the per-bs branch on the single-trace flag being unset
(
decode_cuda_graph_runner.py:765-766), so setting it is enough; the buggyindexing is never reached.
Behaviour change and blast radius
Breaking changes: no. This is a profile-time capture setting; nothing in the
serving or baseline path reads it.
Graph-capture trace layout changes. One combined trace instead of one file
per
(batch size, rank)._capture_shapes.pyclassifies capture sidecars byshape rather than by an exact name, and matches
graph_captureanywhere in afilename, so the combined trace is still recognised as a capture rather than
mistaken for the workload trace.
What this does not fix
Stated so the PR is not read as closing more than it does. A profile round can
still come back with an empty hot-kernel table, for reasons this change does not
touch:
TraceLens issue, tracked there;
hipGraphLaunchunder cuda-graph capture(CUDA-graph capture folds kernels into hipGraphLaunch → trace_analyze hot_kernels=0 (#210) #431), which is what actually empties the steady-state window in the captures
looked at here;
_AGENTX_PROFILE_MAX_ITERS = 8capture cap, which lives in_workload_envs.pyand carries its own host-RAM rationale.Dropped from the first revision of this PR
The first revision also set
SGLANG_PROFILE_WITH_STACKtofalse, on theargument that callstacks crowd GPU kernels out of the steady-state window. That
argument does not hold and the change has been reverted.
python_functionevents carry no correlation id — 0 of 641782 in the capturethat was quoted — and the splitter counts a chunk's GPU events solely by
collecting correlation ids from the CPU events inside a step window and looking
up the GPU events they map to. Callstacks never enter that count; removing them
changes it by zero.
A closer control says the same. Qwen3-0.6B
20260826T072156Z-edacec52profiledat 97.33%
python_function, higher than the 94.62% capture quoted asevidence, under the same concurrency, the same two profiler env vars and the
same 128 steps — and did not hit
steady_state_chunk_empty. Both capturescollapse the same way instead: 128
hipGraphLaunch, exactly one of whichcarries GPU events, leaving 127 of 128 steps with zero. Which chunk the splitter
lands on decides whether a run survives.
The blast-radius check in that revision was also wrong to conclude nothing reads
a callstack. It searched for the env var name, which does appear once. But
_trace_launcher_resolverreads thepython_functionchain rather than the envvar, and runs unconditionally from
tracelens_analysis.py:5064to recover aPython launcher frame for the kernels TraceLens files as
(Synthetic Op)withlauncher_path = "Not found"— hand-written Triton and AITER kernels, the oneskernel optimisation is most likely to want. Without callstacks it falls back to
name-grep, which yields no line number and can match a test file or a CPU
implementation. TraceLens' own
mla_decode_pseudo_opsmatchespython_functionnames too.
Verification
The four suites that read this config: 381 passed.
The config parses and materialises the intended values, and no reference to the
replaced env var remains anywhere in the tree.