Overlap FSDP communication with compute - #5719
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 9, 2026 04:01
356bf43 to
2d43993
Compare
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 9, 2026 04:59
2d43993 to
700c4f0
Compare
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 9, 2026 06:01
700c4f0 to
68aa5a2
Compare
wujingyue
commented
Jul 9, 2026
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 9, 2026 23:26
9045a0d to
4d2f7de
Compare
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 9, 2026 23:35
4d2f7de to
29f3f1f
Compare
wujingyue
marked this pull request as ready for review
July 9, 2026 23:40
wujingyue
commented
Jul 13, 2026
shjwudp
approved these changes
Jul 14, 2026
shjwudp
reviewed
Jul 14, 2026
| all-gather stream (where the buffer was allocated), gated on a consumer | ||
| event recorded on the compute stream so the free never races the kernels | ||
| that read the buffer. | ||
| """ |
Contributor
There was a problem hiding this comment.
Should we collect the states of unsharded models and release module parameter from all models except the current and next ones?
Contributor
Author
There was a problem hiding this comment.
When would that be useful? Some FsdpModule's post-forward or post-backward failed to be triggered? That sounds like a bug that we shouldn't hide.
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 14, 2026 04:41
bb23148 to
75b45e0
Compare
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 14, 2026 04:45
75b45e0 to
89f8b25
Compare
wujingyue
force-pushed
the
mfsdp-prefetch-on-5513
branch
from
July 14, 2026 04:56
89f8b25 to
698d20a
Compare
jiemingz
approved these changes
Jul 15, 2026
Phlip79
approved these changes
Jul 15, 2026
In pre_forward, issue the next FSDP unit's all-gather on the comm stream
(static module-tree order) so AG_{i+1} launches before F_i finishes, instead
of relying on delayed releases for overlap.
Since prefetch now drives overlap, the delayed-release deque is dropped: each
unit frees its own unsharded storage on the all-gather stream right after its
own compute (event-gated on a compute-stream consumer event so the free never
races the consuming kernels), rather than queuing releases. This bounds peak
in-flight memory to the current unit plus the prefetched next.
Each unit tracks its own materialized state via FsdpModule._is_unsharded, so
pre_forward can skip a unit an earlier unit already prefetched -- no central
id-keyed set or per-forward reset.
Applied on top of the FSDP all-gather overlap work (NVIDIA#5513). Verified against the
mfsdp_v2 unit tests: no new failures vs the base, and forward-peak-memory-bounds
now passes (it failed with the deque). resting-forward (32 MB) and flaky-overlap
fail on the base too. Profiling: prefetch raises concurrent AG/compute overlap
but not forward wall-clock because the SM-based all-gather contends with the
persistent full-occupancy GEMM; the real lever is SM-free (CTA-zero) comm.
Static module order assumes forward order == registration order.
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Reuse the recorded forward order in reverse during backward to prefetch each upcoming FSDP unit's parameters on the all-gather stream. Wait only for the current unit before compute so later all-gathers can overlap backward GEMMs. Split gradient reduction into allocation, packing, and reduce-scatter stages, and launch each reduction immediately on a dedicated stream. Register an autograd-final callback from the root backward pre-hook to order optimizer work after communication while preserving mixed-dtype accumulation and symmetric-memory averaging. Extend the profiler test to require the expected all-gather and reduce-scatter overlap counts and verify that communication and compute use distinct streams. Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
The overlapped backward path allocates the reduce-scatter input buffer (torch.empty) on a dedicated reduce_scatter_stream inside post_backward. Under torch.cuda.graph() capture that stream is not part of the capture at the first allocation, so the allocator falls back to a raw cudaMalloc, which is illegal during capture and invalidates the graph. Fork the reduce-scatter stream from the current stream once at the start of backward (root pre_backward) so it joins the capture before the first reduce-scatter allocation. Later modules are covered by the post-copy fork each preceding module already issues, and the existing post-backward final callback supplies the matching join. Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Add FsdpContext.current_stream() to centralize the repeated torch.cuda.current_stream(<stream>.device) lookups and replace the five duplicated call sites. Also inline a redundant context local in _unshard_parameter_groups. Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
test_overlaps_communication_and_compute demanded the exact theoretical maximum overlap (2*(num_children-1) all-gathers, num_children-1 reduce-scatters) on every rank, with all ranks required to pass. SM-based NCCL collectives share SMs with the GEMMs, so communication overlaps compute only partially and the count varies run to run (CI's coverage instrumentation and tighter runners shift it enough to fail), making the exact-maximum assertion flaky. Assert only that communication meaningfully overlaps compute (all-gather >= 2, reduce-scatter >= 1); the deterministic stream-topology checks already prove the overlap machinery is wired correctly. NCCL symmetric-memory collectives, which run with little to no SM usage, would relieve the contention and let these thresholds be tightened again. Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/29476715799 |
This was referenced Aug 17, 2026
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.
Summary
This implements the explicit prefetching approach used in MFSDP v1 for MFSDP v2 (
mfsdp_v2). The current implementation assumes that static module orders match the execution order closely enough for one-step prefetch.Each FsdpModule's pre-forward hook prefetches the next FsdpModule's parameter all-gather from
forward_order. During backward, pre-hooks follow a separately computedbackward_orderinstead of assuming backward is strictly the reverse offorward_order, which matters for nested module trees.An example trace from the added unit test:
Implementation
Forward prefetch
Each FsdpModule tracks materialization with
_unshard_event._unshard_parameter_groups()is idempotent: if_unshard_eventis already set, the FsdpModule was already unsharded or prefetched; otherwise it enqueues the all-gather on the shared all-gather stream and records_unshard_eventafter materialization.pre_forwardwaits on_unshard_eventinstead of waiting on the whole all-gather stream, so compute depends only on the current FsdpModule's materialization and not on later release work queued on that stream. It then issues the next FsdpModule's all-gather fromforward_order._reshard_parameter_groups()queues release on the same all-gather stream where unsharded storage was allocated, then clears_unshard_eventso the next user enqueues a fresh all-gather.Backward overlap
The shared FSDP context maintains separate static
forward_orderandbackward_ordersequences. Each backward pre-hook:_unshard_event;Gradient reduction is split into allocation, packing, and communication stages. Once a module's gradients are packed, its reduce-scatter is launched immediately on a separate reduce-scatter stream, allowing it to overlap subsequent backward GEMMs and all-gathers. Existing symmetric-memory scaling, mixed gradient dtypes, first-backward assignment, and microbatch accumulation semantics are preserved.
An autograd-end callback registered by the root makes the caller's current stream wait for the reduce-scatter stream before
backward()returns. This cannot rely on the root module's own post-backward hook: that hook tracks gradients for parameters owned by the root and may run before descendants finish, or may not run at all when the root owns no trainable parameters.Verification
test_fully_shard.pypasses with two CUDA ranks after the_unshard_eventchange: 14 passed per rank.