Skip to content

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Jul 22, 2025

Proposed Changes

Follow-up to:

The heaptrack feature added in my previous PR was ineffective, because the jemalloc feature was turned on by the Linux target-specific dependency.

This PR tweaks the features such that:

  • The jemalloc feature is just used to control whether jemalloc is compiled in. It is enabled on Linux by the target-specific dependency (see lighthouse/Cargo.toml), and completely disabled on Windows.
  • If the sysmalloc feature is set on Linux then it overrides jemalloc when selecting an allocator, even if the jemalloc feature is enabled (and the jemalloc dep was compiled).

@michaelsproul michaelsproul changed the title Attempt to fix malloc_utils features Fix malloc_utils features (sysmalloc) Jul 25, 2025
@michaelsproul michaelsproul added the ready-for-review The code is ready for review label Jul 25, 2025
Comment on lines +47 to +51
[target.'cfg(not(target_os = "windows"))'.dependencies]
malloc_utils = { workspace = true, features = ["jemalloc"] }

[target.'cfg(target_os = "windows")'.dependencies]
malloc_utils = { workspace = true, features = [] }
Copy link
Member Author

Choose a reason for hiding this comment

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

These changes ensure that lcli actually uses jemalloc on Linux, which is potentially relevant for the benchmarking subcommands.

Copy link

mergify bot commented Jul 25, 2025

Some required checks have failed. Could you please take a look @michaelsproul? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jul 25, 2025
@michaelsproul michaelsproul added v8.0.0-rc.0 Q3 2025 release for Fusaka on Holesky ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Aug 12, 2025
@jimmygchen
Copy link
Member

Added backward-incompat as jemalloc feature has been removed from the top level.

@jimmygchen jimmygchen added the backwards-incompat Backwards-incompatible API change label Aug 15, 2025
Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

The conditional compilation logic looks quite complex, but I think you've correctly covered the scenarios I can think of.

Looks good to me!

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 15, 2025
mergify bot added a commit that referenced this pull request Aug 15, 2025
@mergify mergify bot merged commit 317dc0f into sigp:unstable Aug 15, 2025
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompat Backwards-incompatible API change low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge. v8.0.0-rc.0 Q3 2025 release for Fusaka on Holesky
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants