Skip to content

Add opt-in RMM allocation for WholeMemory - #505

Open
alexbarghi-nv wants to merge 11 commits into
mainfrom
agent/enable-rmm-wholememory-allocation
Open

Add opt-in RMM allocation for WholeMemory#505
alexbarghi-nv wants to merge 11 commits into
mainfrom
agent/enable-rmm-wholememory-allocation

Conversation

@alexbarghi-nv

@alexbarghi-nv alexbarghi-nv commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

  • add C API controls and pylibwholegraph.torch.set_memory_resource() / is_rmm_enabled() to opt supported WholeMemory allocations into RMM
  • allocate distributed and hierarchy device storage with CCCL's cuda::device_buffer<cuda::std::byte>, explicitly backed by RMM's current per-device resource
  • preserve byte-based sizing, uninitialized storage, CUDA allocation alignment, and legacy default-stream behavior
  • retain the allocation-time resource reference for matching deallocation; the Python API also retains supplied memory-resource objects for allocation lifetime safety
  • keep chunked, continuous, and NVSHMEM device storage on their specialized allocation paths and emit a rank-0 fallback warning when RMM is enabled
  • leave host allocations and the default disabled path unchanged, require matching enablement across ranks, and reset RMM enablement during successful WholeMemory finalization
  • add RMM runtime dependencies to conda and Python package metadata, plus Python usage documentation

Test coverage

  • verify the default/disabled path does not allocate through RMM
  • verify distributed and hierarchy device allocations use the configured RMM resource and release all tracked bytes
  • verify an allocation is deallocated through its original resource after RMM's current resource is changed
  • verify chunked and continuous device allocations, plus distributed host allocations, remain outside RMM
  • verify finalization resets RMM enablement
  • verify the Python API installs and retains a valid resource, supports disabling with None, rejects invalid resource types, and reports finalization state correctly

Local testing

Tested locally in the rapids conda environment with CUDA 12.9, GCC 14, RMM 26.10, CCCL 3.4, and NVIDIA V100 GPUs:

  • fresh CMake configure and CUDA build of WHOLEMEMORY_HANDLE_TEST (passed)
  • complete WHOLEMEMORY_HANDLE_TEST on two peer-connected V100s: 20 tests passed
  • focused WholeMemoryHandleRMMTests.* two-GPU run (passed)
  • new Python memory-resource API tests: 3 tests passed
  • existing Python WholeMemory binding regression: 1 test passed
  • repository pre-commit hooks for all changed files, including clang-format, Ruff, RAPIDS dependency generation, CMake formatting/linting, copyright checks, and alpha-spec validation (passed)
  • git diff --check (passed)

@copy-pr-bot

copy-pr-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@alexbarghi-nv

Copy link
Copy Markdown
Member Author

/ok to test 64ea3f3

@alexbarghi-nv alexbarghi-nv self-assigned this Jul 23, 2026
@alexbarghi-nv alexbarghi-nv added breaking Introduces a breaking change feature request New feature or request labels Jul 23, 2026

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally looks fine, but I have a comment about how this code could be a little more future-proof.

Comment thread cpp/src/wholememory/memory_handle.cpp Outdated
@alexbarghi-nv
alexbarghi-nv force-pushed the agent/enable-rmm-wholememory-allocation branch from 83c09bc to 23bb681 Compare July 27, 2026 18:34
@alexbarghi-nv
alexbarghi-nv marked this pull request as ready for review July 27, 2026 18:35
@alexbarghi-nv
alexbarghi-nv requested review from a team as code owners July 27, 2026 18:35
@alexbarghi-nv
alexbarghi-nv requested a review from jameslamb July 27, 2026 18:35
@alexbarghi-nv

Copy link
Copy Markdown
Member Author

/ok to test 23bb681

@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds opt-in RMM allocation support for WholeMemory.

  • Routes supported distributed and hierarchy device allocations through the current per-device RMM resource.
  • Adds C and Python controls, fallback behavior, lifecycle tests, dependencies, and usage documentation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
cpp/src/wholememory/memory_handle.cpp Adds process-wide RMM enablement, RMM-backed device buffers, allocation-path consistency checks, and specialized-path fallbacks.
cpp/src/wholememory/wholememory.cpp Exposes RMM enablement through the C API and resets it after successful finalization.
python/pylibwholegraph/pylibwholegraph/torch/initialize.py Exposes boolean RMM controls without retaining obsolete Python memory-resource objects.
cpp/tests/wholememory/wholememory_handle_tests.cpp Tests opt-in allocation routing, allocation-time resource selection, deallocation accounting, fallback paths, and finalization reset.

Reviews (8): Last reviewed commit: "Merge branch 'main' into agent/enable-rm..." | Re-trigger Greptile

Comment thread python/pylibwholegraph/pylibwholegraph/torch/initialize.py Outdated
@alexbarghi-nv
alexbarghi-nv marked this pull request as draft July 27, 2026 19:26
@alexbarghi-nv
alexbarghi-nv marked this pull request as ready for review July 27, 2026 19:33
@alexbarghi-nv

Copy link
Copy Markdown
Member Author

/ok to test da656ce

Comment thread python/pylibwholegraph/pylibwholegraph/torch/initialize.py
@alexbarghi-nv

Copy link
Copy Markdown
Member Author

/ok to test 0004b48

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One CMake issue needs to be fixed before we can merge.

Comment thread cpp/CMakeLists.txt Outdated

include(./cmake/thirdparty/get_raft.cmake)
include(./cmake/thirdparty/get_nccl.cmake)
find_package(rmm "${RAPIDS_VERSION}" REQUIRED)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please copy https://github.com/rapidsai/cudf/blob/main/cpp/cmake/thirdparty/get_rmm.cmake, don't use plain find_package here. This must go before get_raft.cmake so that RMM is found by wholegraph (potentially with custom branches/etc.) before RAFT attempts to find RMM.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done; can you re-review @bdice ?

Comment thread python/pylibwholegraph/README.md
@alexbarghi-nv

Copy link
Copy Markdown
Member Author

/ok to test 217ff8a

@linhu-nv linhu-nv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM thanks!

Comment thread cpp/src/wholememory/memory_handle.cpp
@alexbarghi-nv

Copy link
Copy Markdown
Member Author

/ok to test 5c9ee13

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Thank you.

@alexbarghi-nv

Copy link
Copy Markdown
Member Author

/ok to test 08f3ba7

@jameslamb
jameslamb removed their request for review August 7, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Introduces a breaking change feature request New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants