-
Notifications
You must be signed in to change notification settings - Fork 286
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
docs: add documentation for threading tuning parameters #5917
Conversation
6c360c3
to
d39a3ac
Compare
👍 Looks good. Ping me when it is ready to review. |
d39a3ac
to
8c02136
Compare
doc/mpich/TUNING_PARAMETERS
Outdated
|
||
MPICH also supports a non-standard per-communicator info hint that provides the user with more control of a communicator-VCI mapping: | ||
* `vci`: Specific sender and receiver VCI can be selected for each communicator using this hint. Applications can utilize this feature to control how messages are spread across the VCIs. | ||
For instance, programmers can write the applications so that pairs of threads communicate by using different communicator, where each communicator can be mapped to a different VCI through the `vci` info hint. Valid values for this info hints are 0 to (total vcis - 1). This hint is also utilized by MPICH to select a progress thread to process reduction collective operation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you wrap the text to width of less than 80 chars?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
ba3fadf
to
f20999e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good explanations. However, it is immediately clear for a user how to configure/tune. List configure options and cvars in separate code snippet block should help.
doc/mpich/TUNING_PARAMETERS
Outdated
the MPI runtime, the runtime will serialize the calls. Another strategy uses a | ||
fine-grained lock to protect the access to each network context or VCI. With | ||
this approach, threads trying to access the same VCI get serialized, but threads | ||
accessing different VCIs can execute in parallel. With the fine-grained locking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need explain how to configure/select between "global" and "find-grained".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
doc/mpich/TUNING_PARAMETERS
Outdated
model, MPICH supports a `direct` mode where the MPI runtime itself grabs a per | ||
VCI lock and a `lockless` mode where MPI does not grab a lock and relies on the | ||
network provider to provide fine grain synchronization (MPICH uses the | ||
FI_THREAD_SAFE mode from OFI to support the `lockless` mode). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document the configure option and runtime cvars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
This document will discuss various tuning parameters in MPICH.
f20999e
to
38a129b
Compare
@hzhou I've added a fixup patch with your requested changes. Could you check if it looks fine? I'll squash it after the review. |
doc/mpich/TUNING_PARAMETERS
Outdated
A threading synchronization strategy can be selected at the configuration time | ||
as: | ||
--enable-thread-cs=global to select coarse-grained locking, and | ||
--enable-thread-cs=per-vci to select fine-grained locking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you break the line so the command snippet is on its own line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was debating with myself about it. Sure, let me do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done + squashed
38a129b
to
0241d87
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice you are using MARKDOWN formating throughout. How about rename this document into tuning_parameters.md
and then fix the part that doesn't look right (like headings)?
I am approving it as I am OK to merge it as is. It's up to you to make additional changes. Just let me know when you are happy for it to merge.
doc/mpich/TUNING_PARAMETERS
Outdated
https://wiki.mpich.org/mpich/index.php/Using_the_Hydra_Process_Manager#Process-core_Binding. | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra empty lines.
doc/mpich/TUNING_PARAMETERS
Outdated
|
||
**1. Multi-threading** | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra empty line
These are the currently supported tuning parameters for MPI_THREAD_MULTIPLE.
0241d87
to
58f272b
Compare
Fixed the space issues. Also converted the doc file to |
8bd5e78
to
6ddcd21
Compare
How about add a title? |
Is the how we add title to a MARKDOWN file? : |
Title is just bigger heading. This is a titleSection title |
This is now formatted as MARKDOWN (.md) file. This patch also fixes some of the formatting issues that appeared during the format change.
6ddcd21
to
3d9ed35
Compare
Updated |
Pull Request Description
This PR adds documentation for tuning parameters related to multi-threading (MPI_THREAD_MULTIPLE).
Author Checklist
Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
Commits are self-contained and do not do two things at once.
Commit message is of the form:
module: short description
Commit message explains what's in the commit.
Whitespace checker. Warnings test. Additional tests via comments.
For non-Argonne authors, check contribution agreement.
If necessary, request an explicit comment from your companies PR approval manager.