Repository navigation
TOOLS/PERF: Honor configured device id in ZE and ROCm allocators #11993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -337,6 +337,37 @@ ucx_perf_mem_alloc_name(const ucx_perf_params_t *params, int is_send) | |
| return ucs_memory_type_names[mem_type]; | ||
| } | ||
|
|
||
| /* | ||
| * Select the GPU to be used by this process: the device id given by the | ||
| * '-a <type:dev-id>' option if it was specified, otherwise spread the | ||
| * processes over the available devices according to their group index. | ||
| */ | ||
| static inline ucs_status_t | ||
| ucx_perf_get_gpu_index(const ucx_perf_context_t *perf, unsigned group_index, | ||
| int num_gpus, const char *dev_type, int *gpu_index_p) | ||
| { | ||
| int gpu_index; | ||
|
|
||
| if (num_gpus <= 0) { | ||
| ucs_error("no %s devices available", dev_type); | ||
| return UCS_ERR_NO_DEVICE; | ||
| } | ||
|
|
||
| /* group index 0 is the receiver side */ | ||
| gpu_index = (group_index == 0) ? perf->params.recv_device.device_id : | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor: should we also check that the |
||
| perf->params.send_device.device_id; | ||
| if (gpu_index == UCX_PERF_MEM_DEV_DEFAULT) { | ||
| gpu_index = group_index % num_gpus; | ||
| } else if (gpu_index >= num_gpus) { | ||
| ucs_error("illegal %s device %d, number of devices %d", dev_type, | ||
| gpu_index, num_gpus); | ||
| return UCS_ERR_NO_DEVICE; | ||
| } | ||
|
|
||
| *gpu_index_p = gpu_index; | ||
| return UCS_OK; | ||
| } | ||
|
|
||
| static UCS_F_ALWAYS_INLINE int ucx_perf_context_done(ucx_perf_context_t *perf) | ||
| { | ||
| return ucs_unlikely((perf->current.iters >= perf->max_iter) || | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-acurrently also switches the run into device api mode: it addsUCP_FEATURE_DEVICE(libperf.c:958), movesthread_countintodevice_thread_count(perftest_params.c:866), and routes ucp dispatch throughucx_perf_mem_type_device_dispatchers[](ucp_tests.cc:1209), which is registered for cuda only. So using-ato pick a ze/rocm device also pulls in device mode semantics that those allocators do not support.Can we add a separate knob to enable device mode, and keep
-afor device type and id only, as its help text already says? Then cuda could also select a device in a regular run, which is not possible today.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed,
-acurrently mixes device selection with device-API mode, and this is pre-existing: on master,ucx_perftest -t tag_bw -m cuda -a cuda:1also fails withInvalid test case, so CUDA can't select a device in a regular UCP run either. Adding a separate knob changes CLI semantics that CI relies on (test_jenkins.shuses-a cuda:0to enable device mode), so I'd like to keep this PR limited to honoring the id in the allocators and do the knob in a follow-up PR. With this PR alone, the ZE/ROCm device id takes effect for UCT tests.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-atoday is only usable for the device feature: it impliesUCP_FEATURE_DEVICE(libperf.c:958), movesthread_countintodevice_thread_count(perftest_params.c:866) and routes ucp dispatch throughucx_perf_mem_type_device_dispatchers[](ucp_tests.cc:1209), which is registered for cuda only. So it would make ze/rocm honor a device id only in a mode those allocators do not support, and no allocator supports placement outside that mode today.Can we instead move the device id to
-m, with the same<type>:<dev-id>syntax, and keep-awithout argument to only enable the device feature?-malready selects the allocator that places the buffers, and it avoids the strange-m ze-device -a ze-device:0.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually started #12003 that should cover this PR, if you could confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, #12003 subsumes this PR: the shared device-index helper, ZE/ROCm honoring an explicit id, the range checks and the ROCm zero-device guard, and it also separates device mode from device selection, which answers the open
-athreads and the type-mismatch question here. Since it targets master independently and moves the id from-a <type:id>to-m <allocator:id>, I'll treat it as a replacement and close this PR once #12003 is merged. I left a few notes on #12003; the main one is preserving the GPU-0 pinning in the CUDA device CI job.