Add optional mimalloc feature with allocator metrics - #10046
Open
maxjtwelftree wants to merge 4 commits into
Open
maxjtwelftree wants to merge 4 commits into
maxjtwelftree wants to merge 4 commits into
Conversation
Adds a `mimalloc` feature to `malloc_utils` and the `lighthouse` binary, so a
node can be built with mimalloc instead of jemalloc for comparison.
Both `lighthouse` and `lcli` declare `malloc_utils` with the `jemalloc` feature
on every non-Windows target, so that feature cannot be switched off through
feature selection. Rejecting "two allocators enabled" with `compile_error!`
would therefore break every Linux and macOS build. mimalloc instead resolves by
precedence, reusing the mechanism `sysmalloc` already relies on:
sysmalloc > mimalloc > jemalloc > glibc > system
`scrape_allocator_metrics()` is called unconditionally by the beacon node and
validator client metrics servers, so an allocator that reports nothing leaves a
node's /metrics with no allocator data at all. This exports five gauges from
`mi_process_info` to keep that endpoint useful. The process CPU counters that
`mi_process_info` also returns are intentionally left out: they measure the
whole process rather than time spent in the allocator, and the Prometheus
process collector already reports them.
Default builds are unaffected.
|
|
The benchmarks suggested on sigp#8840 use `lcli transition-blocks`, but `lcli` hardcoded `malloc_utils` with the `jemalloc` feature and had no way to select mimalloc, so the build the issue asks for could not be produced. Adds the passthrough. Adds `malloc_utils` to the existing `cargo-hack` feature-powerset job. Nothing in CI compiled the mimalloc arm, so the `cfg` precedence lattice had no regression guard; the powerset covers all 97 combinations, including the glibc arm that only selects on Linux. Also drops an unrelated `Cargo.lock` change. Adding the dependency caused the resolver to move `rustix` from `windows-sys` 0.52.0 to 0.59.0, which is unrelated to mimalloc and would show up as noise in a dependency review. Reverted, and `cargo check --locked` confirms the lockfile is still self-consistent, leaving the diff additions-only. Softens a comment that claimed the new gauges mirror the `jemalloc_*` ones closely enough for a dashboard to chart either allocator. The metric names differ, so they are not drop-in.
Add `mimalloc` to wordlist.txt. The book mentions it outside a code span and the spellcheck job only skips `code` and `pre` elements. Exclude `jemalloc-profiling` and `jemalloc-unprefixed` from the powerset check. Neither appears in a cfg gate, so they cost build time without adding coverage.
Build artifact from running pyspelling locally, committed by accident.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Issue Addressed
#8840
Adds an optional
mimallocfeature tomalloc_utils,lighthouseandlcli, so a node or a benchmark can be built with mimalloc in place of jemalloc, and exports allocator metrics for it.This builds on the feature plumbing @0xMars42 wrote in #8884
Resolution by precedence
lighthouse/Cargo.tomlandlcli/Cargo.tomlboth declaremalloc_utilswithfeatures = ["jemalloc"]for every non-Windows target, so thejemallocfeature cannot be switched off through feature selection. Rejecting "two allocators enabled" with acompile_error!would fail every Linux and macOS build.mimalloc therefore resolves by precedence, reusing the mechanism
sysmallocalready relies on:Metrics
Five gauges from
mi_process_info, via theextendedfeature oflibmimalloc-sys:mimalloc_current_rss_bytesmimalloc_peak_rss_bytesmimalloc_current_commit_bytesjemalloc_bytes_mappedmimalloc_peak_commit_bytesmimalloc_page_faultsVerification
Every feature combination compiles and selects the expected allocator. The matrix below was run on
aarch64-apple-darwin; thesysmalloc,jemallocandmimallocrows were independently confirmed onx86_64-unknown-linux-gnu, where each binary printed its own allocator name during the benchmark runs below.allocator_name()sysmallocjemallocmimallocmimalloc,jemallocmimalloc,sysmallocsysmallocjemallocmimallocjemalloc returns 90% of its peak to the OS; mimalloc returns 26% and settles at 2.0x jemalloc's resident footprint. Given that Lighthouse's live memory problems are memory-shaped, #9530 being an 8.8 GB OOM, that seems like the more decision-relevant column than throughput.
Per the LLM note in
CONTRIBUTING.md: I used AI assistance while working on this. I have read, built and tested every line of the diff myself, and the verification figures above are from my own machines.