TL/UCP: avoid copy in knomial scatter #771
Merged
Conversation
This file contains 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
| @@ -62,13 +63,7 @@ void ucc_tl_ucp_scatter_knomial_progress(ucc_coll_task_t *coll_task) | |||
| while (!ucc_knomial_pattern_loop_done(p)) { | |||
| step_radix = ucc_kn_compute_step_radix(p); | |||
| block_count = ucc_sra_kn_compute_block_count(count, rank, p); | |||
| sbuf = (rank == root) | |||
| ? args->src.info.buffer : args->dst.info.buffer; | |||
| rbuf = args->dst.info.buffer; | |||
There was a problem hiding this comment.
where are you setting rbuf instead?
There was a problem hiding this comment.
can you elaborate pls, i don't understand the question
There was a problem hiding this comment.
nevermind, i see it is already set in the beginning of progress and this is redundant.
| task->scatter_kn.recv_offset = 0; | ||
| is_zcopy = UCC_TL_UCP_TEAM_LIB(team)->cfg.scatter_kn_enable_recv_zcopy; | ||
| if (((is_zcopy == UCC_CONFIG_AUTO) && | ||
| (args->src.info.mem_type != UCC_MEMORY_TYPE_HOST)) || |
There was a problem hiding this comment.
this tricky, depending on CPU vendor I see different results. For AMD it's better to have extra copy and for Intel it's better to receive with correct offset. For GPUs it's always better to avoid copy
shimmybalsam
approved these changes
May 24, 2023
vspetrov
approved these changes
May 26, 2023
manjugv
approved these changes
May 31, 2023
Sergei-Lebedev
force-pushed
the
topic/scatter_no_copy
branch
from
2f26eaf to
eae1a2e
Compare
janjust
pushed a commit
to janjust/ucc
that referenced
this pull request
Jan 31, 2024
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.
What
Adding configuration parameter for knomial scatter algorithm (part of scatter allgather broadcast) to receive data directly into destination buffer and avoid memory copy as a last step of the algorithm
Why ?
Improves performance of large message broadcast. By default enabled only for GPU collective