Skip to content

feat: expose mimalloc memory stats - #1602

Draft
Wodann wants to merge 7 commits into
mainfrom
mimalloc-mem-stats
Draft

feat: expose mimalloc memory stats#1602
Wodann wants to merge 7 commits into
mainfrom
mimalloc-mem-stats

Conversation

@Wodann

@Wodann Wodann commented Aug 6, 2026

Copy link
Copy Markdown
Member

Expose mimalloc memory statistics behind a memory-stats feature flag

Purpose

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 — wraps mi_process_info; returns elapsed/user/system time (ms), current/peak RSS, current/peak commit, and hard page faults as BigInts. The doc comments distinguish OS process-level values (RSS) from mimalloc-internal ones (commit), so consumers don't conflate them.
  • memoryReport(): string — wraps mi_stats_print_out; returns mimalloc's human-readable stats report.

A resetMemoryStats() wrapper was considered and dropped: mi_stats_reset is 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 two memoryStats() snapshots; peaks genuinely cannot be reset in v3.

The bindings come from a direct optional dependency on libmimalloc-sys (with its extended feature), pinned to the version already locked via the mimalloc crate — cargo unifies the two, so this adds bindings only, not a second allocator build. The existing mimalloc dependency and #[global_allocator] are unchanged.

Packaging

Published npm packages deliberately do not ship this feature: build:publish and build:typingFile remain --features op, so the released binaries and the committed index.d.ts are unchanged. The APIs are only available in local builds via the new pnpm build:memory-stats script (mirroring build:scenarios/build:tracing).

Stats detail

The depth of memoryReport() is fixed at compile time by mimalloc's MI_STAT level. EDR's builds get MI_STAT=0 ("only essential") in every cargo profile — libmimalloc-sys only raises it via its debug feature (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_STAT What the report exposes Cost
0 (what EDR builds, all profiles) Arena stats (reserved, committed current+peak, purge/mmap/commit counters), page stats (touched, pages, abandoned, reclaim/retire counters), process info (threads, times, peak RSS/commit, page faults) none
1 Adds per-heap block allocation totals (binned/huge/total bytes of live blocks), tracked on every alloc/free small
2 (requires libmimalloc-sys debug feature) Adds per-size-class bin table, allocation counts, and malloc req (exact requested bytes) measurable, plus internal assertions

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 is MI_STAT=0 data, so it is fully accurate in our builds.

Testing

  • New Rust unit tests constrain the out-param→field mapping of the mi_process_info wrapper: peak_commit >= current_commit must hold, a touched 256 MiB allocation must raise current_commit by ≥200 MiB and drive peak_rss ≥200 MiB, and memoryReport() must be non-empty. They pass under --features memory-stats and --all-features, so CI's llvm-cov --all-features job runs them automatically. (Note: peak_rss >= current_commit is deliberately not asserted — zeroed allocations commit pages without touching them, so commit can legitimately exceed RSS.)
    • This is the first Rust test in edr_napi, linking via the existing dyn-symbols dev-dependency. Before the introduction of dyn-symbols having Rust tests was not possible.
  • cargo check --no-default-features --all-targets and cargo clippy --all-targets --all-features -- -D warnings are clean; cargo tree confirms a single libmimalloc-sys in the graph.
  • Manual smoke test: built the addon with pnpm build:memory-stats and loaded it in Node — memoryStats() returns BigInt values and currentCommit moved from 4.65 MB to 9.04 MB when allocating through an EDR API; memoryReport() returns a real mimalloc stats report. Notably, Buffer.alloc does not move the commit figures (Node's heap isn't mimalloc), which is exactly the RSS-vs-commit distinction the API documents.
  • Committed index.d.ts/index.js are untouched (generated with --features op as before), keeping the typings-check CI job green.

@Wodann
Wodann requested a review from Copilot August 6, 2026 15:38
@Wodann Wodann self-assigned this Aug 6, 2026
@Wodann Wodann added the no changeset needed This PR doesn't require a changeset label Aug 6, 2026
@changeset-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 1c8f1e9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Wodann
Wodann temporarily deployed to github-action-benchmark August 6, 2026 15:38 — with GitHub Actions Inactive
@Wodann
Wodann had a problem deploying to github-action-benchmark August 6, 2026 15:40 — with GitHub Actions Failure
@Wodann
Wodann had a problem deploying to github-action-benchmark August 6, 2026 15:40 — with GitHub Actions Failure

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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(), and resetMemoryStats() (feature-gated).
  • Adds libmimalloc-sys as an optional dependency behind the new memory-stats feature 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;
Comment thread crates/edr_napi/src/memory_stats.rs Outdated
Comment on lines +150 to +159
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()
}

@Wodann
Wodann had a problem deploying to github-action-benchmark August 6, 2026 16:06 — with GitHub Actions Error
Wodann added 2 commits August 6, 2026 16:17
`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

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.93%. Comparing base (a69221b) to head (1c8f1e9).

Files with missing lines Patch % Lines
crates/edr_napi/src/cast.rs 0.00% 8 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Wodann
Wodann force-pushed the mimalloc-mem-stats branch from d95ef5e to 5faf406 Compare August 6, 2026 16:29
@Wodann
Wodann had a problem deploying to github-action-benchmark August 6, 2026 16:29 — with GitHub Actions Error
@Wodann
Wodann had a problem deploying to github-action-benchmark August 6, 2026 16:38 — with GitHub Actions Error
@Wodann
Wodann had a problem deploying to github-action-benchmark August 6, 2026 16:40 — with GitHub Actions Error
@Wodann
Wodann temporarily deployed to github-action-benchmark August 6, 2026 16:42 — with GitHub Actions Inactive
@Wodann
Wodann temporarily deployed to github-action-benchmark August 6, 2026 16:49 — with GitHub Actions Inactive
@Wodann
Wodann had a problem deploying to github-action-benchmark August 6, 2026 16:49 — with GitHub Actions Error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changeset needed This PR doesn't require a changeset

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants