build: drop the MI_NO_SET_VMA_NAME define mimalloc never read - #37080
Conversation
scripts/build/deps/mimalloc.ts passed -DMI_NO_SET_VMA_NAME=1 with a comment saying it skips prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME) after each mmap. No revision of oven-sh/mimalloc has ever checked that macro (git log --all -S finds nothing), so the prctl has always run and mimalloc's regions show up as [anon:mimalloc] in /proc/<pid>/maps -- which is useful, so keep the behavior and delete the dead knob and its misleading comment. Same for the copy in the JSON bench script. No-Verification-Needed: removes an unread preprocessor define; compiled output is unchanged
|
Updated 7:13 PM PT - Aug 6th, 2026
✅ @dylan-conway, your commit 9f8839385698d62536ac0cb1f48c5c0c4df049b7 passed in 🧪 To try this PR locally: bunx bun-pr 37080That installs a local version of the PR into your bun-37080 --bun |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe changes stop defining ChangesMimalloc build flags
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — deletes a dead -D define that mimalloc never read, plus rustfmt-only reflows from autofix.
What was reviewed
- Confirmed
MI_NO_SET_VMA_NAMEhas no remaining references in the repo; the vendored mimalloc gates the prctl only ondefined(PR_SET_VMA), so the define was a no-op. - The four
.rsfile changes are pure formatting (line wraps, import sort) from the autofix.ci commit — no semantic change. - Checked the remaining
MI_FLAGSinbench-json-rust.share unaffected.
Extended reasoning...
Overview
Two substantive edits: scripts/build/deps/mimalloc.ts drops the Linux-only defines.MI_NO_SET_VMA_NAME = 1 block (and its 7-line comment), and scripts/bench-json-rust.sh drops the same flag from MI_FLAGS. The PR description documents that no revision of oven-sh/mimalloc has ever referenced this macro — the prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, …) call in src/prim/unix/prim.c is gated only on defined(PR_SET_VMA) — so the define has always compiled to nothing and the [anon:mimalloc] VMA labels have always been present. A repo-wide grep after the change confirms zero remaining references.
The other four files (tty.rs, ansi_renderer.rs, bake_body.rs, server_body.rs) are the autofix.ci commit and contain only rustfmt reflows: wrapping single-line calls across multiple lines and alphabetizing a use group. Byte-for-byte inspection of the diff shows no token changes, only whitespace/line-break reformatting.
Security risks
None. This removes an unread preprocessor define from build scripts; the compiled mimalloc object is identical. No runtime code paths, inputs, or trust boundaries are touched.
Level of scrutiny
Low. Build-config dead-code deletion with a well-documented rationale, plus formatter noise. The one thing worth verifying — that the macro is truly unread — is confirmed by grep and the PR author's git log -S over the mimalloc fork history.
Other factors
No prior human review comments to address. The change aligns with REVIEW.md's "delete dead code in the same PR that makes it dead" guidance, and the misleading comment (which claimed the define suppressed a syscall it never suppressed) is exactly the kind of stale documentation worth removing.
There was a problem hiding this comment.
LGTM — removes a dead -D define that mimalloc never read; no functional change.
What was reviewed:
- Confirmed
MI_NO_SET_VMA_NAMEhas no remaining references anywhere in the repo. - Checked the
MI_FLAGSarray inbench-json-rust.shis still well-formed after the trailing item was dropped. - Verified surrounding defines in
mimalloc.ts(MI_DEFAULT_ALLOW_THP,MI_LIBC_MUSL,MI_MALLOC_OVERRIDE) are untouched.
Extended reasoning...
Overview
This PR deletes the -DMI_NO_SET_VMA_NAME=1 define from two places: the mimalloc dependency build config (scripts/build/deps/mimalloc.ts) and the JSON bench helper script (scripts/bench-json-rust.sh). The accompanying 7-line explanatory comment in mimalloc.ts is also removed. The PR description states — and a repo-wide grep confirms — that this macro is not consumed anywhere; the vendored mimalloc gates its prctl(PR_SET_VMA, ...) call only on defined(PR_SET_VMA), so the define was always a no-op and the compiled object is byte-identical with or without it.
Security risks
None. This is a build-flag deletion in tooling scripts; no runtime code, no user-facing surface, no auth/crypto/permissions paths involved.
Level of scrutiny
Low. The change is a pure deletion of dead configuration, which REVIEW.md explicitly encourages ("delete dead code in the same PR that makes it dead"). The only correctness question is whether the macro was actually dead — the author verified this against the full history of oven-sh/mimalloc, and I confirmed there are zero references to MI_NO_SET_VMA_NAME remaining in this repo. Even if the macro were live, its removal would only re-enable VMA naming (a debugging aid), which the PR argues is desirable anyway.
Other factors
Both edit sites remain syntactically valid: the bash array in bench-json-rust.sh still closes cleanly on the previous element, and the TS defines object flows straight from the MI_DEFAULT_ALLOW_THP block into the musl/override checks with no dangling references. No prior reviewer comments to address; no CODEOWNER-gated paths; the bug hunting system found nothing. This falls squarely into "simple, mechanical, self-contained with clear intent."
3 participants
What does this PR do?
scripts/build/deps/mimalloc.tspasses-DMI_NO_SET_VMA_NAME=1on Linux with a comment claiming it disables mimalloc'sprctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ...)after each mmap. No revision of oven-sh/mimalloc has ever referenced that macro (git log --all -S MI_NO_SET_VMA_NAMEis empty; the call insrc/prim/unix/prim.cis gated only ondefined(PR_SET_VMA)), so it has been a no-op since it was added and mimalloc's regions have always shown up as[anon:mimalloc]in/proc/<pid>/maps. That labelling is genuinely useful for RSS attribution (it's how the numbers in #36990 were broken down), and the cost is one prctl per arena reservation, so this keeps the behavior and deletes the dead define plus its misleading comment. Same for the copy inscripts/bench-json-rust.sh.No functional change: the compiled mimalloc object is identical with or without the define.
How did you verify your code works?
grep -rn MI_NO_SET_VMA_NAMEover the repo and over oven-sh/mimalloc history is empty after this change; a Linux release build still shows[anon:mimalloc]VMAs in/proc/self/maps, same as before.