Skip to content

ompi/datatype: make datatype pack thread safe #1316

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

Merged
merged 1 commit into from
Jan 22, 2016

Conversation

hjelmn
Copy link
Member

@hjelmn hjelmn commented Jan 21, 2016

This commit makes ompi_datatype_get_pack_description thread safe. The
call is used by osc/pt2pt to send the packed description to remote
peers. Before this commit if MPI_THREAD_MULTIPLE is enabled and the
user uses MPI_Put, MPI_Get, etc we could hit a race where multiple
threads attempt to store the packed description on the datatype. Since
the code in question is not performance-critical the threading fix
uses opal_atomic_* calls instead of bothering with OPAL_THREAD_*.

Signed-off-by: Nathan Hjelm [email protected]

@hjelmn
Copy link
Member Author

hjelmn commented Jan 21, 2016

@bosilca Please take a look. This is targeted for 2.0.0.

@bosilca
Copy link
Member

bosilca commented Jan 21, 2016

Make sense, the patch is correct but sub-optimal (the packed description will be generated by all threads and then most of them will drop it).

@hjelmn
Copy link
Member Author

hjelmn commented Jan 21, 2016

@bosilca Was thinking that would be ok in this case. Using atomics avoids adding a lock to the datatype structure. I will see if I can optimize the atomics version a bit more to avoid the the duplicate description generation.

@hjelmn
Copy link
Member Author

hjelmn commented Jan 21, 2016

@regrant This is one found by the rma threading benchmarks.

@hjelmn hjelmn force-pushed the datatype_pack_threads branch from 76c9c49 to 38a90ba Compare January 21, 2016 22:47
@hjelmn
Copy link
Member Author

hjelmn commented Jan 21, 2016

@bosilca Ok, optimized it a bit better. Now only one thread will create the datatype description.

@bosilca
Copy link
Member

bosilca commented Jan 21, 2016

@hjelmn this looks good. Thanks for the optimization. 👍

@lanl-ompi
Copy link
Contributor

Test FAILed.

This commit makes ompi_datatype_get_pack_description thread safe. The
call is used by osc/pt2pt to send the packed description to remote
peers. Before this commit if MPI_THREAD_MULTIPLE is enabled and the
user uses MPI_Put, MPI_Get, etc we could hit a race where multiple
threads attempt to store the packed description on the datatype. Since
the code in question is not performance-critical the threading fix
uses opal_atomic_* calls instead of bothering with OPAL_THREAD_*.

Signed-off-by: Nathan Hjelm <[email protected]>
@hjelmn hjelmn force-pushed the datatype_pack_threads branch from 38a90ba to b921831 Compare January 22, 2016 00:53
hjelmn added a commit that referenced this pull request Jan 22, 2016
ompi/datatype: make datatype pack thread safe
@hjelmn hjelmn merged commit 243d973 into open-mpi:master Jan 22, 2016
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.

3 participants