Skip to content
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

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

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.

@sandersn sandersn added this to Not started in PR Backlog May 7, 2024
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
PR Backlog
  
Not started
Development

Successfully merging this pull request may close these issues.

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