Skip to content

[Feat] Route FSDP weight all-gather through copy-engine symmetric memory - #65

Merged
jiahy0825 merged 16 commits into
SandAI-org:mainfrom
wtr0504:feat/ce_ag_reuse_torch
Sep 9, 2026
Merged

[Feat] Route FSDP weight all-gather through copy-engine symmetric memory#65
jiahy0825 merged 16 commits into
SandAI-org:mainfrom
wtr0504:feat/ce_ag_reuse_torch

Conversation

@wtr0504

@wtr0504 wtr0504 commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

🗂️ PR Category

  • ✨ New Feature
  • 🚀 Optimization (performance, memory, etc.)
  • 💥 Breaking Change
  • 🐛 Bug Fix
  • 🛠️ Development / Refactoring
  • 📚 Documentation
  • 🧹 Chore (Dependencies, CI/CD, Configuration, etc.)
  • 🧪 Testing

📝 Description

Add a copy-engine (CE) transport for SimpleFSDP weight all-gather, as an alternative to NCCL ring kernels.
Today enable_fullgraph_overlap hides weight gathers under upstream compute, but those gathers still occupy SMs and contend with the compute that is supposed to hide them. With fsdp_config.transport="copy_engine":

  1. Bind eligible FSDP weights into torch.distributed._symmetric_memory windows, driven from the captured FX graph between lowering and bucketing, using the live tensors Dynamo handed the backend. Windows are pooled per (process group, dtype) and capped at 4 GiB, so dense FSDP and MoE edp meshes never share a rendezvous. The publish barrier is paid once at bind, never per step.
  2. Gate on what the copy engine can actually serve: a single even Shard(0) DTensor, contiguous, on CUDA, whose gather input is exactly to_local(placeholder|get_attr). An uneven Shard(0) is refused — the gather copies one fixed-size slab per peer, so a rank whose peers own fewer rows would read past the end of theirs. The decision is agreed across ranks before anything moves: a gather retargeted on only some ranks would never complete.
  3. Rewrite the marked gathers to magi::ce_all_gather / magi::ce_all_gather_coalesced. Bucketing splits on the bind flag so a bucket is all-bound or all-unbound; cast/pad gathers and anything left unbound stay on NCCL, so mixed graphs are legal.
  4. Gather by peer copy-engine reads (cudaMemcpyBatchAsync, one submission per bucket) into a fresh dest — zero SM occupancy, no per-step cross-rank barrier. Weights are static in inference, so the NCCL barrier is wasted work.
  5. Reuse the existing latest-safe-launch reorder: CE ops lower to FallbackKernel, so the pass now treats magi::ce_all_gather* as collectives (_issues_transfer) instead of counting them as compute.
  6. Cost-model the gather as wait(launch()). Timing the launch alone measures CPU issue (~3 µs) and misses the side-stream copies, which undersizes the overlap window by an order of magnitude.
    Default remains transport="nccl". CE requires NVLink within the FSDP mesh dim and static weights (inference).

Config

Option Default Purpose
fsdp_config.transport "nccl" "nccl": SM ring kernels. "copy_engine": symmetric-memory peer copies.
fsdp_config.symm_min_shard_mib 0 Size floor for binding. Local shards below it stay on NCCL; 0 binds every eligible weight.
Enable with existing fullgraph overlap:
compile_config.disable_graph_split = True
compile_config.cudagraph_mode = CudaGraphMode.NONE
compile_config.fsdp_config.enable_fsdp = True
compile_config.fsdp_config.transport = "copy_engine"

@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Aug 29, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Aug 29, 2026
Comment thread magi_compiler/passes/fsdp_overlap/symm_ag_rewrite.py Outdated
Comment thread magi_compiler/symm_mem/arena.py Outdated
Comment thread magi_compiler/_api.py

@jiahy0825 jiahy0825 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image Another review comment from ai~

@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 7, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 7, 2026
@wtr0504
wtr0504 force-pushed the feat/ce_ag_reuse_torch branch from 606d1ab to 1070f5e Compare September 7, 2026 13:41
@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 7, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 7, 2026
@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 7, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 7, 2026
…s memory

``symm_all_gather`` named the precondition -- the input must live in a symmetric
window -- rather than the mechanism, and symmetric memory does not imply the
copy engine: PyTorch's own symm_mem all-gathers run on SM kernels.  Not doing
that is this op's entire reason to exist, and it is what the reorder pass and
the cost model key off, so the name should say it.

Rename the op to ``magi::ce_all_gather`` / ``_coalesced``, matching the
vocabulary already in use at ``transport="copy_engine"`` and
``node_meta.CE_BOUND``.  The buffer, binding and registry keep the ``symm``
name: those really are about the memory model, and the split now marks the
boundary instead of blurring it.

The uneven-transport helper compared sorted results against a literal whose
order encoded the old label's alphabetical position, so its expectation moves
with the rename.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 7, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 7, 2026
@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 8, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 8, 2026
@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 8, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 8, 2026
@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 9, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 9, 2026
@wtr0504 wtr0504 added the ci:run Trigger CI integration tests label Sep 9, 2026
@github-actions github-actions Bot removed the ci:run Trigger CI integration tests label Sep 9, 2026

@jiahy0825 jiahy0825 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jiahy0825
jiahy0825 merged commit d8f2c74 into SandAI-org:main Sep 9, 2026
30 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants