Skip to content

[CRL] Deprecate flagcxHandlerGroup, separate device handle/uniqueId/comm lifecycle#480

Merged
MC952-arch merged 2 commits into
flagos-ai:mainfrom
MC952-arch:deprecate-handle-group
Jun 1, 2026
Merged

[CRL] Deprecate flagcxHandlerGroup, separate device handle/uniqueId/comm lifecycle#480
MC952-arch merged 2 commits into
flagos-ai:mainfrom
MC952-arch:deprecate-handle-group

Conversation

@MC952-arch

@MC952-arch MC952-arch commented May 25, 2026

Copy link
Copy Markdown
Collaborator

PR Category

CRL

PR Types

Deprecations

PR Description

This PR deprecates flagcxHandlerGroup and refactors call sites to manage the device handle (flagcxDeviceHandle_t), unique ID (flagcxUniqueId), and communicator (flagcxComm_t) lifecycles separately. It also updates the public API so flagcxGetUniqueId writes into caller-provided storage instead of allocating.

@MC952-arch MC952-arch force-pushed the deprecate-handle-group branch 2 times, most recently from c0d4e88 to 5c29df0 Compare May 26, 2026 06:22
@MC952-arch MC952-arch requested a review from Copilot May 26, 2026 06:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR deprecates flagcxHandlerGroup and refactors call sites to manage the device handle (flagcxDeviceHandle_t), unique ID (flagcxUniqueId), and communicator (flagcxComm_t) lifecycles separately. It also updates the public API so flagcxGetUniqueId writes into caller-provided storage instead of allocating.

Changes:

  • Introduce flagcxDeviceHandleInit/Free and migrate tests/plugins from flagcxHandleInit/Free to the new device-handle lifecycle.
  • Change flagcxGetUniqueId API to require a caller-provided flagcxUniqueId buffer and update MPI broadcast patterns accordingly.
  • Ensure flagcxCommInitRank/Destroy loads/unloads device/CCL plugins as part of communicator lifecycle.

Reviewed changes

Copilot reviewed 54 out of 54 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
test/unittest/topo/test_p2p_topo.cpp Switch unit test setup/teardown from handler group to device handle.
test/unittest/topo/include/flagcx_topo_test.hpp Replace handler group member with devHandle/comm members.
test/unittest/topo/flagcx_topo_test.cpp Update topo fixture to use device handle and stack flagcxUniqueId.
test/unittest/symmem/symmem_test.cpp Migrate SymMem tests to device-handle init/free and stack unique ID.
test/unittest/symmem/include/symmem_test.hpp Replace static handler member with static device handle.
test/unittest/symmem/coll_sym_access.cpp Update device API calls to use devHandle directly.
test/unittest/runner/main_mpi.cpp Migrate MPI runner fixture from handler group to device handle + comm members.
test/unittest/runner/include/runner_fixtures.hpp Replace handler group member with devHandle/comm.
test/unittest/runner/coll_sendrecv.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_scatter.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_reducescatter.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_reduce.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_gather.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_broadcast.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_alltoall.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_allreduce.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/runner/coll_allgather.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/p2p/test_p2p_engine_read.cpp Update p2p engine read test to init/free device handle directly.
test/unittest/main.cpp Update tests to stop referencing handler group and adjust unique-id handling.
test/unittest/kernel/include/flagcx_kernel_test.hpp Replace handler group member with devHandle/comm.
test/unittest/kernel/include/deviceapi_test.hpp Replace static handler with static device handle in device API tests.
test/unittest/kernel/flagcx_kernel_test.cpp Migrate kernel test fixture from handler group to device handle + stack unique ID.
test/unittest/kernel/deviceapi_test.cpp Migrate device API test suite setup to device handle + stack unique ID.
test/unittest/kernel/coll_kernel_p2p.cpp Remove handler-group aliases; rely on fixture members.
test/unittest/kernel/coll_deviceptr_allreduce.cpp Update device memcpy/sync calls to use devHandle.
test/unittest/device/device_adaptor_test.cpp Update device adaptor test to init/free device handle directly.
test/unittest/core/main_mpi.cpp Update core MPI fixture to init/free device handle and manage comm separately.
test/unittest/core/include/core_fixtures.hpp Replace handler group member with devHandle/comm.
test/unittest/coll/include/flagcx_coll_test.hpp Replace handler group member with devHandle/comm.
test/unittest/coll/flagcx_coll_test.cpp Migrate coll test fixture from handler group to device handle + stack unique ID.
test/unittest/adaptor/test_device_adaptor.cpp Update adaptor test to init/free device handle directly.
test/perf/host_api/test_put.cpp Migrate perf test from handler group to device handle + stack unique ID.
test/perf/host_api/test_one_side_register.cpp Migrate perf test from handler group to device handle + stack unique IDs.
test/perf/host_api/test_ipc_sendrecv.cpp Migrate perf test from handler group to device handle.
test/perf/host_api/test_get.cpp Migrate perf test from handler group to device handle + stack unique ID.
test/perf/host_api/test_core_sendrecv.cpp Update hetero unique-id handling to use stack unique ID.
test/perf/device_api/test_intranode_kernel.cpp Migrate perf test to device handle + stack unique ID.
test/perf/device_api/test_internode_kernel.cpp Migrate perf test to device handle + stack unique ID.
test/perf/device_api/test_device_api_allreduce.cpp Migrate perf test to device handle + stack unique ID.
test/perf_common.cc Update perf setup/teardown to use device handle and standalone comm.
test/kernel/test_multi_fifo.cpp Migrate kernel perf tool to device handle + stack unique ID.
test/kernel/test_intranode.cpp Migrate kernel perf tool to device handle + stack unique ID.
test/kernel/test_internode_twosided.cpp Migrate kernel perf tool to device handle + stack unique ID.
test/kernel/test_internode_onesided.cpp Migrate kernel perf tool to device handle + stack unique ID.
test/kernel/test_device_ir.cpp Migrate kernel tool to device handle + stack unique ID.
test/kernel/test_device_api.cpp Migrate kernel tool to device handle + stack unique ID.
test/include/perf_common.h Update PerfContext to store devHandle/comm directly; adjust comment.
test/fixtures/comm_fixture.hpp Migrate common MPI comm fixture to device handle + stack unique ID.
plugin/torch/flagcx/src/backend_flagcx.cpp Migrate torch backend from handler group to device handle + comm members.
plugin/torch/flagcx/include/backend_flagcx.hpp Update torch backend types/members for new lifecycle.
plugin/nccl/src/nccl_wrapper.cc Update NCCL wrapper to store devHandle/comm separately; stack unique ID.
plugin/interservice/flagcx_wrapper.py Update Python wrapper to expose new init/free APIs and new unique-id signature.
flagcx/include/flagcx.h Add device handle init/free APIs; change flagcxGetUniqueId signature; deprecate handler group APIs.
flagcx/flagcx.cc Implement device handle init/free; update handler group init/free; update unique-id and plugin lifecycle behavior.
Comments suppressed due to low confidence (1)

plugin/interservice/flagcx_wrapper.py:228

  • The exported signature for flagcxCommInitRank now expects a POINTER(flagcxUniqueId) (matching the C API). Ensure all Python call sites pass a pointer (ctypes.byref(unique_id)) rather than the struct value, otherwise ctypes will raise an argument type error at runtime (e.g., the FLAGCXLibrary.flagcxCommInitRank wrapper currently passes unique_id directly).
        Function("flagcxGetUniqueId", flagcxResult_t,
                [ctypes.POINTER(flagcxUniqueId)]),
        # Note that flagcxComm_t is a pointer type, so the first argument
        # is a pointer to a pointer
        Function("flagcxCommInitRank", flagcxResult_t, [
            ctypes.POINTER(flagcxComm_t), ctypes.c_int, ctypes.POINTER(flagcxUniqueId),
            ctypes.c_int
        ]),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flagcx/flagcx.cc
Comment thread flagcx/flagcx.cc Outdated
Comment thread flagcx/flagcx.cc
Comment thread flagcx/flagcx.cc
Comment thread test/unittest/topo/flagcx_topo_test.cpp
Comment thread test/unittest/topo/include/flagcx_topo_test.hpp Outdated
Comment thread test/unittest/core/main_mpi.cpp
Comment thread test/unittest/main.cpp

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 54 out of 54 changed files in this pull request and generated 2 comments.

Comment thread test/unittest/topo/include/flagcx_topo_test.hpp
Comment thread test/unittest/topo/flagcx_topo_test.cpp
@MC952-arch MC952-arch force-pushed the deprecate-handle-group branch from 94128e4 to 4ead4f5 Compare May 28, 2026 07:17

@mikethegoblin mikethegoblin 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

@heavyrain-lzy heavyrain-lzy 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

@MC952-arch MC952-arch force-pushed the deprecate-handle-group branch from 4ead4f5 to 098309a Compare June 1, 2026 09:39
@MC952-arch MC952-arch force-pushed the deprecate-handle-group branch from 098309a to c9f04fb Compare June 1, 2026 09:46
@MC952-arch MC952-arch merged commit 5cc6c42 into flagos-ai:main Jun 1, 2026
9 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.

4 participants