Skip to content

chore(track-memory-allocations): swap cal.com.tsx for excalidraw App.tsx#22590

Merged
graphite-app[bot] merged 1 commit into
mainfrom
chore/replace-cal-com-bench-fixture
May 19, 2026
Merged

chore(track-memory-allocations): swap cal.com.tsx for excalidraw App.tsx#22590
graphite-app[bot] merged 1 commit into
mainfrom
chore/replace-cal-com-bench-fixture

Conversation

@Dunqing

@Dunqing Dunqing commented May 19, 2026

Copy link
Copy Markdown
Member

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 OxcDiagnostics. 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, pinned to master @ f6d85bc8 (415KB, 13k lines, single source file). Semantic sys_allocs drops from 12,064 → 174, in line with the other fixtures.

@Dunqing Dunqing requested review from Boshen and camc314 May 19, 2026 13:50
@codspeed-hq

codspeed-hq Bot commented May 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing chore/replace-cal-com-bench-fixture (0c6c14b) with main (7a4120e)

Open in CodSpeed

Footnotes

  1. 3 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.

@camc314 camc314 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good call.

@camchenry camchenry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, cal.com.tsx is not a very realistic benchmark scenario, i think this is a good change

@Dunqing Dunqing force-pushed the chore/replace-cal-com-bench-fixture branch from 3267226 to 0c6c14b Compare May 19, 2026 14:44
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label May 19, 2026

Dunqing commented May 19, 2026

Copy link
Copy Markdown
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.
@graphite-app graphite-app Bot force-pushed the chore/replace-cal-com-bench-fixture branch from 0c6c14b to c7dfa6e Compare May 19, 2026 14:59
@graphite-app graphite-app Bot merged commit c7dfa6e into main May 19, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 19, 2026
@graphite-app graphite-app Bot deleted the chore/replace-cal-com-bench-fixture branch May 19, 2026 15:03
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.
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.

3 participants