Skip to content
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

Add Support for Dynamic Symmetric Memory #909

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

wfaderhold21
Copy link
Collaborator

What

Currently, UCC has support for symmetric memory segments that are added during context creation for use with one-sided collectives. This PR adds support for memory segments to be added after a context and team have been created to better support programming models such as MPI. In addition, this adds support for attaching a key with the memory region for use with DPUs and offloading of collectives.

Why ?

With the current approach, a user will need to perform a memory copy of source and destination buffers to a known and already mapped memory region before a collective can begin using one-sided or offloaded collectives. This removes the need for the memory copy at the expense of memory exchanges on the first collective. This is useful if multiple collectives occur with the same buffers.

@swx-jenkins3
Copy link

Can one of the admins verify this patch?

@manjugv
Copy link
Contributor

manjugv commented Mar 6, 2024

@wfaderhold21 Is this ready for review? @janjust Does this work for you?

@janjust
Copy link
Collaborator

janjust commented Mar 6, 2024

@manjugv In general this works, but I'm still a little concerned if keeping mapped data until tl destruction is the right approach, or if we should enable symmetric buffer unmapping.

@wfaderhold21
Copy link
Collaborator Author

@manjugv yes. It’s ready for review.

@janjust
Copy link
Collaborator

janjust commented May 29, 2024

@wfaderhold21 Did we address the possibility of adding an imported key? What about the ability to important keys with an API? I think we did a follow up study and concluded that parallelization does not improve perf much, and requires a unique memory region to handle key imports otherwise we run into segfaults.

@janjust
Copy link
Collaborator

janjust commented May 29, 2024

OK - after WG meeting I think I realize that my point is moot. I think this is good as is, we just need to put a protection for when keys are imported by multiple threads.

@wfaderhold21
Copy link
Collaborator Author

@Sergei-Lebedev @janjust was there any more feedback on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants