Skip to content

UCT/ROCM: adjust rocm/ipc performance parameters - #9568

Merged
yosefe merged 1 commit into
openucx:masterfrom
edgargabriel:topic/rocm-ipc-v2-tuning
Jan 3, 2024
Merged

yosefe merged 1 commit into
openucx:masterfrom
edgargabriel:topic/rocm-ipc-v2-tuning

Conversation

@edgargabriel

Copy link
Copy Markdown
Contributor

What

introduce two new parameters in rocm/ipc that allow to fine tune the protocol selection.

Why ?

Help to resolve the issue seen with v2 protocols for short message intra-node collective operations.

@edgargabriel
edgargabriel force-pushed the topic/rocm-ipc-v2-tuning branch 2 times, most recently from 360b138 to 8e1f267 Compare December 28, 2023 20:56
@edgargabriel edgargabriel added the WIP-DNM Work in progress / Do not review label Dec 29, 2023
@edgargabriel

Copy link
Copy Markdown
Contributor Author

setting the DoNotMerge flag since I might have a better solution than the min_zcopy (which did most of the heavy lifting in this fix)

@edgargabriel
edgargabriel force-pushed the topic/rocm-ipc-v2-tuning branch from 8e1f267 to e1e7493 Compare January 1, 2024 15:10
@edgargabriel edgargabriel removed the WIP-DNM Work in progress / Do not review label Jan 1, 2024
@edgargabriel

Copy link
Copy Markdown
Contributor Author

Lets stay for now with this solution here to deal with the intra-node collectives performance issue that I'm looking into.

An alternative approach that I explored was to set the pre_send_overhead to a non-zero value in rocm_ipc, that gave me similar performance improvements as the solution in this pr. However, I started to hit issue #9372 with the pre_send_overhead in a number of tests. Until we find a solution to the problem in that issues, this solution here (which mostly relies on setting a minimum message length for the rocm/ipc/zcopy operations) seems to be more stable.

This pr will also have to go to the 1.16 branch.

yosefe
yosefe previously approved these changes Jan 2, 2024
@yosefe

yosefe commented Jan 2, 2024

Copy link
Copy Markdown
Member

introduce two new parameters for rocm/ipc that allow to fine tune
the protocol selection. Specifically, they help to resolve the issue
seen with v2 protocols for short message intra-node collective
operations.
@edgargabriel
edgargabriel force-pushed the topic/rocm-ipc-v2-tuning branch from 6ef3cfa to 1e91ddb Compare January 2, 2024 14:41
@edgargabriel

Copy link
Copy Markdown
Contributor Author

@yosefe thank you, I think I fixed the issues

@yosefe
yosefe enabled auto-merge January 3, 2024 07:59
@yosefe
yosefe merged commit f93e4d5 into openucx:master Jan 3, 2024
ovidiusm pushed a commit to ai-dynamo/nixl that referenced this pull request Oct 12, 2026
## What?
Set `ROCM_IPC_MIN_ZCOPY=0` in the UCX context NIXL creates.

## Why?
`rocm_ipc` defaults to a 128 byte minimum zero-copy size
(openucx/ucx#9568, for small-message
MPI collectives). UCP put/get zero-copy protocols reject any lane with a
non-zero minimum, so
NIXL VRAM to VRAM transfers on AMD never use `rocm_ipc` (XGMI) and are
staged through host
memory instead. `cuda_ipc` already uses 0.

Setting it only in NIXL's UCX context leaves MPI and other UCX users
unchanged. A user-set
`UCX_ROCM_IPC_MIN_ZCOPY` still wins. On UCX builds without ROCm support
the setting is
unknown and is ignored with a debug log.

With NIXL's default peer error handling, `rocm_ipc` is selected only
once it advertises
peer-failure support (openucx/ucx#11977, open). This PR does not depend
on it, but the full
benefit needs both.

## Results
MI355X, NIXL WRITE GPU0 to GPU4, UCX master + openucx/ucx#11977:

| size | without this PR | with this PR |
|---|---|---|
| 64 KiB | 1.68 GB/s | 4.54 GB/s |
| 512 KiB | 11.89 GB/s | 23.74 GB/s |
| 2 MiB | 30.05 GB/s | 43.69 GB/s |
| 128 MiB | 60.22 GB/s | 60.85 GB/s |

UCX protocol selection (`UCX_PROTO_INFO=y`) changes from a rendezvous
read to direct
`rocm_ipc` zero-copy for all sizes.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Improvements**
  * Updated ROCm IPC zero-copy settings during UCX initialization.
* Added a warning when AMD GPUs are detected but UCX lacks ROCm
memory-type support, helping identify a configuration that may affect
GPU communication.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Colin Hirsch <chirsch@nvidia.com>
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