Fix support for using a system hwloc and jemalloc #23409
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ronawho
force-pushed
the
fix-system-jemalloc-hwloc
branch
2 times, most recently
from
f45232c to
b5ef8b3
Compare
Our chunk hooks (only used for some multi-locale configs) don't work with jemalloc 5.x (which switched to extent hooks) so don't try to enable them for 5.x. For system jemalloc and hwloc, provide compile/link args instead of assuming they are in a default search path. Lastly don't filter out include paths to jemalloc/hwloc in the qthreads makefile (qthreads users our allocator and hwloc so we need to provide it with paths to build.) This is motivated by wanting to use system hwloc/jemalloc for our homebrew formulae. Homebrew hwloc/jemalloc aren't available in default compiler search paths, so we need to use pkg-config to find them and ensure qthreads (which uses our hwloc and jemalloc) can find them too. Signed-off-by: Elliot Ronaghan <[email protected]>
ronawho
force-pushed
the
fix-system-jemalloc-hwloc
branch
from
b5ef8b3 to
8801b17
Compare
|
Hmm, with a clean build, this isn't working. It looks like we're getting into the jemalloc makefile when we shouldn't be and failing with: This is because I think we'll need to update |
stonea
added a commit
that referenced
this pull request
Sep 15, 2023
In #23366, I updated the Homebrew formula to use system versions of jemalloc and hwloc. Unfortunately, this change runs into issues. Some of which may be fixed later by this PR (#23409 (comment)). There's also this issue about the bundled jemalloc makefile being run and failing despite the fact that we're supposed to be using the system one #23409 (comment)). Given all this, and how close we are to the release, we decided to have the formula use cstdlib for memory management instead. [Reviewed by @ronawho]
1 task
jabraham17
added a commit
to jabraham17/chapel
that referenced
this pull request
Jun 3, 2024
Signed-off-by: Jade Abraham <[email protected]>
1 task
jabraham17
added a commit
that referenced
this pull request
Jun 3, 2024
This PR patches some of our build scripts to work with `CHPL_HWLOC=system`. By default, we will still continue to default to `CHPL_HWLOC=bundled` with qthreads, but users can opt in to using a system install by explicitly setting `CHPL_HWLOC=system` This is a step in the right direction to allow us to use the preferred Chapel configuration for Macs in homebrew. Testing: - [x] `start_test test/release/examples` with `CHPL_HWLOC=system` on Mac Notes: - Partially unblocks #24931 and #24655 - Used #23409 as a starting point [Reviewed by @jhh67]
jabraham17
added a commit
to jabraham17/chapel
that referenced
this pull request
Jun 3, 2024
Signed-off-by: Jade Abraham <[email protected]>
jabraham17
added a commit
that referenced
this pull request
Jun 6, 2024
…#25158) This PR patches some of our build configuration scripts to be able to use `CHPL_TARGET_JEMALLOC=system`. This allows us to use our preferred configuration for performance (`CHPL_MEM=jemalloc`) in situations where we cannot build the bundled jemalloc (i.e. for packaging) When using a system jemalloc 5.x, we have to turn off chunk hooks (jemalloc 5.x does not support it) Testing: - [x] `start_test test/release/examples` with `CHPL_MEM=jemalloc` and `CHPL_TARGET_JEMALLOC` on Mac Notes: - Builds on top of #25145 - Fully unblocks #24931 and #24655, and enables homebrew to use the preferred config (#25148) - Uses diffs from #23409 [Reviewed by @jhh67]
|
@jabraham17 : I stumbled across this PR looking for another one and am wondering whether the work you did recently supplants this effort such that this could/should be closed? I see above you mentioned it in your commits/PRs, but I admittedly didn't go to see what was said. |
|
Great, thanks. @ronawho I'm going to close rather than asking you to, but shout if you think that's wrong. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our chunk hooks (only used for some multi-locale configs) don't work with jemalloc 5.x (which switched to extent hooks) so don't try to enable them for 5.x. For system jemalloc and hwloc, provide compile/link args instead of assuming they are in a default search path. Lastly don't filter out include paths to jemalloc/hwloc in the qthreads makefile (qthreads users our allocator and hwloc so we need to provide it with paths to build.)
This is motivated by wanting to use system hwloc/jemalloc for our homebrew formulae. Homebrew hwloc/jemalloc aren't available in default compiler search paths, so we need to use pkg-config to find them and ensure qthreads (which uses our hwloc and jemalloc) can find them too.