Repository navigation
[Diffusion] Correct the resident-layer help text to match its scope - #40593
Merged
mickqian merged 3 commits intoSep 22, 2026
Merged
Conversation
Both resident-layer flags describe behaviour the runtime does not have. --dit-layerwise-resident-layers calls the layers "permanently resident on GPU" and says they are "transferred once". --layerwise-resident-layers goes further: "transferred once at startup rather than streamed", cutting "the transfer of every pass including the first", and names the case explicitly -- "an auxiliary component that runs once per request still benefits". The resident set is released when a component's use ends. For a DiT that costs nothing, because one use spans every denoise step. For a component whose use is a single forward pass, the set is prefetched at the start of the use and dropped at the end of it, and the next request transfers all of it again. Measured on Qwen-Image-2.1, `--layerwise-resident-layers text_encoder=0.8` reports `resident=53/66` and changes neither steady VRAM (17047 MiB either way) nor latency (14.168s against 14.136s). Corrects both help texts and adds "(per use)" to the readiness log, where the bare `resident=53/66` reads as a standing state. No behaviour change; this is the claim catching up with the code. The warning worth having -- telling an operator their setting cannot pay -- needs to know how many times a component runs per request, which `estimate_layerwise_layer_uses` computes from warmup records in auto_residency.py rather than anywhere this code can see, so it belongs with the residency planner and is not approximated here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mickqian
requested review from
AgainstEntropy,
BBuf,
HaiShaw,
ping1jing2 and
yichiche
as code owners
September 21, 2026 14:23
Collaborator
Author
|
/tag-and-rerun-ci extra |
Three follow-ups to the same mistake, found by CI. `test_configure_logs_component_start_and_completion` pins the startup log line verbatim, and adding "(per use)" to it broke that assertion. Updated, with a comment saying why the words are there so the next edit does not quietly drop them. `docs/.../api/cli.mdx` repeats the claim the flags were making -- "Resident layers are transferred once at startup, so they are removed from every pass" -- and its own per-component example sets the flag on `text_encoder`, which is exactly the component where it does nothing. Corrected both, and moved the example to `dit`. The per-component help said the flag buys nothing for "an auxiliary component". That is too broad in the direction that costs a user performance: `deployment_cookbook.mdx` measures `video_vae=36` at 13 s of decode against 150 s streamed, because a video VAE's single use makes many passes over the layers, the same shape as a DiT across denoise steps. The condition is the number of passes per use, not whether a component is auxiliary, so the help now says that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mickqian
requested review from
JustinTong0323,
sogalin,
wisclmy0611 and
zijiexia
as code owners
September 21, 2026 15:19
"Use" is a `ComponentUse` -- the runtime's unit of a component's work, and not a word a user setting a CLI flag has any reason to know. Both help strings, the CLI reference and the readiness log now say what happens in the terms the user already has: the layers are held while the component does its work for a request and released when it finishes, so they are transferred once per request rather than once per step or pass. The flag pays for a component that runs its layers many times per request (a DiT across denoise steps, a video VAE across latent chunks) and does nothing for one that runs them once (a text encoder). Verified against the runtime before wording it: the set is pinned by the layer-0 pre-hook and released by `finish_use`; `finish_request` forces `preferred = False` for any module holding residents, so nothing keeps it past the request. "Per request" is the user-facing truth of that. Also corrects the example key I had just introduced. `dit=4` is silently ignored by `--layerwise-resident-layers`: `_parse_component_value_map` does no aliasing and the DiT's component name is `transformer`. The doc example now uses `transformer=4`; the help uses `video_vae=36`, the one measured per-component recipe in the repo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 21, 2026
Collaborator
Author
|
CI status note (from the author's triage, so reviewers need not re-derive it). Every red on this PR falls in one of three buckets, none of them from the diff:
|
mickqian
added a commit
to mickqian/sglang
that referenced
this pull request
Sep 22, 2026
Resolves the textual overlap with sgl-project#40593 (help strings, CLI reference, readiness log): the merged text keeps sgl-project#40593's user vocabulary and adds the lifetime axis on top, and the readiness log names the lifetime (`(forward)` / `(permanent)`) where sgl-project#40593 wrote `(per request)`. Brings in sgl-project#40590 and sgl-project#40592, which this PR composes with unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
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.
Third of three, independent of the other two — this one needs no behaviour change
at all, because the behaviour is fine and the description is not.
Stack
release_after_usefromrelease_allThe claims
--dit-layerwise-resident-layers:--layerwise-resident-layers:What happens
The resident set is released when a component's use ends —
LayerwiseOffloadStrategy.finish_use→release_all(), whose own docstring saysit "ends the denoise stage that the resident set is scoped to".
For a DiT that costs nothing: one use spans every denoise step, so "transferred
once per use" and "transferred once per request" coincide, which is the case both
flags were written against. For a component whose use is a single forward pass
they do not coincide at all — the set is prefetched at the start of the use and
dropped at the end, and the next request transfers all of it again.
Measured on Qwen-Image-2.1, one RTX 5090, five requests per arm:
resident=0/66--layerwise-resident-layers text_encoder=0.8resident=53/66Identical memory, identical latency, identical per-step time. The setting is
accepted and reported and has no effect — and the help text for it names exactly
this case as one that benefits.
What this PR changes
Both help strings, the CLI reference and the readiness log, in the user's own
terms. The layers are held while the component does its work for a request and
released when it finishes, so they are transferred once per request rather than
once per step or pass -- and not kept for the life of the server. The readiness
log says
resident=53/66 (per request)where the bare number read as a standingstate.
"Per request" rather than the runtime's "per use" on purpose: a use is a
ComponentUse, the runtime's unit of a component's work, and not a word a usersetting a CLI flag has any reason to know. Verified against the runtime before
wording it that way: the set is pinned by the layer-0 pre-hook on the component's
first forward, released by
finish_use, andfinish_requestforcespreferred = Falsefor any module holding residents, so nothing keeps it pastthe request. In every ordinary pipeline the two scopes coincide.
docs/docs/sglang-diffusion/api/cli.mdxcarried the same claim in its own words-- "Resident layers are transferred once at startup, so they are removed from
every pass" -- and its per-component example set the flag on
text_encoder,which is the one component where it demonstrably does nothing. Both corrected;
the example now uses
transformer=4. (Notdit=4, which I tried first: theper-component map does no aliasing --
_parse_component_value_mapis literaland
_pickmatches the component's real name -- sodit=4is silentlyignored, a worse example than the one it replaced.) The help's own example is
now
video_vae=36, the one measured per-component recipe in the repo.One correction to my own first wording, from reading the cookbook: the help said
the flag buys nothing for "an auxiliary component", and that is too broad in the
direction that costs a user performance.
deployment_cookbook.mdxmeasures--layerwise-resident-layers video_vae=36at 13 s of decode against 150 sstreamed, because a video VAE's single use makes many passes over its layers --
the same shape as a DiT across denoise steps, not the text encoder's single
forward pass. The condition is how many times the component runs its layers per request, not
whether it is auxiliary, and the help now says so.
Nothing else. The diagnostic actually worth having — telling an operator at
startup that their setting cannot pay — has to know how many times the component
runs per request.
estimate_layerwise_layer_usesinauto_residency.pycomputesthat, from warmup records this code has no access to, so the warning belongs with
the residency planner. I have not approximated it here.
Tests
test_resident_layer_help_describes_the_actual_scopeasserts the retired phrasesare gone and the scope is stated, because a help string is exactly the kind of
claim that drifts back when someone edits nearby.
test_server_args.py: 4 failed / 195 passed against clean main's 4 failed / 194passed — the same pre-existing failures, plus this one.
CI then caught what that run did not cover:
test_layerwise_offload.py'stest_configure_logs_component_start_and_completionpins the readiness log lineverbatim, so annotating it broke that assertion. Fixed in
0e18e49, with acomment on the assertion saying why the words are there.
test_resident_layer_help_describes_the_actual_scopenow anchors on theuser-facing claims --
once per request,life of the server,has no effect-- rather than on runtime vocabulary (
6db268e).🤖 Generated with Claude Code
CI States
Latest PR Test (Base): ✅ Run #35619823972
Latest PR Test (Extra): ✅ Run #35619823478
Latest PR Test (AMD ROCm 10): ❌ Run #35619824063