Skip to content

Fixed crash when cross-file reusing nodes for class member snippet completions #58216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 4, 2024

Conversation

Andarist
Copy link
Contributor

fixes #58205

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 16, 2024
@DanielRosenwasser
Copy link
Member

Uhhhhh, kinda nervous about walking the tree on every literal.

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 16, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
perf test this ✅ Started 👀 Results

@DanielRosenwasser
Copy link
Member

I think the right thing to do is do a clone of the node from wherever this comes from. Especially since there are other bugs that often come up like when a node is reused within the tree and the formatter tries to overwrite it.

@gabritto
Copy link
Member

I think the right thing to do is do a clone of the node from wherever this comes from. Especially since there are other bugs that often come up like when a node is reused within the tree and the formatter tries to overwrite it.

Yep, if the underlying problem is that node re-use messes up the formatter calls in the services layer, we seem to have many occurrences of that. @armanio123 is working on a fix for #57924 (comment), which is an instance of this, although that crash is caused by re-use of a question token instead of a literal.

@typescript-bot
Copy link
Collaborator

@DanielRosenwasser
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v18.15.0, x64)
Memory used 297,011k (± 0.00%) 297,012k (± 0.01%) ~ 296,983k 297,030k p=0.810 n=6
Parse Time 3.27s (± 0.50%) 3.27s (± 0.57%) ~ 3.25s 3.30s p=1.000 n=6
Bind Time 0.98s (± 0.42%) 0.98s (± 0.56%) ~ 0.98s 0.99s p=0.282 n=6
Check Time 9.80s (± 0.57%) 9.83s (± 0.41%) ~ 9.78s 9.88s p=0.333 n=6
Emit Time 8.43s (± 0.44%) 8.46s (± 0.42%) ~ 8.40s 8.51s p=0.170 n=6
Total Time 22.48s (± 0.28%) 22.54s (± 0.25%) ~ 22.47s 22.63s p=0.092 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 192,955k (± 0.99%) 193,598k (± 1.01%) ~ 191,747k 195,433k p=0.575 n=6
Parse Time 2.02s (± 2.03%) 2.03s (± 1.10%) ~ 2.00s 2.06s p=0.568 n=6
Bind Time 1.07s (± 0.70%) 1.08s (± 0.78%) ~ 1.06s 1.08s p=0.432 n=6
Check Time 14.03s (± 0.52%) 14.07s (± 0.51%) ~ 14.00s 14.18s p=0.810 n=6
Emit Time 3.95s (± 3.94%) 3.87s (± 0.99%) ~ 3.85s 3.95s p=0.227 n=6
Total Time 21.07s (± 0.51%) 21.04s (± 0.35%) ~ 20.96s 21.15s p=0.744 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,749,560k (± 0.00%) 1,749,595k (± 0.00%) ~ 1,749,568k 1,749,632k p=0.109 n=6
Parse Time 8.11s (± 0.72%) 8.13s (± 0.48%) ~ 8.07s 8.17s p=0.747 n=6
Bind Time 2.74s (± 0.50%) 2.73s (± 0.65%) ~ 2.71s 2.76s p=0.413 n=6
Check Time 66.71s (± 0.33%) 66.72s (± 0.29%) ~ 66.49s 67.05s p=1.000 n=6
Emit Time 0.16s (± 0.00%) 0.16s (± 4.99%) ~ 0.16s 0.18s p=0.405 n=6
Total Time 77.71s (± 0.30%) 77.74s (± 0.24%) ~ 77.52s 78.03s p=0.689 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,306,621k (± 0.03%) 2,307,961k (± 0.05%) +1,340k (+ 0.06%) 2,306,847k 2,309,890k p=0.031 n=6
Parse Time 7.32s (± 0.38%) 7.37s (± 0.91%) ~ 7.28s 7.48s p=0.093 n=6
Bind Time 2.73s (± 0.49%) 2.74s (± 0.65%) ~ 2.71s 2.76s p=0.333 n=6
Check Time 49.31s (± 0.19%) 49.36s (± 0.98%) ~ 48.84s 49.95s p=1.000 n=6
Emit Time 4.01s (± 3.53%) 3.92s (± 3.94%) ~ 3.79s 4.20s p=0.173 n=6
Total Time 63.38s (± 0.12%) 63.40s (± 0.74%) ~ 62.84s 64.05s p=0.810 n=6
self-build-src-public-api - node (v18.15.0, x64)
Memory used 2,382,439k (± 0.04%) 2,382,370k (± 0.03%) ~ 2,381,168k 2,382,982k p=0.936 n=6
Parse Time 6.16s (± 0.68%) 6.15s (± 0.82%) ~ 6.08s 6.23s p=0.630 n=6
Bind Time 2.07s (± 0.25%) 2.07s (± 1.01%) ~ 2.04s 2.09s p=1.000 n=6
Check Time 40.16s (± 0.23%) 40.17s (± 0.14%) ~ 40.10s 40.24s p=0.936 n=6
Emit Time 3.28s (± 3.72%) 3.21s (± 2.48%) ~ 3.09s 3.31s p=0.298 n=6
Total Time 51.69s (± 0.37%) 51.62s (± 0.25%) ~ 51.43s 51.78s p=0.378 n=6
self-compiler - node (v18.15.0, x64)
Memory used 419,473k (± 0.01%) 419,458k (± 0.01%) ~ 419,396k 419,486k p=0.810 n=6
Parse Time 4.19s (± 2.05%) 4.21s (± 0.81%) ~ 4.14s 4.23s p=0.567 n=6
Bind Time 1.60s (± 3.79%) 1.57s (± 1.61%) ~ 1.54s 1.60s p=0.368 n=6
Check Time 22.37s (± 0.31%) 22.28s (± 0.44%) ~ 22.13s 22.44s p=0.171 n=6
Emit Time 1.71s (± 1.79%) 1.72s (± 0.48%) ~ 1.72s 1.74s p=0.406 n=6
Total Time 29.88s (± 0.43%) 29.78s (± 0.38%) ~ 29.61s 29.94s p=0.173 n=6
ts-pre-modules - node (v18.15.0, x64)
Memory used 368,971k (± 0.02%) 369,075k (± 0.02%) +104k (+ 0.03%) 369,019k 369,196k p=0.045 n=6
Parse Time 2.44s (± 0.66%) 2.45s (± 0.92%) ~ 2.43s 2.48s p=0.357 n=6
Bind Time 1.31s (± 1.12%) 1.36s (± 2.11%) +0.05s (+ 3.94%) 1.32s 1.41s p=0.012 n=6
Check Time 13.29s (± 0.37%) 13.32s (± 0.32%) ~ 13.25s 13.38s p=0.332 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 17.04s (± 0.36%) 17.14s (± 0.42%) +0.10s (+ 0.58%) 17.04s 17.22s p=0.045 n=6
vscode - node (v18.15.0, x64)
Memory used 2,914,815k (± 0.00%) 2,914,780k (± 0.00%) ~ 2,914,657k 2,914,846k p=0.378 n=6
Parse Time 11.24s (± 0.28%) 11.23s (± 0.19%) ~ 11.21s 11.26s p=0.685 n=6
Bind Time 3.41s (± 0.30%) 3.40s (± 0.24%) ~ 3.39s 3.41s p=0.121 n=6
Check Time 62.68s (± 0.67%) 62.69s (± 0.43%) ~ 62.35s 62.97s p=1.000 n=6
Emit Time 16.56s (± 0.41%) 17.16s (± 8.95%) ~ 16.44s 20.29s p=0.936 n=6
Total Time 93.90s (± 0.49%) 94.48s (± 1.81%) ~ 93.44s 97.91s p=0.936 n=6
webpack - node (v18.15.0, x64)
Memory used 409,504k (± 0.01%) 409,444k (± 0.01%) ~ 409,381k 409,524k p=0.128 n=6
Parse Time 4.83s (± 1.01%) 4.80s (± 0.84%) ~ 4.74s 4.85s p=0.295 n=6
Bind Time 2.02s (± 0.67%) 2.03s (± 0.68%) ~ 2.02s 2.06s p=0.241 n=6
Check Time 21.04s (± 0.34%) 21.04s (± 0.34%) ~ 20.95s 21.12s p=0.872 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 27.90s (± 0.31%) 27.88s (± 0.37%) ~ 27.77s 28.01s p=0.872 n=6
xstate-main - node (v18.15.0, x64)
Memory used 458,756k (± 0.01%) 458,746k (± 0.02%) ~ 458,584k 458,860k p=0.936 n=6
Parse Time 3.23s (± 0.57%) 3.22s (± 0.44%) ~ 3.20s 3.24s p=0.514 n=6
Bind Time 1.17s (± 0.35%) 1.17s (± 0.47%) ~ 1.17s 1.18s p=0.282 n=6
Check Time 18.18s (± 0.69%) 18.08s (± 0.62%) ~ 17.94s 18.22s p=0.172 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.58s (± 0.56%) 22.47s (± 0.49%) ~ 22.35s 22.62s p=0.199 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)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 3,506ms (± 0.67%) 2,825ms (± 1.00%) 🟩-681ms (-19.43%) 2,783ms 2,861ms p=0.005 n=6
Req 2 - geterr 7,677ms (± 1.57%) 6,185ms (± 1.39%) 🟩-1,492ms (-19.43%) 6,097ms 6,340ms p=0.005 n=6
Req 3 - references 426ms (± 0.55%) 349ms (± 1.86%) 🟩-78ms (-18.19%) 344ms 361ms p=0.005 n=6
Req 4 - navto 342ms (± 2.57%) 277ms (± 1.67%) 🟩-65ms (-19.00%) 270ms 281ms p=0.005 n=6
Req 5 - completionInfo count 1,357 (± 0.00%) 1,357 (± 0.00%) ~ 1,357 1,357 p=1.000 n=6
Req 5 - completionInfo 116ms (± 3.93%) 99ms (±10.26%) 🟩-17ms (-14.84%) 92ms 113ms p=0.011 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 3,655ms (± 0.75%) 3,690ms (± 0.61%) +36ms (+ 0.98%) 3,651ms 3,711ms p=0.037 n=6
Req 2 - geterr 5,782ms (± 1.98%) 5,668ms (± 0.46%) -114ms (- 1.97%) 5,639ms 5,713ms p=0.045 n=6
Req 3 - references 448ms (± 1.93%) 455ms (± 0.31%) ~ 453ms 457ms p=0.466 n=6
Req 4 - navto 336ms (± 2.93%) 336ms (± 0.25%) ~ 335ms 337ms p=0.369 n=6
Req 5 - completionInfo count 1,519 (± 0.00%) 1,519 (± 0.00%) ~ 1,519 1,519 p=1.000 n=6
Req 5 - completionInfo 115ms (± 7.64%) 123ms (± 1.40%) ~ 121ms 125ms p=0.061 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 3,933ms (± 0.58%) 3,928ms (± 0.38%) ~ 3,904ms 3,943ms p=1.000 n=6
Req 2 - geterr 2,220ms (± 1.03%) 2,194ms (± 0.90%) ~ 2,162ms 2,216ms p=0.109 n=6
Req 3 - references 182ms (± 1.01%) 184ms (± 2.51%) ~ 180ms 192ms p=0.506 n=6
Req 4 - navto 531ms (± 1.62%) 529ms (± 1.15%) ~ 522ms 538ms p=0.810 n=6
Req 5 - completionInfo count 2,079 (± 0.00%) 2,079 (± 0.00%) ~ 2,079 2,079 p=1.000 n=6
Req 5 - completionInfo 447ms (± 1.29%) 452ms (± 1.62%) ~ 439ms 460ms p=0.228 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)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v18.15.0, x64)
Execution time 223.20ms (± 0.17%) 223.08ms (± 0.19%) -0.12ms (- 0.05%) 220.85ms 228.38ms p=0.001 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 341.90ms (± 0.29%) 341.91ms (± 0.30%) ~ 333.67ms 349.95ms p=0.598 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 223.87ms (± 0.16%) 223.82ms (± 0.14%) ~ 222.51ms 229.15ms p=0.311 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 274.12ms (± 0.28%) 274.03ms (± 0.29%) ~ 266.97ms 278.07ms p=0.123 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)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@gabritto
Copy link
Member

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 17, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
perf test this ✅ Started 👀 Results

@typescript-bot
Copy link
Collaborator

@gabritto
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v18.15.0, x64)
Memory used 297,017k (± 0.01%) 297,014k (± 0.01%) ~ 296,939k 297,043k p=0.688 n=6
Parse Time 3.25s (± 0.54%) 3.27s (± 0.37%) ~ 3.26s 3.29s p=0.188 n=6
Bind Time 0.98s (± 0.64%) 0.98s (± 0.42%) ~ 0.98s 0.99s p=0.673 n=6
Check Time 9.83s (± 0.40%) 9.83s (± 0.18%) ~ 9.81s 9.86s p=1.000 n=6
Emit Time 8.44s (± 0.47%) 8.43s (± 0.49%) ~ 8.38s 8.47s p=0.745 n=6
Total Time 22.50s (± 0.29%) 22.51s (± 0.18%) ~ 22.46s 22.56s p=0.459 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 192,987k (± 0.95%) 192,401k (± 0.79%) ~ 191,742k 195,517k p=0.109 n=6
Parse Time 2.02s (± 1.07%) 2.03s (± 0.79%) ~ 2.01s 2.05s p=0.871 n=6
Bind Time 1.07s (± 1.09%) 1.08s (± 0.51%) ~ 1.07s 1.08s p=0.498 n=6
Check Time 14.04s (± 0.21%) 14.02s (± 0.22%) ~ 13.98s 14.07s p=0.190 n=6
Emit Time 3.88s (± 0.64%) 3.85s (± 0.64%) ~ 3.82s 3.89s p=0.089 n=6
Total Time 21.02s (± 0.16%) 20.97s (± 0.17%) ~ 20.92s 21.01s p=0.064 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,751,076k (± 0.00%) 1,751,096k (± 0.00%) ~ 1,751,075k 1,751,136k p=0.297 n=6
Parse Time 10.00s (± 0.41%) 9.92s (± 0.70%) ~ 9.87s 10.06s p=0.064 n=6
Bind Time 3.35s (± 0.44%) 3.33s (± 0.31%) ~ 3.32s 3.35s p=0.084 n=6
Check Time 81.69s (± 0.29%) 81.73s (± 0.34%) ~ 81.29s 82.08s p=0.630 n=6
Emit Time 0.20s (± 0.00%) 0.20s (± 2.06%) ~ 0.19s 0.20s p=0.405 n=6
Total Time 95.24s (± 0.22%) 95.18s (± 0.30%) ~ 94.71s 95.53s p=1.000 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,307,752k (± 0.04%) 2,307,770k (± 0.06%) ~ 2,305,598k 2,309,478k p=0.810 n=6
Parse Time 7.37s (± 0.44%) 7.42s (± 0.96%) ~ 7.30s 7.49s p=0.173 n=6
Bind Time 2.73s (± 0.61%) 2.73s (± 0.43%) ~ 2.71s 2.74s p=0.936 n=6
Check Time 49.35s (± 0.56%) 49.32s (± 0.53%) ~ 49.13s 49.76s p=0.936 n=6
Emit Time 3.95s (± 1.48%) 3.95s (± 2.32%) ~ 3.84s 4.07s p=0.810 n=6
Total Time 63.40s (± 0.39%) 63.43s (± 0.48%) ~ 63.07s 63.90s p=1.000 n=6
self-build-src-public-api - node (v18.15.0, x64)
Memory used 2,382,122k (± 0.03%) 2,382,247k (± 0.03%) ~ 2,380,950k 2,383,133k p=0.575 n=6
Parse Time 7.61s (± 0.79%) 7.65s (± 0.74%) ~ 7.58s 7.75s p=0.199 n=6
Bind Time 2.52s (± 1.14%) 2.50s (± 0.78%) ~ 2.48s 2.53s p=0.572 n=6
Check Time 49.71s (± 0.59%) 49.80s (± 0.57%) ~ 49.50s 50.19s p=0.575 n=6
Emit Time 4.00s (± 4.94%) 3.96s (± 2.88%) ~ 3.82s 4.13s p=1.000 n=6
Total Time 63.85s (± 0.45%) 63.93s (± 0.42%) ~ 63.61s 64.26s p=0.575 n=6
self-compiler - node (v18.15.0, x64)
Memory used 419,441k (± 0.01%) 419,482k (± 0.01%) ~ 419,413k 419,551k p=0.149 n=6
Parse Time 4.23s (± 0.25%) 4.15s (± 2.34%) -0.08s (- 1.89%) 4.02s 4.21s p=0.009 n=6
Bind Time 1.60s (± 1.81%) 1.64s (± 4.93%) ~ 1.55s 1.75s p=0.520 n=6
Check Time 22.36s (± 0.20%) 22.32s (± 0.43%) ~ 22.23s 22.50s p=0.128 n=6
Emit Time 1.71s (± 1.42%) 1.71s (± 1.19%) ~ 1.69s 1.73s p=0.683 n=6
Total Time 29.89s (± 0.12%) 29.81s (± 0.21%) -0.08s (- 0.25%) 29.76s 29.93s p=0.044 n=6
ts-pre-modules - node (v18.15.0, x64)
Memory used 368,961k (± 0.03%) 368,978k (± 0.01%) ~ 368,922k 369,029k p=0.423 n=6
Parse Time 2.93s (± 1.15%) 2.94s (± 0.85%) ~ 2.90s 2.97s p=0.467 n=6
Bind Time 1.56s (± 0.87%) 1.63s (± 0.92%) 🔻+0.06s (+ 4.05%) 1.60s 1.64s p=0.005 n=6
Check Time 15.67s (± 0.16%) 15.67s (± 0.28%) ~ 15.62s 15.72s p=0.872 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.17s (± 0.12%) 20.25s (± 0.25%) +0.08s (+ 0.39%) 20.17s 20.31s p=0.019 n=6
vscode - node (v18.15.0, x64)
Memory used 2,915,303k (± 0.00%) 2,915,243k (± 0.00%) ~ 2,915,118k 2,915,480k p=0.199 n=6
Parse Time 13.47s (± 0.27%) 13.46s (± 0.56%) ~ 13.41s 13.61s p=0.420 n=6
Bind Time 4.06s (± 0.58%) 4.06s (± 0.13%) ~ 4.06s 4.07s p=0.360 n=6
Check Time 72.48s (± 0.42%) 72.43s (± 0.33%) ~ 72.12s 72.69s p=1.000 n=6
Emit Time 22.74s (± 7.05%) 22.76s (± 6.58%) ~ 19.75s 23.73s p=1.000 n=6
Total Time 112.74s (± 1.45%) 112.72s (± 1.21%) ~ 109.96s 113.44s p=1.000 n=6
webpack - node (v18.15.0, x64)
Memory used 409,350k (± 0.01%) 409,371k (± 0.01%) ~ 409,334k 409,412k p=0.298 n=6
Parse Time 3.26s (± 0.63%) 3.25s (± 1.18%) ~ 3.20s 3.31s p=0.936 n=6
Bind Time 1.38s (± 0.30%) 1.38s (± 0.79%) ~ 1.37s 1.39s p=0.865 n=6
Check Time 14.41s (± 0.23%) 14.38s (± 0.23%) ~ 14.35s 14.42s p=0.076 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 19.05s (± 0.23%) 19.01s (± 0.29%) ~ 18.95s 19.08s p=0.257 n=6
xstate-main - node (v18.15.0, x64)
Memory used 458,960k (± 0.02%) 458,925k (± 0.01%) ~ 458,890k 458,962k p=0.521 n=6
Parse Time 2.68s (± 0.86%) 2.68s (± 0.44%) ~ 2.67s 2.70s p=1.000 n=6
Bind Time 0.98s (± 0.52%) 0.98s (± 0.77%) ~ 0.97s 0.99s p=0.784 n=6
Check Time 15.34s (± 0.30%) 15.33s (± 0.21%) ~ 15.28s 15.36s p=0.624 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 19.00s (± 0.28%) 18.99s (± 0.13%) ~ 18.96s 19.02s p=0.687 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)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,336ms (± 0.91%) 2,342ms (± 0.59%) ~ 2,323ms 2,356ms p=0.810 n=6
Req 2 - geterr 5,128ms (± 1.39%) 5,183ms (± 1.85%) ~ 5,097ms 5,327ms p=0.128 n=6
Req 3 - references 287ms (± 1.85%) 289ms (± 2.50%) ~ 280ms 298ms p=0.747 n=6
Req 4 - navto 228ms (± 0.66%) 227ms (± 0.81%) ~ 224ms 229ms p=0.618 n=6
Req 5 - completionInfo count 1,357 (± 0.00%) 1,357 (± 0.00%) ~ 1,357 1,357 p=1.000 n=6
Req 5 - completionInfo 80ms (± 8.43%) 80ms (± 7.96%) ~ 76ms 92ms p=1.000 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 3,664ms (± 0.86%) 3,687ms (± 0.74%) ~ 3,645ms 3,722ms p=0.230 n=6
Req 2 - geterr 5,764ms (± 1.75%) 5,766ms (± 1.72%) ~ 5,685ms 5,895ms p=1.000 n=6
Req 3 - references 454ms (± 1.65%) 454ms (± 1.57%) ~ 443ms 459ms p=1.000 n=6
Req 4 - navto 338ms (± 0.91%) 342ms (± 1.70%) ~ 338ms 353ms p=0.124 n=6
Req 5 - completionInfo count 1,519 (± 0.00%) 1,519 (± 0.00%) ~ 1,519 1,519 p=1.000 n=6
Req 5 - completionInfo 119ms (± 6.05%) 119ms (± 6.69%) ~ 105ms 124ms p=1.000 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,657ms (± 0.31%) 2,654ms (± 0.34%) ~ 2,637ms 2,661ms p=0.470 n=6
Req 2 - geterr 1,483ms (± 0.91%) 1,488ms (± 0.73%) ~ 1,474ms 1,505ms p=0.873 n=6
Req 3 - references 137ms (± 7.19%) 134ms (± 6.60%) ~ 127ms 147ms p=0.806 n=6
Req 4 - navto 348ms (± 0.43%) 350ms (± 0.52%) ~ 348ms 353ms p=0.117 n=6
Req 5 - completionInfo count 2,079 (± 0.00%) 2,079 (± 0.00%) ~ 2,079 2,079 p=1.000 n=6
Req 5 - completionInfo 297ms (± 0.30%) 297ms (± 0.73%) ~ 295ms 300ms p=0.934 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)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v18.15.0, x64)
Execution time 154.91ms (± 0.16%) 154.93ms (± 0.18%) ~ 153.66ms 157.23ms p=0.736 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 228.46ms (± 0.15%) 228.45ms (± 0.16%) ~ 227.10ms 235.53ms p=0.633 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 224.96ms (± 0.14%) 224.83ms (± 0.15%) -0.12ms (- 0.05%) 223.46ms 231.59ms p=0.000 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 223.69ms (± 0.15%) 223.48ms (± 0.19%) -0.21ms (- 0.09%) 222.07ms 231.41ms p=0.000 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)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@Andarist
Copy link
Contributor Author

So I thought that the node reuse would be desired here. If I understand correctly you are saying that it should never happen here, right? Cloning the original node would likely make it synthesized or something and that would prevent reuse altogether.

@gabritto
Copy link
Member

So I thought that the node reuse would be desired here. If I understand correctly you are saying that it should never happen here, right? Cloning the original node would likely make it synthesized or something and that would prevent reuse altogether.

The current state of the formatter is such that we should never pass it an AST that has node reuse, as that will cause a crash. Other parts of the compiler don't suffer from this problem. I think the emitter doesn't care about node reuse, for the most part.

I think the problem is that the formatter was written to handle actual parse trees, coming from actual files, but we've been using it to format trees we generate for completions.

@DanielRosenwasser
Copy link
Member

I think the problem is that the formatter was written to handle actual parse trees, coming from actual files, but we've been using it to format trees we generate for completions.

Yeah, and I believe the formatter actually mutates the parse trees, so that complicates things too.

@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 20, 2024
@Andarist
Copy link
Contributor Author

I didn't see (yet?) any mutation like that happening. getSynthesizedDeepCloneWorker (that gets already called!) has some specific logic to handle string literals:

if (visited === node) {
// This only happens for leaf nodes - internal nodes always see their children change.
const clone = isStringLiteral(node) ? setOriginalNode(factory.createStringLiteralFromNode(node), node) as Node as T :
isNumericLiteral(node) ? setOriginalNode(factory.createNumericLiteral(node.text, node.numericLiteralFlags), node) as Node as T :
factory.cloneNode(node);
return setTextRange(clone, node);
}

