Use trie for removeStringLiteralsMatchedByTemplateLiterals - #63900
Use trie for removeStringLiteralsMatchedByTemplateLiterals#63900Sebastian "Sebbie" Silbermann (eps1lon) wants to merge 3 commits into
Conversation
Optimize removeStringLiteralsMatchedByTemplateLiterals by building a prefix trie from TemplateLiteralType patterns and using O(L) trie traversal per string literal instead of O(m) linear scan across all templates. StringMappingType templates (which cannot be trie-indexed) are checked separately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
typescript-automation
Bot
added
the
For Uncommitted Bug
The prefix trie built by `buildTemplateLiteralTrieFromTypes` is constructed fresh on every call, and each node was its own small heap allocation. This change routes those nodes through a function-local `core.Arena[templateLiteralTrieNode]`, which batches them into a single backing array that grows by doubling, so building a trie of N nodes costs O(log N) allocations instead of N. Returning pointers into a function-local arena is safe here because `Arena.New` hands out pointers into a heap-allocated slice, and growth allocates a fresh array rather than copying into a larger one. Previously returned nodes are therefore never invalidated, and they stay reachable through the returned root. Co-Authored-By: Claude Code (kimi-k3[1m]) <noreply@anthropic.com>
3728e59 to
16a1b44
Compare
There was a problem hiding this comment.
Pull request overview
Optimizes template-literal union reduction using prefix-based lookup.
Changes:
- Adds a trie for template-literal prefixes.
- Separates string-mapping fallback checks.
- Uses trie traversal when multiple templates exist.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tsc/internal/checker/types.go |
Defines trie nodes. |
tsc/internal/checker/relater.go |
Builds and searches the trie. |
tsc/internal/checker/checker.go |
Integrates trie-based filtering. |
| var arena core.Arena[templateLiteralTrieNode] | ||
| root := arena.New() | ||
| for _, t := range templateTypes { | ||
| prefix := t.AsTemplateLiteralType().texts[0] |
There was a problem hiding this comment.
Done. Each prefix node now stores its templates in a nested trie keyed by the reversed final static text, plus a skip when prefix and suffix would have to overlap, both mirroring the necessary conditions inferFromLiteralPartsToTemplateLiteral already checks. For the DynamicRoutes shape (shared / prefix, distinct final segments) a route literal now only pays full validation for templates whose final static text it ends with.
One correction backed by measurement: the issue's scaling was not left intact by the prefix-only trie. On the repro from #63342, check time went from ~8.5s at the base to ~0.31s at this PR's head, because the hot union is dominated by ${StaticRoute}${SearchOrHash} templates with long distinct prefixes. The suffix discrimination brings it to ~0.25s. Templates sharing a prefix and ending in a placeholder (final text "") still share a bucket; separating those would require matching middle segments.
| if len(templateLiterals) >= 2 { | ||
| trie = c.buildTemplateLiteralTrieFromTypes(templateLiterals) |
There was a problem hiding this comment.
Confirmed empirically before gating. The new BenchmarkTemplateLiteralMatchingPaths sweeps literals x templates for both paths (trie construction included, since the trie is rebuilt per union reduction), and with the original map-per-node children the trie lost to the linear scan at every measured size (up to 900x at 1x128). Children are now small slices scanned linearly, which cut construction cost roughly 3-5x.
The trie is now built only when templates x literals >= 512 (the measured crossover; above it the trie wins up to ~2.5x for selective unions) and when the templates differ in their first or final static text. Without any selectivity the trie cannot discriminate and measured ~1.3-1.4x slower than linear at large sizes, so those unions stay on the linear path. The gate check itself is a flag-count pass plus a short string-equality scan, measured far cheaper than building even a two-template trie.
| for i > 0 { | ||
| i-- | ||
| t := types[i] | ||
| if t.flags&TypeFlagsStringLiteral != 0 && c.isStringLiteralMatchedByTemplates(t, trie, templateLiterals, stringMappings) { |
There was a problem hiding this comment.
Added tsc/testdata/tests/cases/compiler/templateLiteralUnionReduction.ts: 16 route templates with overlapping prefixes (several sharing both prefix and segment count, so only the final static text distinguishes them), 32 literals with matching and non-matching members, and an Uppercase<up${string}> constituent. The union is sized to actually pass the trie gate, and the errors baseline pins the reduced unions, covering both the trie traversal and the separated string-mapping fallback.
Also added gate-independent Go unit tests in template_literal_trie_test.go covering root (empty-prefix) candidates, the prefix/suffix overlap edge case, and both the linear and trie paths of removeStringLiteralsMatchedByTemplateLiterals.
…its construction The trie indexed templates only by their first static text, so patterns that share a prefix (every dynamic route in microsoft#63342 starts with "/") still placed all templates in one bucket and scanned them linearly per string literal. Each prefix node now stores its templates in a nested trie keyed by the reversed final static text, and candidates are skipped when the static prefix and suffix would have to overlap in the literal. Both filters are sound because they mirror the necessary conditions inferFromLiteralPartsToTemplateLiteral already checks. On the issue's repro this reduces check time from ~0.31s to ~0.25s compared with the prefix-only trie (the base without any trie takes ~8.5s). Benchmarking both matching paths showed that building the trie was a pessimization for small unions: with map-based child tables, even a handful of templates cost more to index than the linear scan they replaced. Child tables are now small slices scanned linearly, which reduced construction cost by roughly 3-5x, and removeStringLiteralsMatchedByTemplateLiterals only builds the trie once the expected comparison count (templates times string literals) reaches 512, the crossover measured by the new BenchmarkTemplateLiteralMatchingPaths. Templates that all share their first and final static text stay on the linear path entirely, since the trie cannot discriminate between them and measured 30-40% slower at large sizes. The gate check itself is a flag-count pass plus a short string-equality scan, far cheaper than building even a two-template trie. Adds a compiler regression test with overlapping template prefixes, matching and non-matching literals, and a string mapping constituent, plus unit tests that cover the trie directly regardless of the gate. Co-Authored-By: Claude Code (kimi-k3[1m]) <noreply@anthropic.com>
5fb281c to
72114fc
Compare
Continuation of microsoft/typescript-go#3331
Started out as a guess that template literal checking should be done using a trie which led to
removeStringLiteralsMatchedByTemplateLiteralsbeing the major contributor in CPU traces.Actual implementation is vibe-coded with Claude Opus 4.6 (1M context)
Tested this in our internal apps (~90s
tscduration before). Ideally we'd run this against TypeScript's extensive benchmarks since this a space vs runtime tradeoff so we might want to opt out of building the trie for small unions.Optimize
removeStringLiteralsMatchedByTemplateLiteralsby building a prefix trie fromTemplateLiteralTypepatterns and using O(L) trie traversal per string literal instead of O(m) linear scan across all templates.StringMappingTypetemplates (which cannot be trie-indexed) are checked separately.Fixes #63342