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

Move comm profiling initialization into comm thread #626

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

devreal
Copy link
Contributor

@devreal devreal commented Jan 31, 2024

The comm thread must call remote_dep_mpi_profiling_init to make sure it's thread-local variable is properly set.

Supersedes #622

@devreal devreal requested a review from a team as a code owner January 31, 2024 19:03
Copy link
Contributor

@therault therault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove #ifdef / #endif around pre-declaration

parsec/remote_dep_mpi.c Show resolved Hide resolved
@abouteiller abouteiller added this to the v4.0 milestone Jan 31, 2024
@abouteiller
Copy link
Contributor

the CI errors can be ignored @therault are you satisfied with the changes? If so please merge

Copy link
Contributor

@bosilca bosilca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remote_dep_mpi_profiling_init function does two things: registers the dictionary entries for the communication thread and allocate the first bucket for the profiling stream. As far as I recall the dictionary must be created early in the process, this is not something that one can be part of the lazy initialization.

The correct approach would be to split the current remote_dep_mpi_profiling_init function in two, one for registration (that must be called where remote_dep_mpi_profiling_init is called today), and one doing memory allocation (basically the last 2 lines ofremote_dep_mpi_profiling_init), that should be called from the communication thread.

@devreal
Copy link
Contributor Author

devreal commented Feb 6, 2024

@bosilca please recheck

Signed-off-by: Joseph Schuchart <schuchart@icl.utk.edu>
@bosilca bosilca merged commit b9ad587 into ICLDisco:master Feb 7, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants