Repository navigation
[Diffusion] Separate a use-scoped layerwise release from release_all - #40590
Conversation
…yerwise offload `LayerwiseOffloadStrategy.finish_use` called `manager.release_all()`, and `release_all` did exactly what its name says: dropped every layer, the resident set included. Those are two different questions. When a component's use ends the streamed window is certainly dead, but whether the resident set is depends on whether anything will want it before another stage needs the room — and `release_all` is also the right call for the full reset in `enable_offload`, which must leave nothing behind. Conflating them is why `--layerwise-resident-layers` does nothing for any component whose use is a single forward pass rather than a denoise loop: the set is prefetched at the start of the use and dropped at the end of it, every request. For the DiT one use spans all denoise steps, so the set survives and earns its memory back; for a text encoder it never survives anything. This change only names the two operations apart: release_after_use(keep_resident=False) the use ended release_all() a full reset `finish_use` now says `release_after_use()`, and the default argument makes that byte-identical to what it did before — a test pins that. `enable_offload` keeps `release_all()`. `keep_resident=True` has no caller yet; giving the decision a home is a separate change. `release_after_use` rather than `end_use` because `ComponentManager.end_use` already exists with an unrelated signature, and `manager.end_use(...)` would read ambiguously across the two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
/tag-and-rerun-ci extra |
|
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:
|
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>
First of three, splitting a naming problem from the behaviour change it hides.
The two questions that were one call
LayerwiseOffloadStrategy.finish_usecalledmanager.release_all(), andrelease_alldid what its name says — dropped every layer, the resident setincluded. But its two callers want different things:
enable_offloadfinish_useA use ending is the narrower question. The streamed window is certainly dead;
whether the resident set is depends on whether anything will want it before
another stage needs the room. There was no way to say that, so
finish_usesaid"drop everything".
Why it matters
That is why
--layerwise-resident-layersdoes nothing for any component whoseuse is a single forward pass rather than a denoise loop. The set is prefetched at
prepare_for_useand dropped atfinish_use, every request. For the DiT one usespans all the denoise steps, so the set survives the loop and earns its memory
back — which is the case the feature was built for, under its original name
--dit-layerwise-resident-layers. The per-component override generalised theflag to other components and inherited a DiT-shaped scope with it.
Measured on Qwen-Image-2.1 / one RTX 5090, five requests per arm:
resident=0/66--layerwise-resident-layers text_encoder=0.8resident=53/66The flag is accepted,
resident=53/66is logged, and nothing moves. Its own helptext meanwhile promises the opposite, naming this exact case:
What this PR does
Only names the two operations apart:
finish_usenow saysrelease_after_use(). With the default argument that isbyte-identical to what it did before, and
test_release_after_use_defaults_to_the_old_release_allpins it.
enable_offloadkeepsrelease_all(), andtest_release_all_still_drops_everythingpins that it did not inherit theresident-set exemption.
No behaviour change.
keep_resident=Truehas no caller in this PR; givingthat decision a home is #2 of the stack, and it needs a memory picture this layer
does not have — a naive local threshold takes the same pipeline from 14.136 s to
13.737 s on a 31.4 GiB card and stops a 23.5 GiB one from starting at all.
Naming
release_after_userather thanend_use:ComponentManager.end_use(use, module=…)already exists with an unrelated signature, and
manager.end_use(...)would readambiguously across the two.
release_after_usealso sits in this file's existingvocabulary —
release_layer,release_all,_release_unneeded_streamed_layers.Tests
Four, in
test_layerwise_offload.py: the default equals the old call;release_allstill drops everything;
keep_resident=Truekeeps exactly_retained_setandskips the first-pass fault-in those layers do not need;
keep_resident=Truewithno residents configured is still a full release.
Whole-file run: 81 failed / 51 passed against clean main's 81 failed / 47 passed —
the same pre-existing failures on a machine without CUDA, plus these four.
🤖 Generated with Claude Code
CI States
Latest PR Test (Base): ✅ Run #35611211982
Latest PR Test (Extra): ✅ Run #35611211877
Latest PR Test (AMD ROCm 10): ❌ Run #35611211509