Skip to content

fix(track-memory-allocations): make arena size snapshots platform-independent#24565

Merged
graphite-app[bot] merged 1 commit into
mainfrom
allocs-yaml-tolerance
Jul 15, 2026
Merged

fix(track-memory-allocations): make arena size snapshots platform-independent#24565
graphite-app[bot] merged 1 commit into
mainfrom
allocs-yaml-tolerance

Conversation

@Boshen

@Boshen Boshen commented Jul 15, 2026

Copy link
Copy Markdown
Member

The Allocations CI job fails whenever the snapshots are regenerated on a different CPU architecture than CI (e.g. https://github.com/oxc-project/oxc/actions/runs/29420345285/job/87368903834): every arena size value differs by 32–128 bytes between aarch64 and x86_64. The cause is target-dependent type layout — hashbrown's table control groups are 16 bytes wide on x86_64 (SSE2) but 8 bytes on aarch64 (NEON) — so each live HashMap in the arena shifts used_bytes() slightly. This is the same root cause that #22621 handled for chunk counts; the arena size gauge added in #24553 re-exposed it. Allocation counts are unaffected and stay exact.

Exact byte equality across architectures isn't achievable (the arena genuinely occupies a different number of bytes), so instead the snapshot writer tolerates layout noise:

  • Snapshots are now YAML (allocs_*.yaml) with the same layout as before: one value per line, file names at column 0 for git hunk headers, exact numbers as the diffed values, and human-readable sizes moved to trailing # comments.
  • When regenerating, cargo allocs parses the committed snapshot (with saphyr, already a workspace dependency used by oxc_coverage) and keeps the committed arena size when the measured value is within ARENA_SIZE_TOLERANCE (1024 bytes, ~8× the observed worst-case cross-arch drift). Differences beyond the tolerance — real regressions such as growing an AST node type change sizes by orders of magnitude more — rewrite the value and fail the git diff --exit-code check as before.
  • The trailing comments are rendered from the recorded value, so a snapshot regenerated on any platform stays byte-identical while within tolerance. No CI workflow changes are needed, and drift can't accumulate silently: each run compares against the committed value, so creeping sub-tolerance changes eventually cross the threshold and surface.

The committed values here are aarch64-measured; x86_64 CI measures 32–128 bytes higher and keeps them. Open PRs that touch the old .snap files will need a rebase and a cargo allocs rerun.

🤖 Implemented with Claude Code.

@Boshen
Boshen requested a review from overlookmotel as a code owner July 15, 2026 15:39
@Boshen
Boshen force-pushed the allocs-yaml-tolerance branch from a51150b to 22b1924 Compare July 15, 2026 15:47
@codspeed-hq

codspeed-hq Bot commented Jul 15, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing allocs-yaml-tolerance (22b1924) with main (747feec)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jul 15, 2026

Boshen commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Merge activity

…ependent (#24565)

The `Allocations` CI job fails whenever the snapshots are regenerated on a different CPU architecture than CI (e.g. https://github.com/oxc-project/oxc/actions/runs/29420345285/job/87368903834): every `arena size` value differs by 32–128 bytes between aarch64 and x86_64. The cause is target-dependent type layout — hashbrown's table control groups are 16 bytes wide on x86_64 (SSE2) but 8 bytes on aarch64 (NEON) — so each live `HashMap` in the arena shifts `used_bytes()` slightly. This is the same root cause that #22621 handled for chunk counts; the `arena size` gauge added in #24553 re-exposed it. Allocation counts are unaffected and stay exact.

Exact byte equality across architectures isn't achievable (the arena genuinely occupies a different number of bytes), so instead the snapshot writer tolerates layout noise:

- Snapshots are now YAML (`allocs_*.yaml`) with the same layout as before: one value per line, file names at column 0 for git hunk headers, exact numbers as the diffed values, and human-readable sizes moved to trailing `#` comments.
- When regenerating, `cargo allocs` parses the committed snapshot (with `saphyr`, already a workspace dependency used by `oxc_coverage`) and keeps the committed `arena size` when the measured value is within `ARENA_SIZE_TOLERANCE` (1024 bytes, ~8× the observed worst-case cross-arch drift). Differences beyond the tolerance — real regressions such as growing an AST node type change sizes by orders of magnitude more — rewrite the value and fail the `git diff --exit-code` check as before.
- The trailing comments are rendered from the recorded value, so a snapshot regenerated on any platform stays byte-identical while within tolerance. No CI workflow changes are needed, and drift can't accumulate silently: each run compares against the committed value, so creeping sub-tolerance changes eventually cross the threshold and surface.

The committed values here are aarch64-measured; x86_64 CI measures 32–128 bytes higher and keeps them. Open PRs that touch the old `.snap` files will need a rebase and a `cargo allocs` rerun.

🤖 Implemented with [Claude Code](https://claude.com/claude-code).
@graphite-app
graphite-app Bot force-pushed the allocs-yaml-tolerance branch from 22b1924 to b1165f9 Compare July 15, 2026 16:07
@graphite-app
graphite-app Bot merged commit b1165f9 into main Jul 15, 2026
30 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 15, 2026
@graphite-app
graphite-app Bot deleted the allocs-yaml-tolerance branch July 15, 2026 16:12
graphite-app Bot pushed a commit that referenced this pull request Jul 16, 2026
…r heap and arena (#24587)

Follow-up to #24565. Restructures the task so that adding a new heap or arena metric is a small, local change instead of edits scattered across measurement, diffing, rendering, and committed-value lookup:

- `HeapTracker` groups the global-allocator atomics into one static `HEAP` with `read()`/`reset()`; its doc comment describes how to add a heap counter.
- `Counters { heap: HeapCounters, arena: ArenaCounters }` replaces the flat `AllocatorStats`, with `record()` / `diff_since()` (the arena-chunk exclusion logic from #22621 is unchanged, just relocated).
- `StageStats::metrics()` is now the single declaration of what a snapshot contains: an ordered list of `Metric`s, each a label plus a `MetricKind`.
- `MetricKind` encodes the per-metric snapshot policy: `Count` (platform-exact), `ExactBytes` (exact, human-readable comment), `ApproxBytes` (tolerates cross-architecture layout noise by keeping the committed value within `APPROX_BYTES_TOLERANCE`, renamed from `ARENA_SIZE_TOLERANCE` since it is no longer specific to arena size).
- `write_snapshot` / `render_file_stats` are fully metric-driven; nothing downstream needs touching when a metric is added.

Adding e.g. a `sys deallocs` count is now: one `AtomicCounter` in `HeapTracker`, one field in `HeapCounters`, one `Metric::count(...)` entry. A new byte gauge (heap peak, arena capacity) is one gauge read plus one `Metric::approx_bytes(...)` entry.

Behavior is unchanged: the committed snapshots are byte-identical when regenerated with the refactored code.

🤖 Implemented with [Claude Code](https://claude.com/claude-code).
graphite-app Bot pushed a commit that referenced this pull request Jul 17, 2026
… peak growth (#24619)

Adds three heap-side metrics to the allocation snapshots, using the metric machinery from #24587:

- `sys deallocs` (exact count) — system deallocations per stage; read against `sys allocs` it shows whether a stage churns or retains heap.
- `sys alloc bytes` — cumulative bytes requested from the system allocator per stage (allocation sizes plus reallocation growth).
- `sys peak growth` — high-water mark of live heap during a stage relative to the live bytes at stage start, via live-bytes accounting in the `GlobalAlloc` wrapper. Captures transient heap a stage needs even when it frees it before finishing.

Arena chunk operations are excluded from all `sys` metrics at the source: `oxc_allocator` marks each chunk allocation/deallocation immediately before calling the system allocator (task-only `track_allocations` feature, compiled out everywhere else), and the task's tracking allocator consumes the marker and skips the call. This replaces the previous global chunk counter + per-stage subtraction: a subtraction can correct counts but not a high-water mark, and chunk sizes are quantized and platform-dependent (whether an arena needs one more chunk depends on byte totals that vary across architectures with type layout — the #22621 mechanism). The first CI run demonstrated this concretely: kitchen-sink's minifier peak differed deterministically by 720 kB between aarch64 and x86_64 because the mangler's temporary arena crossed a chunk-growth boundary only on x86_64; and antd's formatter "peak" was 762 MB of main-arena chunk-doubling history rather than formatter behavior (now 13.8 MB of actual heap use).

Cross-platform tolerances: the cumulative byte metrics still drift with the number of heap hashbrown tables allocated (their control-group width is architecture-dependent), so they use a relative tolerance (1%, 4 kB floor) via the committed-value comparison from #24565; `arena size` keeps its tight 1024-byte tolerance.

Sample values for checker.ts (2.9 MB source): parser needs only 2.4 kB of transient heap (fully arena-based), semantic peaks at 3.9 MB, the minifier is the pipeline's largest heap consumer at 10.2 MB, and the formatter requests 5.9 MB total while peaking at 3 MB (heavy alloc/free churn).

🤖 Implemented with [Claude Code](https://claude.com/claude-code).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant