refactor(profile): move heap-profile/stats hooks out of corealloc.h#80
Merged
jayakasadev merged 8 commits intoJun 19, 2026
Merged
Conversation
Lift all heap-profile and allocator-stats logic out of the critical corealloc.h into two new headers, behind function calls that inline to nothing when the feature flags are off: - mem/alloc_stats.h: AllocStats bundles FrontendStats (SNMALLOC_STATS_BASIC) and SizeClassStats (SNMALLOC_STATS_FULL) plus the per-Allocator hook methods (on_small_refill, on_local_dealloc, on_remote_ingest, ...). Empty and zero-size ([[no_unique_address]]) when no stats tier is built. - profile/hooks.h: record_dealloc / record_dealloc_peek declared unconditionally; empty inline no-ops when SNMALLOC_PROFILE is off (the real definitions stay in profile/record.h). corealloc.h drops from 21 stats/profile #ifdef regions to zero: it holds a single AllocStats member and 11 one-line hook calls (~770 net lines removed). stats_export.cc follows the member rename; the thread-teardown drain call is now unconditional. Flag-off codegen is opcode-multiset-identical to the prior baseline (zero added or removed instructions; only register scheduling differs). Builds clean with no flag, SNMALLOC_STATS_BASIC, SNMALLOC_STATS_FULL and SNMALLOC_PROFILE.
…mments Remove development-bookkeeping citations (phase / ticket / plan pointers) from the heap-profile and allocator-stats headers and tighten the verbose comments, so they read as durable code documentation rather than a development log. Comments only; no code changes.
The first commit's profile/hooks.h defined no-op record_dealloc / record_dealloc_peek when SNMALLOC_PROFILE was off. That collided with profile/record.h, which defines those unconditionally (self-disabling per config) so the profiler tests can call them directly in any build: a TU that includes record.h with the flag off saw two definitions. Introduce distinct entry points profile::on_dealloc / on_dealloc_peek for the allocator to call. hooks.h owns the flag-off no-ops; record.h defines the flag-on forwarders to record_dealloc. record_dealloc itself is untouched, so the direct-call tests keep working. Also update the fast_path_counters test for the stats member rename (a->stats -> a->alloc_stats.frontend). Full func test suite now passes 82/82 with no flag and with SNMALLOC_PROFILE + SNMALLOC_STATS_FULL.
Run the repo's clang-format (v15, matching CI) over the new and changed profiling/stats sources. Whitespace and line-wrapping only.
Apply the dealloc-hook treatment to the remaining critical files. The alloc chokepoint (globalalloc.h) and the in-place realloc fast path (libc.h) now call profile::on_alloc / on_realloc unconditionally instead of wrapping profile::record_alloc / record_realloc in `#ifdef SNMALLOC_PROFILE`. The on_* entry points (declared in profile/hooks.h, defined in profile/record.h when profiling is on, no-op inline when off) keep the call sites free of feature ifdefs; globalalloc.h keeps its single gated record.h include for the definition at instantiation. Also strip the remaining plan/ticket references from the realloc hook comments (libc.h, rust.cc) and the stale forward-declaration note in backend_helpers.h. Builds clean and the func suite passes 82/82 with no flag and with SNMALLOC_PROFILE + SNMALLOC_STATS_FULL; clang-format-15 and clang-tidy-15 clean.
Apply the comment cleanup to the allocator headers and profiler tests the
first pass missed: backend_helpers (largebuddyrange, commonconfig, commitrange,
statsrange, buddy), mem/metadata.h, pal/pal_stack_walker.h,
global/runtime_config.h, override/{runtime_config,stats_dump}.cc, and the
src/test func/perf profile_* / fast_path_counters / lazy_array_client_meta
tests. Comments only (plus two clang-format empty-block normalizations);
plan/ticket strings inside test output (check()/static_assert messages) are
intentionally left as-is.
Func suite 82/82 with no flag and with SNMALLOC_PROFILE + SNMALLOC_STATS_FULL;
clang-format-15 clean.
… files Extend the comment cleanup to the rest of the PR: snmalloc-rs src + tests, snmalloc-tools, criterion benches, build.rs, CMakeLists.txt, Cargo.toml, Bazel/CI configs, the branch-hint script docstrings, and README prose. Comments / doc-comments / prose only -- no code, test names, or build directives changed. The two phase-named test-probe identifiers (snmalloc_rs_phase_4_4_symbolize_probe, ..._phase_11_3_callsite_probe_*) are left as-is: they are symbolication-test fixtures coupled to assertion strings. cargo test passes with stats-full and profiling; snmalloc-tools builds clean.
- Delete the development-log docs that do not belong upstream:
docs/heap-profiling-benchmarks.md (1675-line bench log) and
docs/heap-profiling-diagnostic-11-10.md.
- Clean docs/profiling-pmu.md (a genuine user doc) of phase/deliverable
framing; the perf / snmalloc-tools workflow and commands are unchanged.
- Strip the remaining plan/ticket references from comments in the files the
earlier passes missed: snmalloc-sys/src/lib.rs, override/{stats_export.cc,
rust.cc, rust_profile.h}, two profile_overhead static_assert messages, and
the now-dead heap-profiling-benchmarks.md references in
Cargo.toml/CMakeLists.txt and the branch-hint script docstrings.
Comments / docs / prose only. Func suite 82/82 with SNMALLOC_PROFILE +
SNMALLOC_STATS_FULL; cargo profiling tests pass; clang-format-15 clean.
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.
What: Heap-profile + allocator-stats logic lifted out of
corealloc.hinto two new headers (mem/alloc_stats.h,profile/hooks.h). The hot paths call hook functions that inline to nothing when the flags are off. 21 stats/profile#ifdefregions incorealloc.h→ 0; ~770 net lines removed. Second commit strips plan/phase/ticket comments from the profile + stats headers and tightens the verbose ones.Why: Addresses mjp41's feedback on upstream microsoft#857 — keep the critical
corealloc.hminimal (no piles of#ifdef; reviewable function calls that inline to nothing; stats/profiling in a separate file), and make comments document the code rather than the development plan.Cost: Zero runtime when off. Flag-off codegen is opcode-multiset-identical to baseline (16581 = 16581 instrs, 0 added/removed; only register scheduling differs).
AllocStatsis empty + zero-size ([[no_unique_address]]) when no stats tier is built.Evidence: Builds clean in all four configs (no-flag /
SNMALLOC_STATS_BASIC/SNMALLOC_STATS_FULL/SNMALLOC_PROFILE). Rust tests:stats-full21/21,profiling26/26. Flag-off disasm diff = pure register reorder, identical opcode multiset.