Add Support for Dynamic Symmetric Memory #909
Conversation
|
Can one of the admins verify this patch? |
wfaderhold21
force-pushed
the
topic/mem_ext
branch
from
0451a7d
to
5c25b9c
Compare
wfaderhold21
force-pushed
the
topic/mem_ext
branch
from
b7610cc
to
3db29e9
Compare
wfaderhold21
requested review from
samnordmann,
Sergei-Lebedev,
manjugv and
janjust
|
@wfaderhold21 Is this ready for review? @janjust Does this work for you? |
|
@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. |
|
@manjugv yes. It’s ready for review. |
wfaderhold21
force-pushed
the
topic/mem_ext
branch
from
ef33001
to
fef2a37
Compare
|
@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. |
|
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. |
|
@Sergei-Lebedev @janjust was there any more feedback on this PR? |
|
@wfaderhold21 I don't think so - I think we're just really backlogged with other PRs and reviews - sorry for the slow progress but it's on my radar |
|
@samnordmann @nsarka I think I made the corrections you all asked, but I also updated some code to bugs I found. Could you all take another look to make sure everything is OK there? |
|
@Sergei-Lebedev @wfaderhold21 Hey guys, sorry to harp on this, but are we done with this PR? Is it ready to be merged, @Sergei-Lebedev any other input needed? |
|
@janjust AFIAK nothing to be added on my end. |
|
@Sergei-Lebedev @manjugv are there any outstanding comments? Can we push this forward? |
+1 from my side. |
|
@Sergei-Lebedev @janjust @manjugv The requested changes were made to remove all API changes. I believe this PR now conforms to what we have discussed. |
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.