perf(cli): use mimalloc as the global allocator - #1355
Merged
Conversation
The Linux release binaries are static-musl. musl's mallocng serializes every allocation in the process behind one global lock and is substantially slower than mimalloc under this CLI's allocation patterns — parsing tens of thousands of build events, building large Starlark heaps, and streaming lint outputs — with the BES, sink, and file-probe threads all contending with the Starlark thread. mimalloc keeps per-thread free lists, removing that contention. It builds through the crate universe for every release target with no annotation (the same path ring's C/asm already takes), adds ~2MB, and needs no build-script workaround. This also settles an open question on the intermittent lint segfault: the crash is musl mallocng detecting corrupted heap metadata (a_crash in enframe). Running on a different allocator tells us whether the fault tracks the allocator or our own writes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✨ Aspect Workflows Tasks📅 Tue Jul 28 23:29:47 UTC 2026 ❌ 1 failed task
|
gregmagolan
added a commit
that referenced
this pull request
Jul 29, 2026
Follow-up to #1355 (mimalloc). Builds mimalloc with its `secure` feature and makes detected heap corruption fatal. Moving off musl's mallocng loses a diagnostic property we were relying on. mallocng validates a check byte on every allocation, so heap corruption aborted the process by itself — that is exactly what produced the `a_crash`/`enframe` abort used to identify the intermittent lint segfault as heap corruption in the first place. A default mimalloc build performs no equivalent check, so the same corruption would now pass unnoticed and resurface later as an unrelated failure. **Secure build.** `MI_SECURE=4` gives guard pages around metadata, encoded free lists, randomized placement, and double-free detection. Verified present in the built `x86_64-unknown-linux-musl` release binary: `corrupted free list entry of size %zub at %p`, `double free detected of block %p with size %zu`, and `corrupted meta-data in thread-free list`. **Abort on detection.** Detection alone is not enough: mimalloc's default handler aborts only on `EFAULT` (corrupted metadata, corrupted thread-free list, and — under `secure` — a detected buffer overflow), while a double free (`EAGAIN`) or a free of an invalid pointer (`EINVAL`) is reported and then execution *continues*. Continuing on a corrupted heap is what makes this class of bug so hard to trace: the eventual crash lands somewhere unrelated, long after the write that caused it. This registers an error handler via `mi_register_error` that aborts on every corruption code, so a detection is fatal where it happens. The SIGABRT is caught by the crash handler, which reports it with a resolvable address. Setting `MIMALLOC_SHOW_ERRORS=1` on a diagnostic run additionally prints the message saying *what* was detected (`show_errors` defaults off in a release build). The abort happens either way. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: no (behavior is documented on the allocator item in `main.rs`) - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes - chore: the allocator is now built in secure mode and aborts when it detects heap corruption (a smashed free list, a double free, corrupted metadata) rather than continuing on a corrupted heap. Set `MIMALLOC_SHOW_ERRORS=1` to also print what was detected. ### Test plan - New test case added: an end-to-end test drives a deliberate double free through the internal crash-test hook and asserts the allocator reports it (`double free detected`) and the process dies by SIGABRT with a crash report — the path that was previously silent. - Covered by existing test cases (full AXL suite: 910 passing; crash-handler suite: 6 passing). - Manual: the `x86_64-unknown-linux-musl` release target builds via `--config=release`; `mi_register_error` and all three secure detectors are linked into the binary, and it runs normally (`aspect --version`). ### Note on cost `MI_SECURE=4` adds guard pages and free-list encoding, which is not free. If the perf hit is material we can land this as a diagnostic-only release and revert, the same way #1352 was scoped. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Sets mimalloc as the global allocator for
aspect-cli.The Linux release binaries are static-musl. musl's mallocng serializes every allocation in the process behind a single global lock, and is substantially slower than mimalloc under this CLI's allocation patterns — parsing tens of thousands of build events, building large Starlark heaps, streaming lint outputs — with the BES, sink, and file-probe threads all contending with the Starlark thread on every
malloc/free. mimalloc keeps per-thread free lists, which removes that contention.It builds through the crate universe for every release target with no annotation — the same path
ring's C/asm already takes — so nogen_build_scriptworkaround or BCR dependency is needed. Binary grows ~2MB (51MB → 53MB on x86_64 musl).Changes are visible to end-users: yes
Searched for relevant documentation and updated as needed: no (no user-facing allocator behavior is documented)
Breaking change (forces users to change their own code or config): no
Suggested release notes appear below: yes
perf: aspect-cli now uses mimalloc as its allocator, removing global allocator-lock contention on Linux (static-musl) builds where the BES, sink, and file-read threads previously serialized against the Starlark thread on every allocation.
Test plan
x86_64-unknown-linux-muslandaarch64-apple-darwinrelease targets both build via--config=release; the musl binary links mimalloc (21 symbol matches), runs (aspect 2026.31.6), and the fatal-signal crash handler still reports correctly under the new allocator.Note for the ongoing lint-segfault investigation
Landing this also acts as a diagnostic. That crash is musl's mallocng aborting from
a_crash()insideenframe— its check that the byte preceding a slot's user pointer is clear — which means something wrote out of bounds. Auditing has not found the writer, and ASan is unavailable on this target (sanitizer is incompatible with statically linked libc), so swapping allocators is the available experiment:There is no known mallocng defect matching this signature. The one recent mallocng correctness fix (musl
bf96b52a, 2026-05-12) traps inget_metaonfree/reallocof allocations with gigabyte-scale alignment — a different function, a different assert, and alignments nothing here requests — so it is not a candidate explanation.