-
Notifications
You must be signed in to change notification settings - Fork 937
opal: clean up init/finalize #6136
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
Conversation
|
This came out of the sessions work. Since we will be bringing up parts of ompi piecemeal we need to keep track of what need to be finalized and in what order. To handle this each init function is expected to register its finalize function. @bosilca Any objections? The only downside is that the datatype tests now have to call |
|
@jsquyres A much larger PR will be headed for ompi. I need to rewrite that since that commit is intertwined with the sessions work. |
|
The IBM CI (GNU Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/1a9c5bbe409d07204aca7abaa7ec7c8b |
|
The IBM CI (XL Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/79ce1fd1142ef82c29e544c1af4061f5 |
|
Opps. Looks like I didn't update ompi_datatype for the changes. Will fix that tomorrow. For now this is for comments anyway. |
|
The IBM CI (PGI Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/53a512f69a4dbc4035211ab369396690 |
bosilca
left a comment
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 think this is a good move, providing a simpler and much cleaner tear down solution. I would propose to add a second parameter to opal_finalize_register_cleanup, a void* that is then passed to the cleanup function when called. This will allow us to add not only single global frameworks to the finalization, but MCA modules as well.
0ccbc02 to
6d55739
Compare
|
@bosilca Take a look now. I added additional functions to opal_framework.h and automated the teardown of frameworks. Now |
|
@jsquyres This will become the underpinning of the ompi_mpi_init/ompi_mpi_finalize cleanup. I generalized it so that we can have multiple finalize domains. |
| /* As the synonyms are just copies of the internal data we should not free them. | ||
| * Anyway they are over the limit of OPAL_DATATYPE_MAX_PREDEFINED so they will never get freed. | ||
| */ | ||
|
|
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.
Let’s invert these 2 so that the destruction of the master convertors could use the dedicated opal_datatype_dfd.
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.
Ok, makes sense. Will make that change.
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.
Fixed.
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.
👍
|
The IBM CI (GNU Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/ea8b18f8e7fc6c34782bd59ad6386737 |
|
The IBM CI (XL Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/ba1fc09a3acc92a21daea07ca92aa0f8 |
|
The IBM CI (PGI Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/0c6db4d4102aa47c478df533dd508377 |
|
The IBM CI (XL Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/949a76a8a82edeb39d22e0cf1aa02c88 |
|
@jsquyres Any comments? |
|
The IBM CI (XL Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/f73b2265afb7b494902c89e84f6de2f3 |
|
The IBM CI (GNU Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/56a7a2219bf9069a5ca08b51c510e442 |
This commit contains the following changes: - Remove the unused opal_test_init/opal_test_finalize functions. These functions are not used by anything in the code base or MTT. Tests use opal_init_util/opal_finalize_util instead. - Get rid of gotos in opal_init_util and opal_init. Replaced them with a cleaner solution. - Automatically register cleanup functions in init functions. The cleanup functions are executed in the reverse order of the initialization functions. The cleanup functions are run in opal_finalize_util() before tearing down the class system. Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
This commit contains the following changes:
Remove the unused opal_test_init/opal_test_finalize
functions. These functions are not used by anything in the code
base or MTT. Tests use opal_init_util/opal_finalize_util instead.
Get rid of gotos in opal_init_util and opal_init. Replaced them
with a cleaner solution.
Automatically register cleanup functions in init functions. The
cleanup functions are executed in the reverse order of the
initialization functions. The cleanup functions are run in
opal_finalize_util() before tearing down the class system.
Signed-off-by: Nathan Hjelm hjelmn@lanl.gov