chore(track-memory-allocations): swap cal.com.tsx for excalidraw App.tsx#22590
Merged
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
camchenry
approved these changes
May 19, 2026
camchenry
left a comment
Member
There was a problem hiding this comment.
yeah, cal.com.tsx is not a very realistic benchmark scenario, i think this is a good change
3267226 to
0c6c14b
Compare
Member
Author
Merge activity
|
…tsx (#22590) ## Summary `cal.com.tsx` (1.0MB) is a concatenation of many Next.js page files. Top-level names like `getServerSideProps`, `useLocale`, `PageWrapper`, `Page`, and `getLayout` are declared 70–113 times each, so semantic emits 2,914 redeclaration `OxcDiagnostic`s. Each diagnostic allocates ~4 strings/Vecs on the system heap, accounting for the bulk of the file's 12,064 reported semantic `sys_allocs` — an artifact of the fixture rather than typical real-world TSX semantic cost. Replace it with excalidraw [`App.tsx`](https://github.com/excalidraw/excalidraw/blob/f6d85bc80fe328e8f472636eb0d541f7bb891aa0/packages/excalidraw/components/App.tsx), pinned to master @ `f6d85bc8` (415KB, 13k lines, single source file). Semantic `sys_allocs` drops from 12,064 → 174, in line with the other fixtures.
0c6c14b to
c7dfa6e
Compare
2 tasks
graphite-app Bot
pushed a commit
that referenced
this pull request
May 21, 2026
…ec (#22599) ## Summary `SemanticBuilder::resolve_references_for_current_scope` is the early-resolution hook fired at function / arrow / catch boundaries — it resolves the references collected since a checkpoint against the outer scope chain so that e.g. `function f(x = outerRef) {}` correctly binds `outerRef` to the outer scope rather than the function body. The old implementation copied the pending slice into a fresh `Vec` on every call: ```rust let refs = self.unresolved_references.slice_from(checkpoint).to_vec(); self.unresolved_references.truncate(checkpoint); for (name, reference_id) in refs { ... } ``` The `to_vec()` is a fresh heap allocation, needed because `walk_up_resolve_reference` takes `&mut self` and would otherwise alias the borrow on the underlying `Vec<(Ident, ReferenceId)>`. Rewrite in-place with a retain-style write cursor — each `(Ident, ReferenceId)` is read by **value** (both are `Copy`), which detaches it from the borrow on the inner Vec. Unresolved refs are compacted forward; resolved refs are dropped via the final `truncate`. No temporary `Vec`, no allocation, **no `unsafe`** — safe bounds-checked indexing via new `get` / `set` methods on `UnresolvedReferences`. ```rust let mut write_idx = checkpoint; for read_idx in checkpoint..end { let (name, reference_id) = self.unresolved_references.get(read_idx); if !self.walk_up_resolve_reference(name, reference_id) { if write_idx != read_idx { self.unresolved_references.set(write_idx, name, reference_id); } write_idx += 1; } } self.unresolved_references.truncate(write_idx); ``` ## Allocation impact `cargo allocs`, `allocs_semantic.snap` (baseline = parent commit, after var-hoist landed): | File | Size | Sys allocs before | Sys allocs after | Δ | |---|---|---|---|---| | `checker.ts` | 2.92 MB | 2,309 | **53** | **−2,256 (−98%)** | | `App.tsx` | 415 kB | 174 | **35** | **−139 (−80%)** | | `binder.ts` | 193 kB | 196 | **19** | **−177 (−90%)** | | `kitchen-sink.tsx` | 733 kB | 1,320 | **290** | **−1,030 (−78%)** | | `pdf.mjs` | 567 kB | 1,366 | 1,355 | −11 | | `antd.js` | 6.69 MB | 1,072 | 1,072 | 0 (ES5 bundled code rarely closes over outer scope from function params) | | `RadixUI.jsx` | 2.5 kB | 10 | 10 | 0 | ## How I found this After [#22580](#22580) and [#22590](#22590) the remaining semantic sys allocs were unexplained. Ran a backtrace-capturing `System` allocator wrapper on `checker.ts` and aggregated allocations by call stack — the overwhelming majority of captured allocations converged on a single site: ``` oxc_semantic::builder::SemanticBuilder::resolve_references_for_current_scope oxc_semantic::builder::SemanticBuilder as ...::Visit::visit_function ... ``` Tracing back to the `slice_from(checkpoint).to_vec()` line and confirming that the entire allocation pattern disappears with the in-place rewrite. ## Why this matters for downstream consumers `resolve_references_for_current_scope` fires once per function / arrow / catch — for normal TS / TSX code that's hundreds to thousands of calls per build. Each call was a fresh heap allocation; eliminating them removes that allocator pressure entirely. For rolldown's preprocessing pipeline (which builds semantic 3-4 times per file across hundreds of files per bundle), the per-build savings compound. ## No behavior change The in-place algorithm preserves the original semantics exactly: `unresolved_references[..checkpoint]` is untouched, and `[checkpoint..]` is the list of refs that didn't resolve, in their original order. Same as the old code. ## Test Plan - [x] `cargo test -p oxc_semantic --lib --tests` — pass - [x] `cargo allocs` — semantic snapshot updated to reflect the reductions AI disclosure: drafted with Claude Code, reviewed manually.
graphite-app Bot
pushed a commit
that referenced
this pull request
Jun 1, 2026
) ## Summary Follow-up to #22590. That PR replaced the synthetic `cal.com.tsx` fixture — a concatenation of Next.js pages that declares top-level names 70–113× and emits thousands of redeclaration diagnostics, inflating semantic `sys_allocs` — with excalidraw's real, single-file `App.tsx`. But it only touched `TestFiles::complicated()` (used by `track_memory_allocations`). This applies the same swap to every remaining benchmark that still used `cal.com.tsx`, and unifies **all** excalidraw `App.tsx` references on one pin (`f6d85bc8`): | File | Change | | --- | --- | | `tasks/common/src/test_file.rs` | `minimal()` (feeds every `tasks/benchmark` criterion bench): `cal.com.tsx` → `App.tsx@f6d85bc8`. `formatter()` re-pinned `@v0.18.0` → `@f6d85bc8` | | `napi/parser/bench.bench.js` | parser benchmark fixture → `App.tsx@f6d85bc8` | | `napi/parser/test/parse-raw.test.ts` | bench-fixture list + size comment → `App.tsx@f6d85bc8` | | `tasks/benchmark/benches/semantic.rs` | rewrote the now-false "`cal.com.tsx` has many errors" comment | | `crates/oxc_ast/src/serialize/mod.rs` | removed the stale `cal.com.tsx` row from the capacity-ratio doc table | ## Why a single pin `get_source_text` caches downloads by the last URL segment only (`App.tsx`). With two different pins live, a single `cargo bench` run would download both into the same `target/App.tsx` and silently bench whichever landed first. Re-pinning `formatter()` to `f6d85bc8` removes that collision — and the pre-existing latent one between `cargo bench` (`@v0.18.0`) and `cargo allocs` (`@f6d85bc8`). ## Notes No committed snapshots change: these fixtures feed only timing benches and a `programRaw.toEqual(programStandard)` self-consistency test; the snapshotted `complicated()` / `minifier()` sets are untouched. --- This PR was prepared with AI assistance; I reviewed, tested, and verified the changes.
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
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.
Summary
cal.com.tsx(1.0MB) is a concatenation of many Next.js page files. Top-level names likegetServerSideProps,useLocale,PageWrapper,Page, andgetLayoutare declared 70–113 times each, so semantic emits 2,914 redeclarationOxcDiagnostics. Each diagnostic allocates ~4 strings/Vecs on the system heap, accounting for the bulk of the file's 12,064 reported semanticsys_allocs— an artifact of the fixture rather than typical real-world TSX semantic cost.Replace it with excalidraw
App.tsx, pinned to master @f6d85bc8(415KB, 13k lines, single source file). Semanticsys_allocsdrops from 12,064 → 174, in line with the other fixtures.