Skip to content

Conversation

@hjelmn
Copy link
Member

@hjelmn hjelmn commented Apr 14, 2015

found at:

http://www.open-mpi.org/community/lists/devel/2014/07/15140.php

The finalize function is set using either the linker option -fini or
__attribute__((destructor)) depending on compiler support. I have
confirmed that this hybrid approach works with all the major
compilers. The attribute is supported by gcc, clang, llvm, xlc, and
icc. The fini function will support pgi. If a compiler/linker
combination does not support either the destructor or fini function a
message will be printed on re-init indicating it is not supported (an
improvement over the current behavior-- SEGV).

I moved the following to the destructor function:

  • Class system finalize. This solves a bug when MPI_T_finalize is
    called before MPI_Init. The only downside to this change is we will
    leave the footprint of the opal class system after
    MPI_Finalize. This footprint should be relatively small.

This is an alternative to #517 but the two PRs are not
mutually-exclusive (with some modifications). This commit should also
be safe for 1.8.x as it does not change internal or external ABI (#517
changes internal ABI).

Signed-off-by: Nathan Hjelm hjelmn@lanl.gov

@hjelmn hjelmn changed the title This commit is related to an RFC from June 2014. Disscussion can be Add opal destructor/fini function Apr 14, 2015
@hjelmn
Copy link
Member Author

hjelmn commented Apr 14, 2015

This is independent of #516 so was pulled into a separate PR.

@hjelmn
Copy link
Member Author

hjelmn commented Apr 14, 2015

@jsquyres, Any comments? The only memory that is kept around after opal_finalize() are the constructor/destructor tables. @rhc54 This is a small amount of memory, do you think this would be ok for 1.8? It fixes a long standing issue with calling MPI_Init after MPI_T_finalize.

@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/435/
Test PASSed.

@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/438/
Test PASSed.

@hjelmn hjelmn mentioned this pull request Apr 14, 2015
@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/439/
Test PASSed.

Copy link
Member

Choose a reason for hiding this comment

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

spelling mistake? compilere - probably wanted compiler

@hppritcha
Copy link
Member

👍
except for minor spelling mistake

This commit is related to an RFC from June 2014. Disscussion can be
found at:

http://www.open-mpi.org/community/lists/devel/2014/07/15140.php

The finalize function is set using either the linker option -fini or
__attribute__((destructor)) depending on compiler support. I have
confirmed that this hybrid approach works with all the major
compilers. The attribute is supported by gcc, clang, llvm, xlc, and
icc. The fini function will support pgi. If a compiler/linker
combination does not support either the destructor or fini function a
message will be printed on re-init indicating it is not supported (an
improvement over the current behavior-- SEGV).

I moved the following to the destructor function:

 - Class system finalize. This solves a bug when MPI_T_finalize is
   called before MPI_Init. The only downside to this change is we will
   leave the footprint of the opal class system after
   MPI_Finalize. This footprint should be relatively small.

This is an alternative to open-mpi#517 but the two PRs are not
mutually-exclusive (with some modifications). This commit should also
be safe for 1.8.x as it does not change internal or external ABI (open-mpi#517
changes internal ABI).

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/451/

Build Log
last 50 lines

[...truncated 33058 lines...]
ok - coverity found no issues for hcoll_451

TAP Reports Processing: FINISH
coverity_for_all    http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr//ws/cov_build/all_451/output/errors/index.html
coverity_for_oshmem http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr//ws/cov_build/oshmem_451/output/errors/index.html
coverity_for_fca    http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr//ws/cov_build/fca_451/output/errors/index.html
[copy-to-slave] The build is taking place on the master node, no copy back to the master will take place.
Setting commit status on GitHub for https://api.github.com/repos/open-mpi/ompi/commit/21518289e51adaa2a8c54a8567cc336089aaede5
ERROR: Publisher com.cloudbees.jenkins.GitHubCommitNotifier aborted due to exception
java.io.IOException: {"message":"No commit found for SHA: 21518289e51adaa2a8c54a8567cc336089aaede5","documentation_url":"https://developer.github.com/v3/repos/statuses/"}
    at org.kohsuke.github.Requester.handleApiError(Requester.java:496)
    at org.kohsuke.github.Requester._to(Requester.java:245)
    at org.kohsuke.github.Requester.to(Requester.java:191)
    at org.kohsuke.github.GHRepository.createCommitStatus(GHRepository.java:780)
    at com.cloudbees.jenkins.GitHubCommitNotifier.updateCommitStatus(GitHubCommitNotifier.java:127)
    at com.cloudbees.jenkins.GitHubCommitNotifier.perform(GitHubCommitNotifier.java:84)
    at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
    at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:761)
    at hudson.model.AbstractBuild$AbstractBuildExecution.performAllBuildSteps(AbstractBuild.java:721)
    at hudson.model.Build$BuildExecution.post2(Build.java:183)
    at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:670)
    at hudson.model.Run.execute(Run.java:1766)
    at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
    at hudson.model.ResourceController.execute(ResourceController.java:98)
    at hudson.model.Executor.run(Executor.java:374)
Caused by: java.io.IOException: Server returned HTTP response code: 422 for URL: https://api.github.com/repos/open-mpi/ompi/statuses/21518289e51adaa2a8c54a8567cc336089aaede5
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:526)
    at sun.net.www.protocol.http.HttpURLConnection$6.run(HttpURLConnection.java:1675)
    at sun.net.www.protocol.http.HttpURLConnection$6.run(HttpURLConnection.java:1673)
    at java.security.AccessController.doPrivileged(Native Method)
    at sun.net.www.protocol.http.HttpURLConnection.getChainedException(HttpURLConnection.java:1671)
    at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1244)
    at sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:254)
    at org.kohsuke.github.Requester.parse(Requester.java:451)
    at org.kohsuke.github.Requester._to(Requester.java:224)
    ... 13 more
