feat(profile): zero-cost-when-off fast path; default MI_PPROF=1 in headers - #281
Conversation
…aders Ports Bun's (oven-sh/mimalloc @ 942b8342, MIT) prof_force_slow strategy: while the profiler is stopped or compiled out, mi_malloc/mi_heap_malloc_small's fast path (free-list pop) now contains zero profiler instructions -- verified by disassembly, byte-identical between MI_PPROF=ON and MI_PPROF=OFF. Fixes the +70% ns/alloc regression measured in #154 (issue #267, Bun parity P2). Design: - mi_theap_t gains prof_force_slow (types.h, #if MI_PPROF). While set, mi_theap_queue_first_update (page-queue.c) leaves pages_free_direct poisoned (pointing at the static empty page) instead of publishing a real free page, so every malloc misses the fast list-pop and lands in _mi_malloc_generic, where the sampling countdown now lives exclusively (moved out of alloc.c:mi_page_malloc_zero, which stays shared with the fast path). - mi_prof_start/mi_prof_stop (profile.c) call a new cross-theap walker, _mi_subproc_prof_set_force_slow (subproc.c), OUTSIDE prof_lock, to flip the flag process-wide (mi_subprocs -> heap->next -> theap->hnext, each level under its existing lock). Start also poisons pages_free_direct directly (_mi_theap_pages_free_direct_poison, page-queue.c); stop deliberately does NOT touch pages_free_direct cross-thread (would race the owning theap's own queue mutations -- a theap re-syncs itself, same-thread, next time it takes the generic path: see mi_find_page and _mi_malloc_generic in page.c). - A theap created concurrently with start/stop reads mi_prof_is_enabled() under the same heap->theaps_lock the walker holds while visiting that heap's list (theap.c), so it is never missed. - _mi_prof_on_alloc now checks _mi_meta_is_meta_page() before prof_auto_start() (was after), since a meta allocation can hold subproc->theap_meta_lock or subproc->heaps_lock and the walker now needs those same locks -- reachable only from a genuine first user allocation, never from meta bootstrap. - Free path (free.c, page.c): hoist the existing page->has_metadata check to the call site, before the (previously unconditional) out-of-line _mi_prof_on_free/_mi_prof_on_free_collect call. - include/mimalloc/types.h: #ifndef MI_PPROF #define MI_PPROF 1 #endif, so a non-CMake consumer compiling src/static.c directly gets the profiler by default (matches CMake's own default); CMake keeps passing -DMI_PPROF=0/1 explicitly, which still wins. Verified both directions with a bare `c++ -x c++ -c src/static.c` compile. Verified: full ctest green on Debug-full+guarded (MI_PPROF=ON) and Release (MI_PPROF=OFF); a clang ASan build of the full suite, including a new test/test-profile.c case that starts/stops/restarts-with-a-different-rate the profiler while worker threads race the fast path, plus mi_prof_reset with a live sampled block, reports no errors. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
…t path Trailing rust/-only commit per CLAUDE.md rule 2: cargo run -p xtask -- amalgamate-c/amalgamate-h/check, picking up the prof_force_slow/poison-walker C changes from the preceding commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
…able PR #281 (issue #267) row, per the merge-gate requirement that every line ported from Bun gets a pass-3 table entry marked IMPORTED with the PR number. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
…indow The comment on mi_prof_start_seeded's cross-theap walk claimed the accepted start-time coverage gap "matches oven-sh/mimalloc @ 942b8342's own window" -- that specific detail of Bun's implementation was never inspected, only the overall prof_force_slow/poison strategy. Rephrase to attribute only what was actually verified: the strategy is ported from Bun, this window is a consequence of walking outside prof_lock in our own port. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
… fix Trailing rust/-only commit per CLAUDE.md rule 2, picking up the preceding commit's comment-only change to src/profile.c. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
Opus review of PR #281 found a real use-after-free hazard and a start/stop ordering race in the #267 zero-cost-when-off design; both fixed here. HIGH -- mi_theap_queue_first_update (page-queue.c) previously early-returned while `theap->prof_force_slow` was set, leaving `pages_free_direct` frozen instead of updated. That is unsafe: when a page is later retired (`_mi_page_free` -> `mi_page_queue_remove` -> this same function) while poisoned, the early return skipped the write entirely, so a stale pointer to that now-freed page could survive in `pages_free_direct` -- a use-after-free on the next fast-path allocation of that size class. Fixed by substituting the empty page for the real one instead of returning early, so every call still writes a value consistent with "currently valid queue page, or empty", regardless of any race with `_mi_subproc_prof_set_force_slow`'s cross-thread flag/poison writes. Reproduced the race itself (not full memory corruption, which needs the arena to actually reclaim/repurpose the freed page's memory within the test's short runtime) with a clang ThreadSanitizer build: with the old early-return code, TSan reliably flags the racy read of `prof_force_slow` gating that early return (`mi_theap_queue_first_update` at page-queue.c racing `_mi_subproc_prof_set_force_slow`'s write); with the fix, the same (by-design, documented-elsewhere-as-benign) read is still flagged by TSan -- expected, since it is a plain non-atomic bool read/write -- but no longer gates skipping the write. MEDIUM -- `_mi_subproc_prof_set_force_slow` took the target state as a snapshot `bool enable` argument captured before its (potentially slow, multi-lock) walk began. Two racing calls (a start and a stop from different threads) could then have their walks finish out of the order their `mi_prof_*` calls actually happened in, leaving every theap's `prof_force_slow` in the wrong final state. Fixed by dropping the parameter and reading `mi_prof_is_enabled()` fresh for each theap, inside the same `heap->theaps_lock` a concurrently-created theap uses to read the same flag (see `_mi_theap_init`, theap.c) -- whichever walk visits a theap last always writes that theap's actual current global state. MEDIUM -- test/test-profile.c's adversarial case previously had workers allocate/free one block at a time, so pages never emptied out and retired during the race window the HIGH bug depended on. Workers now allocate/free in BATCHES of 256 blocks across a few size classes so pages actually cycle through `_mi_page_free`, and the test asserts (via accum-mode sample counts, snapshotted with the main thread idle) that worker-thread allocations are sampled while the profiler runs, not just the pre-existing main-thread canary allocations. Also: reordered profile.c's comment to stop claiming this PR's specific start-time coverage-gap window "matches" Bun's own behavior, which was not independently verified (softened in the prior commit already); documented a latent (not fixed here) self-deadlock hazard in `prof_auto_start` for a mi_subproc_visit_heaps visitor that allocates for the first time. Verified: full ctest green (debug-full+guarded, MI_PPROF=ON); test-profile-race soaked 20x with zero failures; a clang TSan build confirms the fix removes the early-return gate without eliminating the documented-benign flag/poison race itself (still flagged by TSan, as expected). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
…fline uv run --with pyright==1.1.411 pyright tries to download a Node.js runtime via nodeenv on first use; that fails with an SSL cert error in a sandbox with no outbound network access, turning `lint` red for reasons unrelated to any lint finding. pyright[nodejs] bundles its own Node.js instead, so no network access is needed. Verified offline: `uv run --with 'pyright[nodejs]==1.1.411' pyright` now reports "0 errors, 0 warnings, 0 informations" in this same sandbox. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
Trailing rust/-only commit per CLAUDE.md rule 2: cargo run -p xtask -- amalgamate-c/amalgamate-h/check, picking up the page-queue.c/subproc.c/ profile.c changes from the preceding fix commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
…281 re-review) Residual MEDIUM from re-review: mi_theap_queue_first_update's "already set" short-circuit (`if (pages_free[idx] == page) return;`) assumes the whole [start..idx] range was last written as one unit by the SAME thread -- true in Bun, whose poison/unpoison runs same-thread at theap init, but false here since _mi_theap_pages_free_direct_poison (subproc.c's _mi_subproc_prof_sync_force_slow) writes the full [0..MI_PAGES_DIRECT) range cross-thread, one entry at a time, racing this function's own [start..idx] write. Interleave: poisoner writes slot j to empty, gets preempted; the owning thread's own update (having read prof_force_slow as stale-false) writes its whole [start..idx] range -- including j -- to a real page P; poisoner resumes and writes idx (and everything after j) back to empty. Result: pages_free[j] == P (stale), pages_free[idx] == empty. Every later update while poisoned now computes page=empty, matches pages_free[idx], and the short-circuit returns without ever repairing slot j -- P eventually retires, j dangles, and the first allocation of that wsize after mi_prof_stop dereferences freed memory. Multi-entry ranges (size classes spanning several pages_free_direct slots) make this reachable for wsize>8, not just a single-slot edge case. Fix: `if (pages_free[idx] == page && !prof_poisoning) return;` -- disables the short-circuit while prof_force_slow is set, so every update while poisoned re-covers the whole [start..idx] range (at most a few extra stores, already on the profiling-only slow path) instead of trusting a single already-matching tail slot. The next queue touch on that size class self-heals any interior slot the poisoner's non-atomic range write left stale. Also: fixed two stale comments (page.c's mi_theap_queue_first_update references said "early-return", which the prior review round already replaced with a substitution; one still called the re-sync calls a "no-op while poisoned", which is no longer true now that the short-circuit is disabled there) and renamed _mi_subproc_prof_set_force_slow -> _mi_subproc_prof_sync_force_slow (it no longer takes a target-state argument, so "set" was misleading; fixed a stale `(true)` call-site reference in a page-queue.c comment left over from before that parameter was dropped). Verified: Release and Debug-full (MI_PPROF=ON) both build clean; test-profile/test-profile-accum/test-profile-auto/test-profile-race/ test-prof-seed-determinism/test-memory-events/test-memory-events-env-enabled/ test-dhat all green on both configs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
Trailing rust/-only commit per CLAUDE.md rule 2: cargo run -p xtask -- amalgamate-c/amalgamate-h/check, picking up the pages_free_direct short-circuit fix and _mi_subproc_prof_sync_force_slow rename. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
|
Opus review rounds 1–3: APPROVE after fixes. Round 1 found a real UAF window (early-return in Merging: the two reds are |
Issue #274 (Bun parity P9b). docs/bun-gap-analysis-2026-09-02.md re-runs the 2026-09-01 analysis's four checks against Bun's current pin: the scripts/build/deps/mimalloc.ts commit (942b8342) has not moved, and all five consumer files (mimalloc_sys.rs, MimallocArena.rs, MimallocWTFMalloc.h, BunJSCModule.h, heapStats-mimalloc.test.ts) are byte-identical to what was cached in that session -- no new gap exists at Bun's current pin, so no new sub-issue is filed. The rest of the doc is a status table for every item and gap ID (B1-B20) from the 2026-09-01 analysis against what has actually merged since (#276, #281, #284, #286, #289, #291, #297) versus what's still open (#299, #302, and the items tied to them): 7/9 required-before-pitch items done, 11/20 B-numbered gaps done, 1 partial, 0 new. docs/ci-gates.md gains a `bun-surface` row and a short section explaining why that CI job is temporarily continue-on-error (mi_on_thread_idle isn't on main yet) with a dated TODO for when to make it a hard gate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S
Closes #267.
Update: addressed Opus review (3 items)
HIGH (fixed, commit 769adbf):
mi_theap_queue_first_update(page-queue.c) used toearly-return while
prof_force_slowwas set, freezingpages_free_directinstead ofupdating it. That's unsafe: a page retired (
_mi_page_free->mi_page_queue_remove->this function) while poisoned could leave a stale pointer in
pages_free_directafter thearena reclaims its memory -- a use-after-free on the next fast-path allocation of that size
class. Fixed by substituting the empty page for the real one instead of returning early, so
every call keeps writing a value consistent with "currently valid queue page, or empty".
Reproduced the race window itself with a clang ThreadSanitizer build (
-DMI_DEBUG_TSAN=ON -DCMAKE_C_COMPILER=clang -DMI_BUILD_SHARED=ON): with the old early-return code reinstated,TSan reliably (repeatedly, across a single run) flags the racy read of
prof_force_slowgating that early return in
mi_theap_queue_first_update, racing_mi_subproc_prof_set_force_slow's write. With the fix, TSan still flags the sameunderlying (by-design, documented, non-atomic) flag read/write -- expected, and unchanged
in character -- but it no longer gates skipping a
pages_free_directwrite. Plain ASan(gcc
-fsanitize=address) and 30 plain-debug runs of the reverted build did not catchmemory corruption directly within the test's short runtime -- plausible since actually
observing the UAF needs the arena to decommit/repurpose the freed page's memory, which
purge_delay(default ~1s) likely didn't trigger before the short-lived test processexited. TSan is the tool that actually demonstrates this race is reachable, not a
theoretical concern.
MEDIUM (fixed, commit 769adbf):
_mi_subproc_prof_set_force_slowtook the targetstate as a
bool enablesnapshot captured before its walk began. Two racing start/stopcalls could have their walks finish out of order, leaving every theap poisoned while
disabled or vice versa. Fixed by dropping the parameter and reading
mi_prof_is_enabled()fresh per theap, inside the same
heap->theaps_locka concurrently-created theap uses toread the same flag.
MEDIUM (fixed, commit 769adbf): the adversarial test's workers now allocate/free in
batches of 256 across 3 size classes (so pages actually retire during the race, which the
HIGH fix's exercise depends on) and assert -- via accum-mode sample counts snapshotted with
the main thread idle -- that worker-thread allocations are sampled while the profiler runs,
not just the main thread's own canary allocations.
LOW (done):
ci/verify_local.py'slintconfig now usespyright[nodejs]==1.1.411(commit2468ad9) so it resolves offline instead of trying to download a Node.js runtime.
prof_auto_start()now documents a latent (not fixed here) self-deadlock hazard: ami_subproc_visit_heapsvisitor callback that allocates for the first time wouldre-enter
subproc->heaps_locknon-reentrantly via the new walker. Left as a follow-up.applies to
mi_malloc/mi_heap_malloc_smallonly.mi_free's fast local-free pathgains one extra
cmpb $0x0,0x78(%rbx)/jne(the hoistedpage->has_metadatacheck)compared to
MI_PPROF=OFF-- confirmed by disassembling both Release builds'mi_free(aliased to
_ZdaPv/operator delete[]): the OFF build's fast local-free path goesstraight from the double-free check to
_mi_memevt_on_free; the ON build inserts thehas_metadatacompare-and-branch first. This is the intentional, minimal cost of thefree-side fix (avoiding an out-of-line call on every free of an unsampled page); it does
not affect the
mi_mallocdisassembly claim, which remains byte-identical.verify_localnow also haslintgreen;rustre-passes after the trailing amalgamationregen commit.
off/release/debug-full/guarded/shared/memory-gate/diagallstill PASS.
asanremains an environmental FAIL (unrelated toolchain gap, unchanged frombefore) -- see prior comment.
test-profile-racesoaked 20x with zero failures.Summary
Ports Bun's (
oven-sh/mimalloc @ 942b8342, MIT)prof_force_slowstrategy so thatmi_malloc/mi_heap_malloc_small's fast path contains zero profiler instructionswhen the profiler is stopped or compiled out, fixing the +70% ns/alloc regression
measured in #154. Also defaults
MI_PPROFto1in the header, matching CMake's owndefault, per the issue's Step 5 policy decision.
Design
mi_theap_tgainsprof_force_slow(include/mimalloc/types.h,#if MI_PPROF).While set,
mi_theap_queue_first_update(src/page-queue.c) leavespages_free_directpoisoned (pointing at the static empty page) instead ofpublishing a real free page, so every malloc misses the fast list-pop and lands in
_mi_malloc_generic, where the sampling countdown now lives exclusively (moved outof
alloc.c:mi_page_malloc_zero, which stays shared with the fast path and now haszero profiler code).
mi_prof_start/mi_prof_stop(src/profile.c) call a new cross-theap walker,_mi_subproc_prof_set_force_slow(src/subproc.c), outsideprof_lock, to flipthe flag process-wide (
mi_subprocs -> heap->next -> theap->hnext, each level underits existing lock:
mi_subprocs_lock -> subproc->heaps_lock -> heap->theaps_lock).pages_free_directdirectly(
_mi_theap_pages_free_direct_poison,page-queue.c).pages_free_directcross-thread — readinganother theap's
pq->firstwhile that theap's own thread concurrently mutates itsqueues (e.g.
_mi_page_freeremoving that very page) can publish a page that'sfreed moments later, a same-thread-vs-cross-thread use-after-free. Each theap
re-syncs its own
pages_free_direct, same-thread, lock-free, the next time it takesthe generic path (
mi_find_page/_mi_malloc_genericinpage.c).mi_prof_is_enabled()under thesame
heap->theaps_lockthe walker holds while visiting that heap's list(
theap.c), so a racing theap creation is never missed by the walker._mi_prof_on_allocnow checks_mi_meta_is_meta_page()beforeprof_auto_start()(previously after): a meta allocation (
_mi_meta_zalloc, used to bootstrap a thread'sown tld/theap) can hold
subproc->theap_meta_lockorsubproc->heaps_lock, and thenew walker needs those same locks — reachable only from a genuine first user
allocation now, never from meta bootstrap (would otherwise be a same-thread
non-reentrant-lock deadlock).
free.c,page.c): hoisted the existingpage->has_metadatacheck to thecall site, before the (previously unconditional) out-of-line
_mi_prof_on_free/_mi_prof_on_free_collectcall, avoiding a function-call on every free of an unsampledpage.
include/mimalloc/types.h:#ifndef MI_PPROF #define MI_PPROF 1 #endif. CMake keepspassing
-DMI_PPROF=0/1explicitly, which still wins (command-line define beats thein-header default). README's non-CMake-consumer paragraph updated to match.
Every ported block carries
// #267: ...provenance comments; the strategy (not thecode) is imported from
oven-sh/mimalloc @ 942b8342, MIT.Numbers
Disassembly (primary evidence, noise-free):
mi_malloc/mi_heap_malloc_smallinthe Release static lib (
libmimalloc.a,MI_PPROF=ON) contain no call to any_mi_prof_*function and no load of profiler state — the compiled bytes arebyte-for-byte identical to the same functions built with
MI_PPROF=OFF(only the baseaddress differs).
mi_heap_malloc_small's only call is_mi_memevt_on_alloc(thealways-on, accepted memory-events hook), and its slow-path branch jumps straight to
_mi_malloc_generic:Header default verified both directions with a bare compile (issue's exact acceptance
test):
Timing (corroborating, noisy shared dev container — not a dedicated benchmark host,
unlike #154's Windows/MinGW reference machine — direction is unambiguous and the
disassembly proof above is authoritative):
MI_PPROF=OFFMI_PPROF=ON, profiler stoppedMI_PPROF=ON, profiler running (default rate)MI_PPROF=ON-stopped is within noise ofMI_PPROF=OFF(down from #154's measured+70%); the actively-running cost is paid only while a profile is in progress. Full
writeup in
docs/profiler.md(force-added;docs/is gitignored in this repo).Tests
test/test-profile.cwith the issue's adversarial checklist in one new case(
test_start_while_allocating_stop_mid_sample_restart_reset): start while workerthreads are mid-allocation-loop, stop mid-sample, restart with a different rate while
workers race the restart too, and
mi_prof_resetwhile a page still carries a livesampled block.
test-profile-raceandtest-subproc-lifecyclerun directly (outsideverify_local's default fast tier) on Debug-full+guarded (MI_PPROF=ON),MI_PPROF=OFF, and a clang ASan build — all green, ASan reports no errors (nouse-after-free, no leak) across 5 repeated runs of the new adversarial test plus the
full suite.
verify_localtablegh issue comment 10posted with the same numbers.What the issue got wrong / diverged from
mi_prof_resetdoes exist (issue text implied it might not, in the "map thatadversarial case to stop+restart" fallback instruction) — it only resets the accum
counters and interned-stack table, not live records, so the reset-while-flagged test
case verifies the (pre-existing, unaffected-by-this-PR) live-block behavior rather than
anything Bun-specific about "keeping the flag."
ci/dev_linux.py benchrequires Docker orchestration out of scope for a quickbefore/after; used a small standalone microbenchmark matching docs: measure and publish the MI_PPROF=ON disabled-path overhead (#50) #154's methodology
instead (alloc+free pairs, best-of-5, interleaved).
--only bundlefrom the task briefisn't a
verify_local.py --listtarget at this pin — noted, not treated as a gate.🤖 Generated with Claude Code
https://claude.ai/code/session_01YT4jVokb2gdT8ngFrH8i8S