tsgo backend: parse in-process, open the project lazily - #51
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It reworks performance-critical backend AST-parsing infrastructure whose correctness depends on cross-process node-kind/position invariants and the project's runtime-resolved typescript, which warrants final human review despite no defects being found.
Pull request overview
This PR optimizes the TypeScript 7 (tsgo) backend to cut two recurring costs measured on a large (838-file) Ember app. First, the resolver's syntactic parses of imported component files previously went through typescript-go as two IPC snapshot round-trips each; the backend now exposes a new parserSyntax facade that, under tsgo, uses the project's in-process typescript 5/6 library (via createSourceFile) when installed, falling back to the tsgo facade otherwise. Second, preload no longer eagerly opens the whole project — it opens the snapshot lazily only when at least one file misses the cache, matching the existing ts6.ts behavior. The reported effect is a large reduction in cold, warm, single-file, and --no-glint run times.
Changes:
- Add a
parserSyntaxfacade to theTypeBackendinterface and wire both backends to it (tsgo resolves the project'stypescriptlibrary in-process; ts6 reuses its existing syntax facade). - Switch the resolver-side calls in
extractAttrTypeMapandsyntaxForto useparserSyntax, keeping checker-side calls onsyntax. - Make tsgo
preloadopen the project snapshot lazily (if (loaded > 0)), aligning progress reporting withts6.ts.
File summaries
| File | Description |
|---|---|
| lib/backend/types.ts | Adds the required parserSyntax: TsSyntax field with an explanatory doc comment. |
| lib/backend/tsgo.ts | Adds libraryTypeScript() to load the project's in-process typescript; sets parserSyntax; makes preload open the snapshot lazily. |
| lib/backend/ts6.ts | Reuses the single ts6Syntax(ts) facade for both syntax and parserSyntax. |
| lib/glint.ts | Routes resolver syntactic parses (findTemplateSource, resolveTemplate, etc.) through parserSyntax; checker calls unchanged. |
| lib/backend/index.ts | syntaxFor now returns the backend's parserSyntax instead of syntax. |
Review details
- Files reviewed: 5/5 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.
The resolver's syntactic parses of imported component files went through typescript-go as two snapshot round-trips each (~15 ms), on every run, outside the extraction cache: 5.6 s of a fully cached 11 s run on an 838-file app. A syntactic parse needs no checker, so the tsgo backend now hands the resolver the project's `typescript` 5/6 library when one is installed (`TypeBackend.parserSyntax`), and its own facade otherwise. `preload` opened the whole project before knowing whether any file missed the cache (1.9 s on a warm run); it now opens it only when one does. Same app: warm run 10.8 s -> 8.2 s, single cached file 2.8 s -> 0.3 s, `--no-glint` 407 s -> 12 s, cold run 42 s -> 17 s. Both test lanes pass. Cowritten by Claude
🏎️ Benchmark Comparison
Full output |
The benchmark on this PR showed a cached single-file run 35 % slower and in-process extraction 12-29 % slower: the backend loaded the typescript library at construction, and the TS6 parse path re-parsed the same imported component for every consumer. The library now loads on first parse and parses are memoised per file, as the tsgo path already did. Cowritten by Claude
Two costs in the TypeScript 7 backend that hit every run, cached or not, on an 838-file app (profile: 11 s of a 22.8 s fully-cached run inside the tsgo client):
typescript5/6 library when one is installed (TypeBackend.parserSyntax, in-processcreateSourceFile), and its own facade otherwise. The resolver's node kinds must match its parser, which is why it is a separate facade from the checker-side one.preloadopened the whole project before knowing whether any file missed the cache: 1.9 s. It now opens it only when one does.Same app,
@glimmer/syntaxpatched with emberjs/ember.js#21314 on both sides:--no-glint, whole setThe
--no-glintnumber is the same parse path (build-maps.ts→syntaxFor) without the extraction cache in front of it.Both lanes: 291 passed + 1 expected fail.
Cowritten by Claude