Caused by: java.io.IOException: Server returned HTTP response code: 422 for URL: https://api.github.com/repos/open-mpi/ompi/statuses/21518289e51adaa2a8c54a8567cc336089aaede5
    at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1626)
    at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:468)
    at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:338)
    at org.kohsuke.github.Requester.parse(Requester.java:447)
    ... 14 more
[BFA] Scanning build for known causes...
[BFA] No failure causes found
[BFA] Done. 0s
Setting status of 38589c46c02c254489eedf73680fb2334ca4fa33 to FAILURE with url http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr/451/ and message: Merged build finished.

Test FAILed.

@hppritcha
Copy link
Member

bot:retest
something going on with jenkins?
@miked-mellanox

@hjelmn
Copy link
Member Author

hjelmn commented Apr 17, 2015

bot:retest

@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/454/
Test PASSed.

opal/Makefile.am Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I'd change this to:

lib@OPAL_LIB_PREFIX@open_pal_la_LDFLAGS = -version-info $(libopen_pal_so_version)
if NEED_LD_FINI
lib@OPAL_LIB_PREFIX@open_pal_la_LDFLAGS += -Wl,fini,opal_cleanup
endif

and define the AM_CONDITIONAL for NEED_LD_FINI to be "if linker supports it and we the compiler doesn't support the destructor attribute.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/461/

Build Log
last 50 lines

[...truncated 33076 lines...]
ok - coverity found no issues for hcoll_461

TAP Reports Processing: FINISH
coverity_for_all    http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr//ws/cov_build/all_461/output/errors/index.html
coverity_for_oshmem http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr//ws/cov_build/oshmem_461/output/errors/index.html
coverity_for_fca    http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr//ws/cov_build/fca_461/output/errors/index.html
[copy-to-slave] The build is taking place on the master node, no copy back to the master will take place.
Setting commit status on GitHub for https://github.com/open-mpi/ompi/commit/224b4e853b98738f371a35a79db248483e7da21c
ERROR: Publisher com.cloudbees.jenkins.GitHubCommitNotifier aborted due to exception
java.io.IOException: {"message":"No commit found for SHA: 224b4e853b98738f371a35a79db248483e7da21c","documentation_url":"https://developer.github.com/v3/repos/statuses/"}
    at org.kohsuke.github.Requester.handleApiError(Requester.java:496)
    at org.kohsuke.github.Requester._to(Requester.java:245)
    at org.kohsuke.github.Requester.to(Requester.java:191)
    at org.kohsuke.github.GHRepository.createCommitStatus(GHRepository.java:780)
    at com.cloudbees.jenkins.GitHubCommitNotifier.updateCommitStatus(GitHubCommitNotifier.java:127)
    at com.cloudbees.jenkins.GitHubCommitNotifier.perform(GitHubCommitNotifier.java:84)
    at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
    at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:761)
    at hudson.model.AbstractBuild$AbstractBuildExecution.performAllBuildSteps(AbstractBuild.java:721)
    at hudson.model.Build$BuildExecution.post2(Build.java:183)
    at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:670)
    at hudson.model.Run.execute(Run.java:1766)
    at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
    at hudson.model.ResourceController.execute(ResourceController.java:98)
    at hudson.model.Executor.run(Executor.java:374)
Caused by: java.io.IOException: Server returned HTTP response code: 422 for URL: https://api.github.com/repos/open-mpi/ompi/statuses/224b4e853b98738f371a35a79db248483e7da21c
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:526)
    at sun.net.www.protocol.http.HttpURLConnection$6.run(HttpURLConnection.java:1675)
    at sun.net.www.protocol.http.HttpURLConnection$6.run(HttpURLConnection.java:1673)
    at java.security.AccessController.doPrivileged(Native Method)
    at sun.net.www.protocol.http.HttpURLConnection.getChainedException(HttpURLConnection.java:1671)
    at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1244)
    at sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:254)
    at org.kohsuke.github.Requester.parse(Requester.java:451)
    at org.kohsuke.github.Requester._to(Requester.java:224)
    ... 13 more
Caused by: java.io.IOException: Server returned HTTP response code: 422 for URL: https://api.github.com/repos/open-mpi/ompi/statuses/224b4e853b98738f371a35a79db248483e7da21c
    at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1626)
    at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:468)
    at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:338)
    at org.kohsuke.github.Requester.parse(Requester.java:447)
    ... 14 more
[BFA] Scanning build for known causes...
[BFA] No failure causes found
[BFA] Done. 0s
Setting status of 662460b06b8a93f1d3725aa13246190960dd0602 to FAILURE with url http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr/461/ and message: Merged build finished.

Test FAILed.

@hjelmn
Copy link
Member Author

hjelmn commented Apr 20, 2015

&^$%#. Damn Jenkins.

bot:retest

@hjelmn
Copy link
Member Author

hjelmn commented Apr 20, 2015

bot:retest

@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/463/
Test PASSed.

hjelmn added a commit that referenced this pull request Apr 20, 2015
Add opal destructor/fini function
@hjelmn hjelmn merged commit 8287e1d into open-mpi:master Apr 20, 2015
@hjelmn hjelmn deleted the add_destructor branch May 23, 2016 17:47
jsquyres pushed a commit to jsquyres/ompi that referenced this pull request Sep 19, 2016
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