feat: expose mimalloc memory stats - #1602
Conversation
|
There was a problem hiding this comment.
Pull request overview
Adds an opt-in memory-stats Cargo feature to expose mimalloc process/statistics data through the edr_napi N-API surface, enabling consumers to measure native allocator usage that’s otherwise hidden from typical JS/V8 profiling.
Changes:
- Introduces N-API exports for
memoryStats(),memoryReport(), andresetMemoryStats()(feature-gated). - Adds
libmimalloc-sysas an optional dependency behind the newmemory-statsfeature and wires up a local build script (pnpm build:memory-stats). - Adds Rust unit tests for the new memory stats/report/reset functionality.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/edr_napi/src/memory_stats.rs | New feature-gated N-API bindings to mimalloc stats APIs plus unit tests. |
| crates/edr_napi/src/lib.rs | Exposes the new memory_stats module from the crate root. |
| crates/edr_napi/src/cast.rs | Adds usize -> BigInt conversion used by mimalloc stats out-params. |
| crates/edr_napi/package.json | Adds build:memory-stats script for local builds. |
| crates/edr_napi/Cargo.toml | Adds optional libmimalloc-sys dependency and memory-stats feature. |
| Cargo.lock | Lockfile updates for libmimalloc-sys and transitive deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub mod log; | ||
| /// Types for an RPC request logger. | ||
| pub mod logger; | ||
| pub mod memory_stats; |
| mod tests { | ||
| use super::*; | ||
|
|
||
| /// Helper function to convert a `BigInt` to a `u64` for testing purposes. | ||
| /// `TryCast::try_cast` cannot be used directly, as the error type cannot be | ||
| /// inferred in the test context. | ||
| fn to_u64(value: BigInt) -> napi::Result<u64> { | ||
| value.try_cast() | ||
| } | ||
|
|
`mi_stats_reset` is deprecated in mimalloc v3 and no longer resets anything: it merges the main heap's thread-local statistics into the subproc and leaves both the current and peak counters untouched. Since `libmimalloc-sys` builds v3 by default, the binding could not deliver the per-phase measurements it advertised. Peaks cannot be reset in v3 at all, and per-phase deltas for the current values can be computed by diffing two `memoryStats()` snapshots, so drop the binding rather than redefine its semantics.
The previous assertions could not detect the most likely defect in a wrapper over an eight-out-parameter C function: mismapping the out-parameters to struct fields. `peak_rss > 0` holds for essentially any permutation of them. Assert instead that `peak_commit` is a high-water mark of `current_commit`, and that a live, fully touched 256 MiB allocation shows up in both `current_commit` and `peak_rss`. That pins the commit and RSS out-parameters to the fields they are read into. Drop the `to_u64` test helper: annotating the binding infers the target type, and with it the associated error type, so the helper was not needed.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1602 +/- ##
===========================================
- Coverage 79.89% 25.93% -53.96%
===========================================
Files 452 423 -29
Lines 78956 67715 -11241
Branches 78956 67715 -11241
===========================================
- Hits 63078 17562 -45516
- Misses 13697 49231 +35534
+ Partials 2181 922 -1259 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d95ef5e to
5faf406
Compare
Expose mimalloc memory statistics behind a
memory-statsfeature flagPurpose
EDR uses mimalloc as the global allocator of the N-API addon, which makes its native memory invisible to standard profiling tools: a heaptrack run against Hardhat 3's end-to-end benchmarks saw only 1.87 GB of malloc-attributed memory while the process RSS was 15.25 GB. Consumers currently have to approximate EDR's memory as
rss − v8HeapTotal − external − arrayBuffers.This PR exposes mimalloc's own statistics API through N-API, behind a new opt-in cargo feature
memory-stats:memoryStats(): MiMemoryStats— wrapsmi_process_info; returns elapsed/user/system time (ms), current/peak RSS, current/peak commit, and hard page faults asBigInts. The doc comments distinguish OS process-level values (RSS) from mimalloc-internal ones (commit), so consumers don't conflate them.memoryReport(): string— wrapsmi_stats_print_out; returns mimalloc's human-readable stats report.A
resetMemoryStats()wrapper was considered and dropped:mi_stats_resetis deprecated and effectively a no-op in the vendored mimalloc v3 (it merges thread-local stats instead of resetting, so peaks survive), which would silently break per-phase measurements. Per-phase deltas for the current values can be computed by diffing twomemoryStats()snapshots; peaks genuinely cannot be reset in v3.The bindings come from a direct optional dependency on
libmimalloc-sys(with itsextendedfeature), pinned to the version already locked via themimalloccrate — cargo unifies the two, so this adds bindings only, not a second allocator build. The existingmimallocdependency and#[global_allocator]are unchanged.Packaging
Published npm packages deliberately do not ship this feature:
build:publishandbuild:typingFileremain--features op, so the released binaries and the committedindex.d.tsare unchanged. The APIs are only available in local builds via the newpnpm build:memory-statsscript (mirroringbuild:scenarios/build:tracing).Stats detail
The depth of
memoryReport()is fixed at compile time by mimalloc'sMI_STATlevel. EDR's builds getMI_STAT=0("only essential") in every cargo profile — libmimalloc-sys only raises it via itsdebugfeature (which also enables mimalloc's internal assertions), and we don't enable it; a dev-profile report is byte-identical to a release one.MI_STAT0(what EDR builds, all profiles)reserved,committedcurrent+peak, purge/mmap/commit counters), page stats (touched,pages,abandoned, reclaim/retire counters), process info (threads, times, peak RSS/commit, page faults)1binned/huge/totalbytes of live blocks), tracked on every alloc/free2(requires libmimalloc-sysdebugfeature)malloc req(exact requested bytes)Note the practical consequence: the report attributes memory at the arena/commit level — which is the number that explains the RSS gap motivating this PR — but the per-size-class malloc breakdown readers might expect from mimalloc stats is compiled out in EDR's configuration. Everything
memoryStats()returns isMI_STAT=0data, so it is fully accurate in our builds.Testing
mi_process_infowrapper:peak_commit >= current_commitmust hold, a touched 256 MiB allocation must raisecurrent_commitby ≥200 MiB and drivepeak_rss≥200 MiB, andmemoryReport()must be non-empty. They pass under--features memory-statsand--all-features, so CI'sllvm-cov --all-featuresjob runs them automatically. (Note:peak_rss >= current_commitis deliberately not asserted — zeroed allocations commit pages without touching them, so commit can legitimately exceed RSS.)edr_napi, linking via the existingdyn-symbolsdev-dependency. Before the introduction ofdyn-symbolshaving Rust tests was not possible.cargo check --no-default-features --all-targetsandcargo clippy --all-targets --all-features -- -D warningsare clean;cargo treeconfirms a singlelibmimalloc-sysin the graph.pnpm build:memory-statsand loaded it in Node —memoryStats()returnsBigIntvalues andcurrentCommitmoved from 4.65 MB to 9.04 MB when allocating through an EDR API;memoryReport()returns a real mimalloc stats report. Notably,Buffer.allocdoes not move the commit figures (Node's heap isn't mimalloc), which is exactly the RSS-vs-commit distinction the API documents.index.d.ts/index.jsare untouched (generated with--features opas before), keeping the typings-check CI job green.