Skip to content

Less template literal and string literal reduction - #56165

Closed
Pranav Senthilnathan (PranavSenthilnathan) wants to merge 3 commits into
mainfrom
pranas/template-literal-reduction
Closed

Less template literal and string literal reduction#56165
Pranav Senthilnathan (PranavSenthilnathan) wants to merge 3 commits into
mainfrom
pranas/template-literal-reduction

Conversation

@PranavSenthilnathan

@PranavSenthilnathanPranav Senthilnathan (PranavSenthilnathan) commented Oct 20, 2023

Copy link
Copy Markdown
Member

Checking perf impact of the naive change.

Fixes#56081

@typescript-botTypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 20, 2023
@PranavSenthilnathan

Copy link
Copy Markdown
MemberAuthor

@jakebailey

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) perf test this
TypeScript Bot (@typescript-bot) user test this

I don't think you're on the author list so that doesn't work, though maybe it's some other list that does it.

@typescript-bot

TypeScript Bot (typescript-bot) commented Oct 20, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the regular perf test suite on this PR at 8e76d16. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Oct 20, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based user code test suite on this PR at 8e76d16. You can monitor the build here.

Update: The results are in!

@jakebailey

Copy link
Copy Markdown
Member

What test case are you looking at for this? #52345?

@typescript-botTypeScript Bot (typescript-bot) added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 20, 2023
@PranavSenthilnathan

Copy link
Copy Markdown
MemberAuthor

Updated comment. It's #56081

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the user test suite comparing main and refs/pull/56165/merge:

There were infrastructure failures potentially unrelated to your change:

  • 3 instances of "Package install failed"
  • 1 instance of "Unknown failure"

Otherwise...

Everything looks good!

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

Compiler

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Angular - node (v18.15.0, x64)
Memory used295,123k (± 0.01%)295,097k (± 0.01%)~295,046k295,160kp=0.199 n=6
Parse Time2.63s (± 0.57%)2.63s (± 0.29%)~2.62s2.64sp=0.503 n=6
Bind Time0.84s (± 0.90%)0.84s (± 1.30%)~0.83s0.85sp=0.865 n=6
Check Time8.06s (± 0.31%)8.06s (± 0.29%)~8.03s8.10sp=0.935 n=6
Emit Time7.08s (± 0.12%)7.06s (± 0.29%)~7.03s7.08sp=0.119 n=6
Total Time18.61s (± 0.14%)18.58s (± 0.19%)~18.53s18.63sp=0.076 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used192,610k (± 1.56%)193,587k (± 1.64%)~190,674k196,533kp=0.471 n=6
Parse Time1.35s (± 1.30%)1.36s (± 1.27%)~1.34s1.38sp=0.278 n=6
Bind Time0.73s (± 0.00%)0.73s (± 0.00%)~0.73s0.73sp=1.000 n=6
Check Time9.18s (± 0.17%)9.19s (± 0.34%)~9.13s9.21sp=0.413 n=6
Emit Time2.65s (± 0.52%)2.63s (± 0.40%)~2.62s2.65sp=0.161 n=6
Total Time13.90s (± 0.22%)13.91s (± 0.17%)~13.88s13.93sp=0.569 n=6
Monaco - node (v18.15.0, x64)
Memory used347,301k (± 0.00%)347,299k (± 0.00%)~347,276k347,314kp=0.630 n=6
Parse Time2.46s (± 0.33%)2.46s (± 0.60%)~2.44s2.48sp=0.508 n=6
Bind Time0.94s (± 0.43%)0.94s (± 0.00%)~0.94s0.94sp=0.405 n=6
Check Time6.92s (± 0.33%)6.92s (± 0.32%)~6.90s6.96sp=1.000 n=6
Emit Time4.02s (± 0.44%)4.02s (± 0.27%)~4.01s4.04sp=0.806 n=6
Total Time14.34s (± 0.17%)14.34s (± 0.27%)~14.30s14.40sp=0.935 n=6
TFS - node (v18.15.0, x64)
Memory used302,535k (± 0.01%)302,526k (± 0.01%)~302,486k302,541kp=0.688 n=6
Parse Time2.00s (± 0.73%)2.00s (± 0.52%)~1.99s2.02sp=1.000 n=6
Bind Time1.00s (± 0.00%)1.01s (± 1.46%)~0.99s1.03sp=0.295 n=6
Check Time6.26s (± 0.42%)6.25s (± 0.54%)~6.22s6.31sp=0.573 n=6
Emit Time3.56s (± 0.58%)3.56s (± 0.50%)~3.53s3.58sp=0.570 n=6
Total Time12.81s (± 0.17%)12.82s (± 0.17%)~12.79s12.85sp=0.571 n=6
material-ui - node (v18.15.0, x64)
Memory used470,481k (± 0.00%)470,483k (± 0.00%)~470,453k470,504kp=0.688 n=6
Parse Time2.58s (± 0.53%)2.58s (± 0.62%)~2.56s2.61sp=0.315 n=6
Bind Time1.00s (± 1.21%)0.99s (± 1.22%)~0.98s1.01sp=0.680 n=6
Check Time16.62s (± 0.34%)16.59s (± 0.29%)~16.52s16.66sp=0.470 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time20.19s (± 0.38%)20.17s (± 0.28%)~20.08s20.25sp=0.575 n=6
xstate - node (v18.15.0, x64)
Memory used512,617k (± 0.01%)512,621k (± 0.01%)~512,558k512,726kp=0.936 n=6
Parse Time3.27s (± 0.42%)3.28s (± 0.32%)~3.26s3.29sp=0.241 n=6
Bind Time1.55s (± 0.26%)1.55s (± 0.53%)~1.53s1.55sp=0.218 n=6
Check Time2.88s (± 1.49%)2.86s (± 1.01%)~2.82s2.89sp=0.574 n=6
Emit Time0.08s (± 0.00%)0.08s (± 0.00%)~0.08s0.08sp=1.000 n=6
Total Time7.77s (± 0.68%)7.75s (± 0.38%)~7.70s7.78sp=0.521 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

