Skip to content

Add jemalloc feature for using jemalloc instead of default allocator#126

Open
MarkusPettersson98 wants to merge 2 commits into
mimalloc-featurefrom
jemalloc-feature
Open

Add jemalloc feature for using jemalloc instead of default allocator#126
MarkusPettersson98 wants to merge 2 commits into
mimalloc-featurefrom
jemalloc-feature

Conversation

@MarkusPettersson98
Copy link
Copy Markdown
Contributor

@MarkusPettersson98 MarkusPettersson98 commented Apr 28, 2026

This PR builds on top of #27 and provides the jemalloc feature to use jemalloc as the global allocator.


This change is Reviewable

@MarkusPettersson98 MarkusPettersson98 requested a review from dlon April 28, 2026 10:46
Copy link
Copy Markdown
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

:lgtm:

@dlon reviewed 7 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on MarkusPettersson98).

Copy link
Copy Markdown
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

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

@Serock3 reviewed 7 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on MarkusPettersson98).


gotatun-cli/src/main.rs line 19 at r2 (raw file):

#[cfg(all(
    any(feature = "mimalloc", feature = "jemalloc"),
    not(all(feature = "mimalloc", feature = "jemalloc"))

Nit: I am unsure about this style of guards. You would not compile the module at all with all-features. If I ran cargo check --all-features I would expect to compile all code paths. In this case it's a single line module, so it's doesn't matter, but it might be worth considering in the general case.

Copy link
Copy Markdown
Contributor Author

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

@MarkusPettersson98 made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Serock3).


gotatun-cli/src/main.rs line 19 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Nit: I am unsure about this style of guards. You would not compile the module at all with all-features. If I ran cargo check --all-features I would expect to compile all code paths. In this case it's a single line module, so it's doesn't matter, but it might be worth considering in the general case.

I agree, it's fugly and now how features are generally supposed to work but in practice jemalloc and mimalloc need to be mutually exclusive. The alternative would be to enable one over the other if both features are enabled, which I don't think is necessarily better.

@MarkusPettersson98 MarkusPettersson98 marked this pull request as ready for review April 29, 2026 13:08
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.

3 participants