createStringLiteralFromNode sets .textSourceNode on the created node:

node.textSourceNode = sourceNode;

and that gets used here by the emitter in getLiteralTextOfNode:

if (node.kind === SyntaxKind.StringLiteral && (node as StringLiteral).textSourceNode) {

Given all of that It still feels like using the correct source file is the easiest here - but I can change the location where it's looked for because only the code that uses .textSourceNode has to look for that original source file. I pushed out this change - please take a look and let me know what do you think about it. If you like this approach I'll look for more locations using .textSourceNode because I'm pretty sure that this doesn't cover everything right now.

@gabritto
Copy link
Member

gabritto commented Jul 3, 2024

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jul 3, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
perf test this ✅ Started 👀 Results

@gabritto
Copy link
Member

gabritto commented Jul 3, 2024

Some notes for the future:

  1. This is not a bug related to node reuse. The bug is in how the emitter emits literals. A string literal node can be created from another node via createStringLiteralFromNode, and this newly string literal node will have a textSourceNode that points to the node it was created from. When the emitter tries to emit this string literal, it will attempt to get the text from this textSourceNode, because we prefer to get the original text as it was written, rather than the normalized version we store in the node's text property. But when it tries to get the text from this text source node, the emitter assumes the text source node's file is the current file, which is not necessarily true, and thus it gets the text from a different file, so the text is wrong.

  2. In this PR, we'll only ever look up a node's source file when we are emitting a string literal that has the textSourceNode property, i.e., nodes created by transformers or services, so that doesn't seem very impactful for performance in general.

@typescript-bot
Copy link
Collaborator

@gabritto
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 30 30 ~ ~ ~ p=1.000 n=6
Symbols 62,153 62,153 ~ ~ ~ p=1.000 n=6
Types 50,242 50,242 ~ ~ ~ p=1.000 n=6
Memory used 192,831k (± 0.74%) 193,947k (± 1.00%) ~ 192,136k 195,766k p=0.298 n=6
Parse Time 1.58s (± 0.65%) 1.58s (± 1.04%) ~ 1.55s 1.60s p=0.802 n=6
Bind Time 0.85s (± 1.23%) 0.85s (± 1.60%) ~ 0.84s 0.87s p=0.804 n=6
Check Time 11.24s (± 0.37%) 11.20s (± 0.44%) ~ 11.12s 11.26s p=0.199 n=6
Emit Time 3.30s (± 0.49%) 3.27s (± 0.57%) ~ 3.25s 3.30s p=0.089 n=6
Total Time 16.97s (± 0.33%) 16.90s (± 0.28%) -0.07s (- 0.40%) 16.83s 16.97s p=0.044 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,114 944,114 ~ ~ ~ p=1.000 n=6
Types 407,050 407,050 ~ ~ ~ p=1.000 n=6
Memory used 1,218,366k (± 0.00%) 1,218,352k (± 0.00%) ~ 1,218,264k 1,218,395k p=0.747 n=6
Parse Time 6.64s (± 0.49%) 6.64s (± 0.64%) ~ 6.57s 6.68s p=1.000 n=6
Bind Time 1.87s (± 0.55%) 1.86s (± 0.53%) ~ 1.85s 1.87s p=0.203 n=6
Check Time 30.69s (± 0.47%) 30.65s (± 0.35%) ~ 30.54s 30.81s p=0.748 n=6
Emit Time 13.57s (± 0.27%) 13.57s (± 0.31%) ~ 13.51s 13.62s p=1.000 n=6
Total Time 52.76s (± 0.26%) 52.72s (± 0.25%) ~ 52.53s 52.88s p=0.689 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,129,960 2,129,960 ~ ~ ~ p=1.000 n=6
Types 927,566 927,566 ~ ~ ~ p=1.000 n=6
Memory used 2,104,713k (± 0.01%) 2,104,729k (± 0.00%) ~ 2,104,612k 2,104,837k p=0.810 n=6
Parse Time 6.61s (± 0.18%) 6.58s (± 0.38%) ~ 6.56s 6.63s p=0.053 n=6
Bind Time 2.32s (± 0.67%) 2.32s (± 0.63%) ~ 2.31s 2.34s p=1.000 n=6
Check Time 70.64s (± 0.41%) 70.58s (± 0.29%) ~ 70.27s 70.79s p=0.810 n=6
Emit Time 0.14s (± 3.77%) 0.13s (± 3.87%) ~ 0.13s 0.14s p=0.311 n=6
Total Time 79.71s (± 0.37%) 79.61s (± 0.24%) ~ 79.34s 79.80s p=0.689 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,496 1,225,497 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 261,459 261,459 ~ ~ ~ p=1.000 n=6
Memory used 2,342,174k (± 0.04%) 2,342,723k (± 0.02%) ~ 2,342,019k 2,343,201k p=0.298 n=6
Parse Time 6.04s (± 0.22%) 5.99s (± 0.81%) ~ 5.92s 6.05s p=0.065 n=6
Bind Time 2.26s (± 0.86%) 2.26s (± 0.99%) ~ 2.23s 2.29s p=0.936 n=6
Check Time 40.24s (± 0.31%) 40.17s (± 0.29%) ~ 40.07s 40.34s p=0.230 n=6
Emit Time 3.06s (± 1.88%) 2.99s (± 3.81%) ~ 2.90s 3.21s p=0.128 n=6
Total Time 51.61s (± 0.32%) 51.43s (± 0.23%) ~ 51.25s 51.58s p=0.093 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,496 1,225,497 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 261,459 261,459 ~ ~ ~ p=1.000 n=6
Memory used 2,416,221k (± 0.03%) 2,416,023k (± 0.03%) ~ 2,415,159k 2,417,384k p=0.471 n=6
Parse Time 7.73s (± 0.75%) 7.75s (± 0.52%) ~ 7.70s 7.81s p=0.298 n=6
Bind Time 2.50s (± 1.17%) 2.50s (± 0.90%) ~ 2.48s 2.54s p=0.809 n=6
Check Time 50.17s (± 0.23%) 50.11s (± 0.32%) ~ 49.85s 50.27s p=0.468 n=6
Emit Time 3.95s (± 4.59%) 3.90s (± 3.35%) ~ 3.75s 4.13s p=0.748 n=6
Total Time 64.36s (± 0.42%) 64.27s (± 0.42%) ~ 63.80s 64.63s p=0.575 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,194 258,195 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 104,737 104,737 ~ ~ ~ p=1.000 n=6
Memory used 427,498k (± 0.01%) 427,491k (± 0.03%) ~ 427,357k 427,677k p=0.471 n=6
Parse Time 3.29s (± 0.58%) 3.27s (± 0.53%) ~ 3.25s 3.30s p=0.195 n=6
Bind Time 1.31s (± 1.40%) 1.32s (± 1.00%) ~ 1.30s 1.33s p=0.197 n=6
Check Time 17.81s (± 0.24%) 17.87s (± 0.52%) ~ 17.73s 17.96s p=0.297 n=6
Emit Time 1.24s (± 1.41%) 1.25s (± 0.65%) ~ 1.24s 1.26s p=0.408 n=6
Total Time 23.65s (± 0.19%) 23.71s (± 0.35%) ~ 23.60s 23.80s p=0.229 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,734 ~ ~ ~ p=1.000 n=6
Memory used 369,573k (± 0.03%) 369,500k (± 0.03%) ~ 369,398k 369,653k p=0.298 n=6
Parse Time 2.29s (± 0.60%) 2.30s (± 0.71%) ~ 2.28s 2.32s p=0.328 n=6
Bind Time 1.33s (± 1.40%) 1.33s (± 1.16%) ~ 1.32s 1.36s p=0.803 n=6
Check Time 13.16s (± 0.40%) 13.16s (± 0.31%) ~ 13.09s 13.20s p=1.000 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 16.79s (± 0.27%) 16.80s (± 0.30%) ~ 16.73s 16.86s p=1.000 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,881,128 2,881,128 ~ ~ ~ p=1.000 n=6
Types 975,510 975,510 ~ ~ ~ p=1.000 n=6
Memory used 3,044,004k (± 0.00%) 3,043,983k (± 0.00%) ~ 3,043,909k 3,044,051k p=0.689 n=6
Parse Time 13.70s (± 0.52%) 13.69s (± 0.33%) ~ 13.64s 13.76s p=0.808 n=6
Bind Time 4.25s (± 2.59%) 4.26s (± 2.53%) ~ 4.15s 4.37s p=0.519 n=6
Check Time 72.79s (± 0.20%) 73.28s (± 0.99%) ~ 72.62s 74.64s p=0.173 n=6
Emit Time 24.03s (± 0.44%) 23.75s (± 2.31%) ~ 22.66s 24.12s p=0.149 n=6
Total Time 114.77s (± 0.12%) 114.99s (± 0.32%) ~ 114.47s 115.38s p=0.261 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,117 267,117 ~ ~ ~ p=1.000 n=6
Types 108,775 108,775 ~ ~ ~ p=1.000 n=6
Memory used 411,658k (± 0.03%) 411,590k (± 0.03%) ~ 411,475k 411,799k p=0.471 n=6
Parse Time 4.70s (± 0.32%) 4.70s (± 0.47%) ~ 4.68s 4.74s p=0.677 n=6
Bind Time 2.08s (± 0.61%) 2.08s (± 0.64%) ~ 2.06s 2.10s p=1.000 n=6
Check Time 20.75s (± 0.36%) 20.80s (± 0.43%) ~ 20.68s 20.94s p=0.378 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 27.53s (± 0.25%) 27.58s (± 0.27%) ~ 27.46s 27.69s p=0.378 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 525,251 525,251 ~ ~ ~ p=1.000 n=6
Types 178,574 178,574 ~ ~ ~ p=1.000 n=6
Memory used 462,868k (± 0.08%) 462,677k (± 0.10%) ~ 462,339k 463,321k p=0.689 n=6
Parse Time 2.65s (± 0.31%) 2.64s (± 0.63%) ~ 2.62s 2.66s p=0.677 n=6
Bind Time 0.98s 0.97s (± 0.56%) ~ 0.97s 0.98s p=0.071 n=6
Check Time 15.15s (± 0.27%) 15.14s (± 0.44%) ~ 15.07s 15.24s p=0.573 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 18.78s (± 0.17%) 18.76s (± 0.30%) ~ 18.70s 18.83s p=0.520 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,875ms (± 9.45%) 2,766ms (± 0.58%) ~ 2,751ms 2,791ms p=0.936 n=6
Req 2 - geterr 7,146ms (± 7.02%) 6,474ms (±10.44%) ~ 6,008ms 7,373ms p=0.093 n=6
Req 3 - references 345ms (±11.03%) 394ms (± 0.44%) ~ 391ms 396ms p=0.059 n=6
Req 4 - navto 310ms (±10.53%) 339ms (± 0.63%) ~ 336ms 341ms p=0.157 n=6
Req 5 - completionInfo count 1,357 1,357 ~ ~ ~ p=1.000 n=6
Req 5 - completionInfo 127ms (± 7.71%) 133ms (± 2.30%) 🔻+6ms (+ 4.59%) 131ms 139ms p=0.024 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 2,930ms (± 1.06%) 2,945ms (± 1.26%) ~ 2,873ms 2,975ms p=0.229 n=6
Req 2 - geterr 4,878ms (±11.71%) 4,695ms (± 9.43%) ~ 4,503ms 5,600ms p=0.936 n=6
Req 3 - references 362ms (±11.04%) 402ms (± 7.57%) ~ 340ms 417ms p=0.061 n=6
Req 4 - navto 305ms (± 9.86%) 336ms (± 2.66%) 🔻+32ms (+10.40%) 319ms 345ms p=0.041 n=6
Req 5 - completionInfo count 1,519 1,519 ~ ~ ~ p=1.000 n=6
Req 5 - completionInfo 106ms (± 7.60%) 104ms (± 7.06%) ~ 90ms 110ms p=0.317 n=6
xstate-main-1-tsserver - node (v18.15.0, x64)
Req 1 - updateOpen 6,226ms (± 0.33%) 6,223ms (± 0.35%) ~ 6,194ms 6,246ms p=0.810 n=6
Req 2 - geterr 1,641ms (± 8.11%) 1,645ms (± 7.86%) ~ 1,382ms 1,709ms p=0.688 n=6
Req 3 - references 114ms (± 4.99%) 119ms (± 6.65%) ~ 111ms 128ms p=0.191 n=6
Req 4 - navto 595ms (± 1.71%) 593ms (± 2.01%) ~ 578ms 606ms p=0.688 n=6
Req 5 - completionInfo count 3,413 3,413 ~ ~ ~ p=1.000 n=6
Req 5 - completionInfo 1,248ms (± 1.88%) 1,265ms (± 2.27%) ~ 1,230ms 1,303ms p=0.630 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)
  • xstate-main-1-tsserver - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v18.15.0, x64)
Execution time 157.09ms (± 0.17%) 157.07ms (± 0.19%) ~ 156.10ms 160.38ms p=0.225 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 230.85ms (± 0.12%) 231.04ms (± 0.18%) +0.19ms (+ 0.08%) 229.57ms 241.52ms p=0.000 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 337.74ms (± 0.30%) 337.99ms (± 0.30%) +0.25ms (+ 0.07%) 330.19ms 347.29ms p=0.001 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 225.63ms (± 0.15%) 226.17ms (± 0.80%) +0.54ms (+ 0.24%) 224.45ms 257.25ms p=0.000 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)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@gabritto gabritto merged commit 3163fe7 into microsoft:main Jul 4, 2024
28 checks passed
@Andarist Andarist deleted the fix/class-member-completion-crash branch July 4, 2024 21:54
@sandersn sandersn removed this from PR Backlog Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debug Failure. False expression: Token end is child end
5 participants