[c10d][xccl2] Port the XCCL work object, work queue and bootstrap - #7
Open
frost-intel wants to merge 1 commit into
Open
frost-intel wants to merge 1 commit into
frost-intel wants to merge 1 commit into
Conversation
Ports comms/torchcomms/xccl/TorchWorkXCCL.{hpp,cpp},
TorchWorkXCCLQueue.cpp and TorchCommXCCLBootstrap.{hpp,cpp} to
torch/csrc/distributed/c10d/xccl2/ as WorkXCCL.{hpp,cpp},
WorkXCCLQueue.cpp and XCCLBootstrap.{hpp,cpp}. Still not referenced by
anything; build wiring lands later in the stack.
Behaviour is kept identical to torchcomms. Completion is still tracked with a
pair of XPU events, and the timeout is still wall-clock measured from the
moment the *start* event is observed complete, so time spent queued behind
earlier collectives is not charged against a work's timeout.
Structural changes, all mirroring how nccl2 ported the NCCL equivalents:
- WorkXCCL derives from c10d::Work rather than torchcomms::TorchWork, so the
result()/getFuture() handling that the torchcomms BackendWrapper used to
supply is folded into the work itself, and the wait pre/post hooks (a
TorchWork concept) are dropped.
- The back-pointer to the owning backend is dropped entirely rather than
ported. torchcomms holds an owning shared_ptr<TorchCommXCCL>, which c10d
cannot mirror because backends are owned by intrusive_ptr, but demoting it
to a raw pointer would be unsound: a work can outlive its backend. Python
may still hold the handle returned by an async collective when
destroy_process_group() runs, and ~WorkXCCL returns its events to the
backend's pool, so the destructor would lock a destroyed mutex inside freed
memory. Instead the work borrows nothing: it snapshots the name, rank, size
and device it needs for logging and stream lookup into an XcclCommInfo at
construction, and shares ownership of the event pool, which is hoisted out
of the backend into a shared_ptr<XcclEventPool> so it outlives it. An
owning intrusive_ptr back to the backend was rejected because the backend
reaches the work through its queue, and the resulting cycle would leak a
communicator whenever the user never calls destroy_process_group().
- torchcomms' XpuApi indirection is not ported. It exists so torchcomms can
mock the driver in its unit tests; in-tree, at::xpu::XPUEvent wraps the same
sycl::event, so eventRecord/eventQuery/streamWaitEvent become
record()/query()/block(). Because XPUEvent reports errors by throwing rather
than by returning a status, the XPU_ERROR_NOT_READY / XPU_ERROR_UNSUPPORTED
checks become try/catch, matching nccl2's handling of CUDAEvent.
- Completed work is parked on a completed_work_queue_ and released by the next
caller-thread enqueueWork() instead of being dropped inline in checkStatus().
This is nccl2's fix, taken deliberately: dropping the last reference to an
at::Tensor from the watchdog thread runs tensor destruction off a thread
c10d does not control.
- XCCLBootstrap takes rank/size/store from the ProcessGroup and keys the
unique ID off a PrefixStore as "xccl_storekey_<generation>". torchcomms
additionally supported building its own TCPStore from MASTER_ADDR/MASTER_PORT
when no Store was supplied; in c10d the Store is always supplied, so that
path -- along with TORCHCOMM_XCCL_BOOTSTRAP_UNIQUEID_EXCHANGE_METHOD, the
internal-store teardown barrier and its XPU barrier buffer -- is unreachable
and is not ported. Same reduction nccl2 made.
populateXcclConfigFromHints keeps torchcomms' targeted warning for hints that
are NCCL-only (trafficClass, CTAPolicy, nvlsCTAs, ...) so that a hint dict
carried over from nccl2 reports which keys oneCCL has no equivalent for. The
strdup() for config.netName is kept as-is: oneCCL's ownership of that pointer
past onecclCommInitRankConfig is undocumented, so this deliberately leaks
rather than risk a dangling pointer.
Typechecked against the real torch_xpu compile flags; all three TUs report
only the build tree's pre-existing stale-TensorBody.h error, same as the
untouched control TUs.
This was referenced Aug 25, 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.
Stack (bottom → top)
xccl2/01-foundationxccl2/02-apixccl2/03-workxccl2/04-engine-declxccl2/05-engine-implxccl2/06-backend-surfacexccl2/07-build-wiringxccl2/08-testsxccl2/09-integration-testsEach PR is exactly one commit and targets the one below it. Review bottom-up.
Ports comms/torchcomms/xccl/TorchWorkXCCL.{hpp,cpp},
TorchWorkXCCLQueue.cpp and TorchCommXCCLBootstrap.{hpp,cpp} to
torch/csrc/distributed/c10d/xccl2/ as WorkXCCL.{hpp,cpp},
WorkXCCLQueue.cpp and XCCLBootstrap.{hpp,cpp}. Still not referenced by
anything; build wiring lands later in the stack.
Behaviour is kept identical to torchcomms. Completion is still tracked with a
pair of XPU events, and the timeout is still wall-clock measured from the
moment the start event is observed complete, so time spent queued behind
earlier collectives is not charged against a work's timeout.
Structural changes, all mirroring how nccl2 ported the NCCL equivalents:
result()/getFuture() handling that the torchcomms BackendWrapper used to
supply is folded into the work itself, and the wait pre/post hooks (a
TorchWork concept) are dropped.
ported. torchcomms holds an owning shared_ptr, which c10d
cannot mirror because backends are owned by intrusive_ptr, but demoting it
to a raw pointer would be unsound: a work can outlive its backend. Python
may still hold the handle returned by an async collective when
destroy_process_group() runs, and ~WorkXCCL returns its events to the
backend's pool, so the destructor would lock a destroyed mutex inside freed
memory. Instead the work borrows nothing: it snapshots the name, rank, size
and device it needs for logging and stream lookup into an XcclCommInfo at
construction, and shares ownership of the event pool, which is hoisted out
of the backend into a shared_ptr so it outlives it. An
owning intrusive_ptr back to the backend was rejected because the backend
reaches the work through its queue, and the resulting cycle would leak a
communicator whenever the user never calls destroy_process_group().
mock the driver in its unit tests; in-tree, at::xpu::XPUEvent wraps the same
sycl::event, so eventRecord/eventQuery/streamWaitEvent become
record()/query()/block(). Because XPUEvent reports errors by throwing rather
than by returning a status, the XPU_ERROR_NOT_READY / XPU_ERROR_UNSUPPORTED
checks become try/catch, matching nccl2's handling of CUDAEvent.
caller-thread enqueueWork() instead of being dropped inline in checkStatus().
This is nccl2's fix, taken deliberately: dropping the last reference to an
at::Tensor from the watchdog thread runs tensor destruction off a thread
c10d does not control.
unique ID off a PrefixStore as "xccl_storekey_". torchcomms
additionally supported building its own TCPStore from MASTER_ADDR/MASTER_PORT
when no Store was supplied; in c10d the Store is always supplied, so that
path -- along with TORCHCOMM_XCCL_BOOTSTRAP_UNIQUEID_EXCHANGE_METHOD, the
internal-store teardown barrier and its XPU barrier buffer -- is unreachable
and is not ported. Same reduction nccl2 made.
populateXcclConfigFromHints keeps torchcomms' targeted warning for hints that
are NCCL-only (trafficClass, CTAPolicy, nvlsCTAs, ...) so that a hint dict
carried over from nccl2 reports which keys oneCCL has no equivalent for. The
strdup() for config.netName is kept as-is: oneCCL's ownership of that pointer
past onecclCommInitRankConfig is undocumented, so this deliberately leaks
rather than risk a dangling pointer.
Typechecked against the real torch_xpu compile flags; all three TUs report
only the build tree's pre-existing stale-TensorBody.h error, same as the
untouched control TUs.