alloc: add force-jemalloc-macos feature #27041
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
guswynn
force-pushed
the
force-jemalloc-macos
branch
from
58dc8d9 to
3d68e74
Compare
guswynn
force-pushed
the
force-jemalloc-macos
branch
from
3d68e74 to
757a2d0
Compare
ParkMyCar
approved these changes
May 13, 2024
There was a problem hiding this comment.
To fix the problem of building tikv_jemallocator on macOS could you do [target.'cfg(...).features]`? e.g. something like:
[target.'cfg(not(target_os = "macos"))'.features]
default = ["jemalloc", "workspace-hack"]
jemalloc = ["tikv-jemallocator", "mz-prof/jemalloc", "mz-prof-http/jemalloc"]
[target.'cfg(target_os = "macos")'.features]
default = ["workspace-hack"]
jemalloc = ["tikv-jemallocator"]
approving to clear the block if its needed!
|
Let me ponder this one today. I'd love to find a way to do this that doesn't require introducing another Cargo feature. (Although I know Cargo makes that hard with its lack of support for target-specific features.) |
|
@benesch yes I'll wait! I couldn't come up with anything, but let me know! @ParkMyCar I don't think cargo allows that! |
benesch
added a commit
to benesch/materialize
that referenced
this pull request
May 13, 2024
Twiddle our crates and Cargo configuration for activating jemalloc to
achieve the following ideal developer experience:
* --no-default-features unequivocally disables jemalloc, regardless of
platform.
* --features=jemalloc unequivocally enables jemalloc, regardless of
platform.
* --default-features chooses the best allocator for the platform: the
system allocator on macOS and jemalloc on Linux.
Since Cargo doesn't explicitly support target-specific features, the
trick is to introduce a crate of indirection, as described in [0].
Here's how it works:
* The `mz-prof`, `mz-prof-http`, and `mz-alloc` crates have a
straightforward setup: they expose a `jemalloc` feature and
conditionally enable or disable jemalloc features based on whether
the `jemalloc` feature is provided or not.
* The `mz-alloc-default` crate depends on the `mz-alloc` crate and
enables the best features for the platform: `jemalloc` on Linux, and
no additional features on macOS. You can think of this crate as
unconditionally enabling the best allocator for the platform.
* The `mz-environmentd` and `mz-clusterd` crates unlock the
conditional behavior. Bothc crates:
1. depend on `mz-alloc-default` by default, and
2. expose a `jemalloc` feature that force enables the
`mz-alloc/jemalloc` feature.
The motivation for all of this is to allow macOS developers to enable
jemalloc upon request, which is useful for certain kinds of testing.
Alternative to MaterializeInc#27041.
Closes MaterializeInc#27041.
[0]: rust-lang/cargo#1197 (comment)
5 tasks
|
I put up #27067, which should address the issues here! AFAICT, it makes |
|
@benesch your replacement pr looks good! |
benesch
added a commit
to benesch/materialize
that referenced
this pull request
May 13, 2024
Twiddle our crates and Cargo configuration for activating jemalloc to
achieve the following ideal developer experience:
* --no-default-features unequivocally disables jemalloc, regardless of
platform.
* --features=jemalloc unequivocally enables jemalloc, regardless of
platform.
* --default-features chooses the best allocator for the platform: the
system allocator on macOS and jemalloc on Linux.
Since Cargo doesn't explicitly support target-specific features, the
trick is to introduce a crate of indirection, as described in [0].
Here's how it works:
* The `mz-prof`, `mz-prof-http`, and `mz-alloc` crates have a
straightforward setup: they expose a `jemalloc` feature and
conditionally enable or disable jemalloc features based on whether
the `jemalloc` feature is provided or not.
* The `mz-alloc-default` crate depends on the `mz-alloc` crate and
enables the best features for the platform: `jemalloc` on Linux, and
no additional features on macOS. You can think of this crate as
unconditionally enabling the best allocator for the platform.
* The `mz-environmentd` and `mz-clusterd` crates unlock the
conditional behavior. Bothc crates:
1. depend on `mz-alloc-default` by default, and
2. expose a `jemalloc` feature that force enables the
`mz-alloc/jemalloc` feature.
The motivation for all of this is to allow macOS developers to enable
jemalloc upon request, which is useful for certain kinds of testing.
Alternative to MaterializeInc#27041.
Closes MaterializeInc#27041.
[0]: rust-lang/cargo#1197 (comment)
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.
Some experiments I am doing would be easier if I could turn on jemalloc stats on macos. Heap profiling doesn't work, but it appears relatively stable.
@benesch unfortunately I couldn't figure out a way to not build
tikv-jemallocatorifforce-jemalloc-macosis off. I don't think you can ANDfeatureandplatform's intarget.cfg()clauses, so the cost of this is macos users have to buildtikv-jemallocatoronce. One way around this is turning offjemallocby default inbin/environmentdand adding--features jemallocforlinuxusers, but that seemed not worth it, because it would also mean we'd have to explicitly enable it everywhere else we build materialize.context: https://materializeinc.slack.com/archives/C0246GEHL8N/p1715300698582579
Motivation
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.