Parse each template once - #54
Conversation
A template was parsed by the transformer, again for the multipass branch tree, and once more per pass; the resolver parsed the same component templates for every consumer. `parseTemplate` memoises `preprocess` by content (64 entries, oldest evicted; parse errors are not cached). Console app, warm run: 31.1 s -> 5.8 s with the published @glimmer/syntax (whose per-node position lookup scales with the number of parses), and the bench's whole-process cases drop 57-60 % with a 12-conditional template in the set — added as test/bench/large.gts, since the examples were too small to show parse cost. Cowritten by Claude
🏎️ Benchmark Comparison
Full output |
There was a problem hiding this comment.
🔵 Needs a closer look
The change introduces a process-global shared mutable AST cache across the transformer, multipass, and resolver whose correctness hinges on an unenforced read-only invariant, warranting final human verification.
Pull request overview
This PR eliminates redundant Glimmer template parsing. Previously the same template content was passed to @glimmer/syntax's preprocess multiple times: once by the transformer (transform.ts), again by blankTemplateContentMultipass to build the branch tree, once more per multipass pass in blankTemplateContent, and repeatedly by the resolver (walk.ts) for every consumer that reaches a given component template. A new lib/parse-template.ts module memoizes preprocess keyed by the content string (bounded to 64 entries with FIFO eviction; parse errors are rethrown and not cached), and all production call sites now route through it. This is a hot-path performance optimization that relies on the fact that all consumers only read the AST.
I verified the safety invariant the change depends on: the traverse visitors in blank.ts (line 1563) and build-maps.ts (lines 33/53/103), and the resolver walkers in walk.ts, all read AST nodes and accumulate offsets/ranges in external structures — none mutate the shared AST. The multipass code re-invokes blankTemplateContent with the same content but different branchSelections, which correctly reuses the cached parse since the AST is independent of branch selection.
Changes:
- Add
lib/parse-template.ts, a bounded content-keyed memoization wrapper aroundpreprocess. - Replace the eight direct
preprocess(...)call sites intransform.ts,blank.ts, andwalk.ts(removing the localparseTemplateinwalk.ts) with the shared helper. - Add a generated 12-conditional bench fixture (
test/bench/large.gts) and wire it intotest/validate.bench.mjs, adjustingSUBSET/ONEso the single-file case still measures a small file.
File summaries
| File | Description |
|---|---|
| lib/parse-template.ts | New content-keyed FIFO cache (64 entries) around preprocess; errors not cached. |
| transform.ts | Swaps two preprocess calls for parseTemplate. |
| blank.ts | Routes the two preprocess calls (single-pass + multipass) through parseTemplate; comment updated. |
| lib/resolver/walk.ts | Removes local parseTemplate/preprocess import in favor of the shared helper. |
| test/validate.bench.mjs | Adds the large fixture to FIXTURES/SUBSET; ONE now indexes a small example file. |
| test/bench/large.gts | New generated large template exercising multipass enumeration cost. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A template was parsed by the transformer (
transform.ts), again byblankTemplateContentMultipassfor the branch tree, and once more byblankTemplateContentper pass; the resolver (walk.ts) parsed the same component templates for every consumer that reached them.lib/parse-template.tsmemoisespreprocessby content string (64 entries, oldest evicted; parse errors are rethrown, not cached) and all four call sites use it.Safe because nothing writes to AST nodes: the walkers read,
build-mapsuses@glimmer/syntax'straverse, and both test lanes pass (291 + 1 expected fail).Measured:
@glimmer/syntax0.95.0, whose per-node position lookup is O(lines) and therefore scales with the number of parses. With thecharPosForfix (Perf: cache newline offsets in Source for O(log n) loc conversion emberjs/ember.js#21314) applied the gain is smaller in absolute terms; the parse count drops the same way.pnpm bench:compare): the whole-process cases drop 57–60 % once the set contains a template with conditionals, and cross-file resolution −30…−38 %.Also adds
test/bench/large.gts— a generated 12-conditional template — to the bench fixtures: theexamples/files are ≤2 KB and parse in microseconds, so the bench could not see parse cost before. While sizing it I measured that run time is superlinear in the number of conditionals (12 → ~0.5 s of plugin work, 30 → 4 s, 120 → 25 s): the multipass enumeration blanks the whole template once per branch combination, up to 2^cap passes. That's a separate optimisation.Cowritten by Claude