fix(fsdp): stage weight-sync buckets in a persistent buffer to stop CUDA-IPC pinning#65
Open
zhihengy wants to merge 1 commit into
Open
fix(fsdp): stage weight-sync buckets in a persistent buffer to stop CUDA-IPC pinning#65zhihengy wants to merge 1 commit into
zhihengy wants to merge 1 commit into
Conversation
zhihengy
added a commit
that referenced
this pull request
Jul 25, 2026
…to stop CUDA-IPC pinning
zhihengy
force-pushed
the
fix/wsync-ipc-staging
branch
from
July 25, 2026 04:19
b81aeb6 to
87e4802
Compare
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.
Problem
DiffusionUpdateWeightFromTensor.update_bucket_weightsserializes each bucket's flattened tensor withMultiprocessingSerializer, which exports a CUDA-IPC handle for the underlying storage. The rollout engine imports the handle and — like any long-lived torch consumer — keeps the mapping open instead of closing it per request (open/close is an expensive driver call, so torch caches imports). A storage whose handle is still held by a peer can never be reclaimed on the producer side:torch.cuda.ipc_collect()only sweeps storages whose import refcount already dropped to zero. We haddel+ipc_collecton the trainer side andgc.collect+ipc_collect+empty_cacheon the engine side; none of it releases a single byte while the peer holds the handle.Since every bucket cat-ed a fresh flattened tensor, every bucket exported a new storage, and each one stayed pinned in the trainer process. A full sync accumulates one model's worth of trainer memory per rank: ~32GB for a 16B model (silently eating a quarter of an H200), ~128GB for a 64B model — the trainer OOMs partway through its first sync. Three consecutive OOMs during the 64B bring-up landed at exactly the same allocation (67.5GB ≈ 34 buckets), which is what pointed at the sync path.
Fix
Keep one persistent staging buffer per parameter dtype. Each bucket is copied into the staging buffer and the serialized payload references a view of it. The whole run therefore exports exactly one storage per dtype; the engine's cached import maps that same storage forever, and the transient cat result — never exported — is freed normally each bucket. Overwrite safety across buckets comes from the existing
ray.getonupdate_weights_from_tensor: the engine has finished reading before the next bucket is staged.Two details that fell out of testing:
flat.dtype.FlattenedTensorBucketflattens every dtype group to a byte view, soflat.dtypeis always uint8 and all groups of a mixed-dtype bucket would collapse onto one buffer. Groups are serialized before any payload is sent, so the second group's copy overwrites the first payload's bytes while the engine may still read them. Worse, visibility of writes made after serialization is undefined across processes (the IPC event is recorded at serialize time) — in testing the engine read NaN weights on some buckets and stale-but-clean bytes on others. Keying by parameter dtype gives concurrent payloads disjoint buffers. All-fp32/all-bf16 state dicts produce one group per bucket and never hit this, but configs that load a frozen base in bf16 do: diffusers'_keep_in_fp32_moduleskeeps e.g.time_embedderin fp32 (Cosmos3OmniTransformer,WanTransformer3DModelpins five module names), so the bucket holding those params carries two dtype groups.min(state-dict per-dtype total, max(--update-weight-buffer-size, largest param)). The bound was checked against a brute-force replay of the bucketing loop on mixed-dtype state dicts.Also logs trainer
memory_allocatedevery 16 buckets ([wsync-mem]) so a regression here shows up as a rising line instead of a mystery OOM.Benchmarks
A/B harness driving the real
update_bucket_weights(this branch vs its parent) over ray against a consumer that replicates the sgl-dWeightsUpdaterflattened-bucket path, including its end-of-callgc.collect/ipc_collect/empty_cache. 12 buckets x 2GB bf16, single H200, payload checksums verified:The leak is exactly (engine-side reference depth) x (bucket size); the staged version is flat regardless of engine behavior because every held payload references the same storage. Latency is unchanged within noise — the staging copy is a 2GB device-to-device copy, ~0.7ms/bucket in an isolated microbench of the transfer path (same microbench puts the alternative fix, forcing the engine to close and reopen the handle every bucket, at 50-80ms/bucket).
Additional runs for the two details above:
In the real 64B run the fix took a full 128GB sync from OOM to a flat 3.9-6.8GB
[wsync-mem]line, 25.5s per sync.Notes
Split out of the cosmos3 branch: the bug affects any model large enough that a full sync's volume doesn't fit in spare GPU memory, and colocated small models merely hide it.