Add MPI3 shared memory resource #927
Conversation
| MemoryResourceTraits traits; | ||
|
|
||
| traits.unified = false; | ||
| traits.size = 0; |
|
Can you provide an example and documentation? You should be able to easily generate something from the IPC shared memory example and docs. |
| { | ||
| void* ptr; | ||
| MPI_Win win; | ||
| MPI_Aint size = (m_local_rank != 0) ? 0 : bytes; |
There was a problem hiding this comment.
Just to avoid a double negative, I think this would be easier understood as
MPI_Aint size = (m_local_rank == 0) ? bytes : 0;
|
How is synchronization handled? i.e. after allocating some shared memory, say I want to fill it in on rank 0 and then make it available to all other ranks? |
| int disp{sizeof(char)}; | ||
|
|
||
| MPI_Win_allocate_shared(size, disp, MPI_INFO_NULL, m_shared_comm, &ptr, &win); | ||
|
|
There was a problem hiding this comment.
Isn't MPI_Win_shared_query needed if m_local_rank != 0?
I'm looking at "Step 3.12 Neighbor access through MPI_WIN_SHARED_QUERY" in https://fs.hlrs.de/projects/par/mooc/mooc-2/mooc2-week3-4.pdf#%5B%7B%22num%22%3A85%2C%22gen%22%3A0%7D%2C%7B%22name%22%3A%22XYZ%22%7D%2C68%2C148%2C0%5D
There was a problem hiding this comment.
That's for "non-contiguous" shared memory.
There was a problem hiding this comment.
So the following statement that website made is incorrect? Pardon my lack of knowledge about MPI shared memory.
"You need this method for non-contiguous shared memory, but also for all processes
with a zero byte window portion, because if a process specifies in MPI_Win_allocate_shared a zero byte window length, then no base pointer is returned."
There was a problem hiding this comment.
Or this statement from the manpage for MPI_Win_allocate_shared?
"Contiguous across process ranks means that the first address in the memory segment of process i is consecutive with the last address in the memory segment of process i - 1. This may enable the user to calculate remote address offsets with local information only."
There was a problem hiding this comment.
I think you are correct. The documentation is difficult to follow.
There was a problem hiding this comment.
I wasn't expecting so many review comments yet - PR still needs tests written :)
There was a problem hiding this comment.
Haha I've been wanting this feature for a while :)
|
|
||
| void HostMpi3SharedMemoryResource::deallocate(void* ptr, std::size_t) | ||
| { | ||
| auto window = m_shared_windows.find(ptr); |
There was a problem hiding this comment.
I don't see anywhere that pointers are inserted into m_shared_windows.
| : MemoryResource{name, id, traits} | ||
| { | ||
| constexpr int IGNORE_KEY{0}; | ||
| MPI_Comm_split_type(MPI_COMM_WORLD, MPI_COMM_TYPE_SHARED, IGNORE_KEY, MPI_INFO_NULL, &m_shared_comm); |
There was a problem hiding this comment.
Could traits be extended to contain the communicator to split on?
Using a communicator. I am going to extend the get_communicator_for_allocator method to handle this case. |
|
@adayton1 did you want to test drive this PR somewhere? |
I have a use case for it, but I'm not sure if I have time to implement this week. |
| @@ -157,7 +157,7 @@ if (UMPIRE_ENABLE_IPC_SHARED_MEMORY) | |||
| umpire INTERFACE Threads::Threads -lrt | |||
| ) | |||
| else() | |||
| message(STATUS "Host Shared Memory Disabled") | |||
| message(STATUS "IPC Shared Memory Disabled") | |||
There was a problem hiding this comment.
Also change the message on line 154?
| { | ||
| if (p == Platform::host) | ||
| return true; | ||
| else // TODO: check this |
No description provided.