tsserver

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,376ms (± 1.44%)2,360ms (± 1.26%)~2,335ms2,412msp=0.575 n=6
Req 2 - geterr5,339ms (± 1.13%)5,446ms (± 1.13%)+107ms (+ 2.01%)5,322ms5,483msp=0.016 n=6
Req 3 - references326ms (± 0.19%)327ms (± 0.81%)~325ms332msp=0.557 n=6
Req 4 - navto278ms (± 1.26%)273ms (± 1.29%)~271ms280msp=0.115 n=6
Req 5 - completionInfo count1,356 (± 0.00%)1,356 (± 0.00%)~1,3561,356p=1.000 n=6
Req 5 - completionInfo78ms (± 4.93%)86ms (± 7.24%)~74ms90msp=0.075 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,490ms (± 1.06%)2,488ms (± 1.07%)~2,455ms2,533msp=0.810 n=6
Req 2 - geterr4,113ms (± 2.06%)4,127ms (± 1.78%)~4,055ms4,219msp=0.378 n=6
Req 3 - references340ms (± 1.61%)341ms (± 1.74%)~333ms348msp=0.934 n=6
Req 4 - navto284ms (± 0.29%)285ms (± 0.48%)~283ms287msp=0.865 n=6
Req 5 - completionInfo count1,518 (± 0.00%)1,518 (± 0.00%)~1,5181,518p=1.000 n=6
Req 5 - completionInfo84ms (± 6.63%)84ms (± 6.77%)~77ms89msp=1.000 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,592ms (± 0.97%)2,595ms (± 0.63%)~2,569ms2,619msp=0.748 n=6
Req 2 - geterr1,672ms (± 2.68%)1,711ms (± 1.18%)~1,686ms1,734msp=0.173 n=6
Req 3 - references113ms (± 9.51%)121ms (± 8.93%)~105ms128msp=0.220 n=6
Req 4 - navto359ms (± 0.45%)362ms (± 0.77%)~359ms367msp=0.101 n=6
Req 5 - completionInfo count2,073 (± 0.00%)2,073 (± 0.00%)~2,0732,073p=1.000 n=6
Req 5 - completionInfo302ms (± 2.18%)304ms (± 1.93%)~297ms313msp=0.520 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • CompilerTSServer - node (v18.15.0, x64)
  • Compiler-UnionsTSServer - node (v18.15.0, x64)
  • xstateTSServer - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Startup

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
tsc-startup - node (v18.15.0, x64)
Execution time152.53ms (± 0.18%)152.36ms (± 0.19%)-0.17ms (- 0.11%)151.08ms156.17msp=0.000 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time227.29ms (± 0.15%)227.18ms (± 0.16%)-0.10ms (- 0.05%)225.95ms232.25msp=0.001 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time228.45ms (± 0.17%)228.35ms (± 0.15%)~226.47ms232.80msp=0.077 n=600
typescript-startup - node (v18.15.0, x64)
Execution time228.73ms (± 0.15%)228.79ms (± 0.15%)~227.44ms231.94msp=0.092 n=600
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • tsc-startup - node (v18.15.0, x64)
  • tsserver-startup - node (v18.15.0, x64)
  • tsserverlibrary-startup - node (v18.15.0, x64)
  • typescript-startup - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@PranavSenthilnathan

Copy link
Copy Markdown
MemberAuthor

So this is not the solution since this was an intentional feature introduced in #41276. But I think there should probably be some middle ground between reducing eagerly during union creation and lazily when subtype reduction is performed. Maybe just doing subtype reduction when checking variable declaration nodes?

@jakebailey

Copy link
Copy Markdown
Member

The main problem is that if we don't always reduce, then two unions that are actually the same thing can be present at once, which means you can't fast check for equality and such (maybe some other invariants).

@typescript-bot

Copy link
Copy Markdown
Contributor

With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies).

Next steps for PRs:

  • For crash bugfixes or language service improvements, PRs are currently accepted at the typescript-go repo
  • Changes to type system behavior should wait until after 7.0, at which point mainline TypeScript development will resume in this repository with the Go codebase
  • Library file updates (lib.d.ts etc) continue to live in this repo or the DOM Generator repo as appropriate

@jakebailey
Jake Bailey (jakebailey) deleted the pranas/template-literal-reduction branch August 28, 2026 19:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory Leak / Infinite Recursion. TS hangs, TSC never finishes, VSCode and Intellisense dies.

4 participants

@PranavSenthilnathan@jakebailey@typescript-bot@RyanCavanaugh