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

Narrow object property when key is a variable #57274

Closed
wants to merge 2 commits into from

Conversation

Zzzen
Copy link
Contributor

@Zzzen Zzzen commented Feb 2, 2024

Fixes #56389

This is a rapid prototype, and I would like to assess whether this change has any impact on performance. Would anyone help run the perf suite?

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Feb 2, 2024
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #56389. If you can get it accepted, this PR will have a better chance of being reviewed.

@jakebailey
Copy link
Member

@typescript-bot test top200
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 2, 2024

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 58a3693. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 2, 2024

Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 58a3693. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 2, 2024

Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 58a3693. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 2, 2024

Heya @jakebailey, I've started to run the faster perf test suite on this PR at 58a3693. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/57274/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

puppeteer

packages/browsers/test/src/tsconfig.json

@typescript-bot
Copy link
Collaborator

@jakebailey
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 295,669k (± 0.01%) 295,688k (± 0.01%) ~ 295,662k 295,754k p=0.229 n=6
Parse Time 2.66s (± 0.19%) 2.67s (± 0.46%) ~ 2.65s 2.68s p=1.000 n=6
Bind Time 0.83s (± 1.45%) 0.83s (± 1.24%) ~ 0.82s 0.85s p=1.000 n=6
Check Time 8.22s (± 0.55%) 8.25s (± 0.42%) ~ 8.21s 8.30s p=0.376 n=6
Emit Time 7.08s (± 0.24%) 7.10s (± 0.22%) ~ 7.08s 7.12s p=0.086 n=6
Total Time 18.80s (± 0.27%) 18.84s (± 0.21%) ~ 18.79s 18.91s p=0.255 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 192,512k (± 1.23%) 191,539k (± 0.01%) ~ 191,516k 191,586k p=0.575 n=6
Parse Time 1.36s (± 0.80%) 1.37s (± 1.26%) ~ 1.35s 1.39s p=0.503 n=6
Bind Time 0.72s (± 0.00%) 0.72s (± 0.00%) ~ 0.72s 0.72s p=1.000 n=6
Check Time 9.36s (± 0.36%) 9.35s (± 0.28%) ~ 9.30s 9.38s p=0.329 n=6
Emit Time 2.62s (± 0.71%) 2.60s (± 0.20%) ~ 2.60s 2.61s p=0.142 n=6
Total Time 14.06s (± 0.20%) 14.04s (± 0.20%) ~ 14.00s 14.07s p=0.296 n=6
Monaco - node (v18.15.0, x64)
Memory used 347,460k (± 0.01%) 347,483k (± 0.00%) ~ 347,465k 347,503k p=0.093 n=6
Parse Time 2.47s (± 0.42%) 2.48s (± 0.51%) ~ 2.46s 2.49s p=0.359 n=6
Bind Time 0.92s (± 0.44%) 0.93s (± 0.44%) +0.01s (+ 0.72%) 0.92s 0.93s p=0.034 n=6
Check Time 6.97s (± 0.50%) 6.96s (± 0.53%) ~ 6.92s 7.03s p=0.871 n=6
Emit Time 4.06s (± 0.13%) 4.05s (± 0.26%) ~ 4.04s 4.07s p=0.794 n=6
Total Time 14.42s (± 0.23%) 14.43s (± 0.36%) ~ 14.37s 14.51s p=0.936 n=6
TFS - node (v18.15.0, x64)
Memory used 302,849k (± 0.00%) 302,877k (± 0.01%) +28k (+ 0.01%) 302,843k 302,927k p=0.044 n=6
Parse Time 2.02s (± 0.88%) 2.02s (± 0.54%) ~ 2.01s 2.04s p=1.000 n=6
Bind Time 1.00s (± 0.41%) 1.00s (± 1.09%) ~ 0.99s 1.02s p=1.000 n=6
Check Time 6.35s (± 0.35%) 6.36s (± 0.33%) ~ 6.33s 6.38s p=0.366 n=6
Emit Time 3.60s (± 0.42%) 3.60s (± 0.34%) ~ 3.58s 3.61s p=0.801 n=6
Total Time 12.97s (± 0.35%) 12.97s (± 0.14%) ~ 12.95s 13.00s p=0.747 n=6
material-ui - node (v18.15.0, x64)
Memory used 511,322k (± 0.01%) 511,303k (± 0.00%) ~ 511,282k 511,317k p=0.575 n=6
Parse Time 2.65s (± 0.63%) 2.65s (± 0.66%) ~ 2.62s 2.67s p=0.720 n=6
Bind Time 0.99s (± 1.04%) 0.99s (± 0.82%) ~ 0.98s 1.00s p=0.932 n=6
Check Time 17.28s (± 0.38%) 17.30s (± 0.38%) ~ 17.21s 17.39s p=0.574 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.92s (± 0.36%) 20.95s (± 0.28%) ~ 20.87s 21.02s p=0.421 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,710,747k (± 0.00%) 1,710,742k (± 0.00%) ~ 1,710,704k 1,710,793k p=1.000 n=6
Parse Time 6.60s (± 0.28%) 6.61s (± 0.35%) ~ 6.59s 6.65s p=0.162 n=6
Bind Time 2.39s (± 0.67%) 2.40s (± 0.35%) ~ 2.38s 2.40s p=0.530 n=6
Check Time 55.52s (± 0.42%) 55.60s (± 0.37%) ~ 55.22s 55.74s p=0.470 n=6
Emit Time 0.16s (± 2.52%) 0.16s (± 3.16%) ~ 0.16s 0.17s p=0.595 n=6
Total Time 64.67s (± 0.38%) 64.76s (± 0.32%) ~ 64.38s 64.91s p=0.378 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,412,942k (± 0.02%) 2,412,788k (± 0.02%) ~ 2,412,008k 2,413,447k p=0.810 n=6
Parse Time 4.95s (± 0.80%) 4.92s (± 0.74%) ~ 4.86s 4.96s p=0.336 n=6
Bind Time 1.87s (± 1.20%) 1.87s (± 0.76%) ~ 1.85s 1.89s p=0.462 n=6
Check Time 33.41s (± 0.20%) 33.50s (± 0.36%) ~ 33.33s 33.65s p=0.230 n=6
Emit Time 2.68s (± 0.99%) 2.70s (± 1.47%) ~ 2.63s 2.74s p=0.148 n=6
Total Time 42.92s (± 0.17%) 43.01s (± 0.29%) ~ 42.80s 43.18s p=0.093 n=6
self-compiler - node (v18.15.0, x64)
Memory used 418,712k (± 0.03%) 418,748k (± 0.01%) ~ 418,706k 418,792k p=0.066 n=6
Parse Time 2.81s (± 2.29%) 2.81s (± 1.91%) ~ 2.71s 2.85s p=1.000 n=6
Bind Time 1.09s (± 5.62%) 1.10s (± 5.36%) ~ 1.07s 1.22s p=0.351 n=6
Check Time 15.09s (± 0.30%) 15.17s (± 0.26%) +0.07s (+ 0.49%) 15.10s 15.21s p=0.030 n=6
Emit Time 1.14s (± 0.72%) 1.16s (± 0.77%) +0.02s (+ 2.05%) 1.15s 1.17s p=0.003 n=6
Total Time 20.14s (± 0.26%) 20.23s (± 0.25%) +0.10s (+ 0.48%) 20.16s 20.29s p=0.025 n=6
vscode - node (v18.15.0, x64)
Memory used 2,814,126k (± 0.00%) 2,814,268k (± 0.00%) +142k (+ 0.01%) 2,814,183k 2,814,355k p=0.005 n=6
Parse Time 10.64s (± 0.35%) 10.64s (± 0.19%) ~ 10.61s 10.67s p=0.935 n=6
Bind Time 3.42s (± 0.68%) 3.41s (± 0.43%) ~ 3.39s 3.43s p=0.625 n=6
Check Time 59.80s (± 0.26%) 60.17s (± 0.55%) +0.37s (+ 0.62%) 59.77s 60.62s p=0.045 n=6
Emit Time 16.15s (± 0.24%) 16.19s (± 0.59%) ~ 16.04s 16.32s p=0.332 n=6
Total Time 90.01s (± 0.20%) 90.41s (± 0.44%) ~ 89.93s 90.88s p=0.128 n=6
webpack - node (v18.15.0, x64)
Memory used 393,473k (± 0.01%) 393,515k (± 0.01%) ~ 393,449k 393,554k p=0.298 n=6
Parse Time 3.12s (± 0.44%) 3.11s (± 0.81%) ~ 3.08s 3.15s p=0.624 n=6
Bind Time 1.39s (± 1.47%) 1.38s (± 1.35%) ~ 1.36s 1.40s p=0.208 n=6
Check Time 14.00s (± 0.38%) 14.02s (± 0.41%) ~ 13.96s 14.12s p=0.628 n=6
Emit Time 0.00s (±244.70%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=0.405 n=6
Total Time 18.51s (± 0.36%) 18.50s (± 0.42%) ~ 18.42s 18.64s p=1.000 n=6
xstate - node (v18.15.0, x64)
Memory used 513,399k (± 0.01%) 513,420k (± 0.01%) ~ 513,382k 513,478k p=0.298 n=6
Parse Time 3.28s (± 0.37%) 3.28s (± 0.36%) ~ 3.26s 3.29s p=0.401 n=6
Bind Time 1.54s (± 0.26%) 1.54s (± 0.54%) ~ 1.53s 1.55s p=0.285 n=6
Check Time 2.84s (± 0.68%) 2.86s (± 0.65%) ~ 2.84s 2.89s p=0.091 n=6
Emit Time 0.08s (± 7.90%) 0.07s (± 5.69%) 🟩-0.01s (-10.42%) 0.07s 0.08s p=0.033 n=6
Total Time 7.75s (± 0.19%) 7.77s (± 0.34%) ~ 7.72s 7.79s p=0.125 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)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/57274/merge:

Everything looks good!

@Zzzen
Copy link
Contributor Author

Zzzen commented Feb 3, 2024

🤔 it seems that the performance regression of the self-compiler may be unrelated to the changes proposed in this pull request.
However, I believe there is an opportunity to improve performance by changing the order of comparisons.

@sandersn
Copy link
Member

@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 15, 2024

Heya @sandersn, I've started to run the faster perf test suite on this PR at cf23b47. You can monitor the build here.

@sandersn
Copy link
Member

@jakebailey the perf test failed yesterday. Could it be because this PR is old?

@Zzzen did you get what you needed from the first perf result? If so, do you still need this PR to be open?

@sandersn sandersn self-assigned this Feb 16, 2024
@jakebailey
Copy link
Member

@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 16, 2024

Heya @jakebailey, I've started to run the faster perf test suite on this PR at cf23b47. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@jakebailey
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 295,642k (± 0.01%) 295,691k (± 0.01%) +49k (+ 0.02%) 295,669k 295,741k p=0.031 n=6
Parse Time 2.66s (± 0.19%) 2.66s (± 0.31%) ~ 2.65s 2.67s p=0.140 n=6
Bind Time 0.84s (± 1.06%) 0.83s (± 1.64%) ~ 0.82s 0.85s p=0.359 n=6
Check Time 8.24s (± 0.36%) 8.27s (± 0.47%) ~ 8.20s 8.31s p=0.226 n=6
Emit Time 7.10s (± 0.14%) 7.10s (± 0.32%) ~ 7.08s 7.14s p=1.000 n=6
Total Time 18.84s (± 0.18%) 18.87s (± 0.11%) ~ 18.84s 18.90s p=0.090 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 193,976k (± 1.46%) 193,988k (± 1.47%) ~ 191,561k 197,457k p=0.936 n=6
Parse Time 1.36s (± 0.72%) 1.35s (± 0.60%) ~ 1.35s 1.37s p=0.144 n=6
Bind Time 0.72s (± 0.00%) 0.72s (± 0.00%) ~ 0.72s 0.72s p=1.000 n=6
Check Time 9.36s (± 0.18%) 9.35s (± 0.32%) ~ 9.31s 9.39s p=0.290 n=6
Emit Time 2.61s (± 0.56%) 2.60s (± 0.51%) ~ 2.58s 2.61s p=0.617 n=6
Total Time 14.05s (± 0.21%) 14.02s (± 0.20%) -0.03s (- 0.24%) 13.99s 14.07s p=0.043 n=6
Monaco - node (v18.15.0, x64)
Memory used 347,467k (± 0.01%) 347,497k (± 0.01%) ~ 347,463k 347,526k p=0.093 n=6
Parse Time 2.48s (± 0.36%) 2.49s (± 0.21%) ~ 2.48s 2.49s p=0.190 n=6
Bind Time 0.93s (± 0.56%) 0.93s (± 0.56%) ~ 0.92s 0.93s p=1.000 n=6
Check Time 6.95s (± 0.28%) 6.96s (± 0.35%) ~ 6.93s 6.99s p=0.290 n=6
Emit Time 4.07s (± 0.24%) 4.06s (± 0.27%) ~ 4.04s 4.07s p=0.143 n=6
Total Time 14.42s (± 0.20%) 14.44s (± 0.22%) ~ 14.40s 14.48s p=0.420 n=6
TFS - node (v18.15.0, x64)
Memory used 302,857k (± 0.01%) 302,875k (± 0.01%) ~ 302,854k 302,904k p=0.378 n=6
Parse Time 2.01s (± 0.70%) 2.02s (± 0.54%) ~ 2.01s 2.03s p=0.236 n=6
Bind Time 1.01s (± 0.81%) 1.00s (± 0.83%) ~ 1.00s 1.02s p=0.718 n=6
Check Time 6.35s (± 0.32%) 6.37s (± 0.21%) ~ 6.35s 6.39s p=0.121 n=6
Emit Time 3.59s (± 0.55%) 3.58s (± 0.42%) ~ 3.55s 3.59s p=0.124 n=6
Total Time 12.96s (± 0.20%) 12.97s (± 0.19%) ~ 12.93s 12.99s p=0.418 n=6
material-ui - node (v18.15.0, x64)
Memory used 511,388k (± 0.00%) 511,388k (± 0.00%) ~ 511,362k 511,417k p=0.936 n=6
Parse Time 2.66s (± 0.70%) 2.65s (± 0.74%) ~ 2.63s 2.68s p=0.731 n=6
Bind Time 0.99s (± 0.90%) 0.99s (± 0.41%) ~ 0.99s 1.00s p=0.787 n=6
Check Time 17.29s (± 0.39%) 17.28s (± 0.35%) ~ 17.20s 17.37s p=1.000 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 20.93s (± 0.31%) 20.92s (± 0.34%) ~ 20.82s 21.02s p=0.810 n=6
mui-docs - node (v18.15.0, x64)
Memory used 2,272,185k (± 0.00%) 2,272,185k (± 0.00%) ~ 2,272,157k 2,272,206k p=0.630 n=6
Parse Time 11.97s (± 0.64%) 11.96s (± 1.05%) ~ 11.85s 12.20s p=0.748 n=6
Bind Time 2.62s (± 0.45%) 2.62s (± 0.24%) ~ 2.61s 2.63s p=1.000 n=6
Check Time 101.82s (± 0.98%) 103.30s (± 0.63%) +1.48s (+ 1.45%) 102.54s 104.30s p=0.013 n=6
Emit Time 0.32s (± 0.00%) 0.32s (± 1.27%) ~ 0.32s 0.33s p=0.405 n=6
Total Time 116.73s (± 0.88%) 118.20s (± 0.54%) +1.48s (+ 1.27%) 117.44s 119.23s p=0.013 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,412,597k (± 0.03%) 2,412,579k (± 0.03%) ~ 2,412,122k 2,413,651k p=1.000 n=6
Parse Time 4.92s (± 0.98%) 4.93s (± 1.34%) ~ 4.84s 5.03s p=0.936 n=6
Bind Time 1.86s (± 0.34%) 1.86s (± 0.88%) ~ 1.84s 1.89s p=0.438 n=6
Check Time 33.57s (± 0.47%) 33.53s (± 0.40%) ~ 33.33s 33.69s p=0.689 n=6
Emit Time 2.67s (± 1.83%) 2.68s (± 2.15%) ~ 2.58s 2.74s p=0.688 n=6
Total Time 43.05s (± 0.48%) 43.01s (± 0.20%) ~ 42.91s 43.15s p=0.936 n=6
self-compiler - node (v18.15.0, x64)
Memory used 418,866k (± 0.01%) 418,894k (± 0.01%) ~ 418,869k 418,930k p=0.297 n=6
Parse Time 2.79s (± 1.93%) 2.83s (± 0.97%) ~ 2.80s 2.87s p=0.089 n=6
Bind Time 1.10s (± 5.00%) 1.08s (± 0.38%) ~ 1.07s 1.08s p=1.000 n=6
Check Time 15.16s (± 0.34%) 15.21s (± 0.20%) ~ 15.17s 15.25s p=0.072 n=6
Emit Time 1.14s (± 0.92%) 1.14s (± 1.95%) ~ 1.11s 1.17s p=0.461 n=6
Total Time 20.18s (± 0.28%) 20.26s (± 0.21%) ~ 20.19s 20.32s p=0.053 n=6
vscode - node (v18.15.0, x64)
Memory used 2,844,132k (± 0.00%) 2,844,306k (± 0.00%) +174k (+ 0.01%) 2,844,242k 2,844,346k p=0.005 n=6
Parse Time 10.77s (± 0.15%) 10.73s (± 0.22%) -0.04s (- 0.34%) 10.70s 10.77s p=0.024 n=6
Bind Time 3.43s (± 0.49%) 3.44s (± 0.31%) ~ 3.42s 3.45s p=0.405 n=6
Check Time 60.52s (± 0.14%) 60.59s (± 0.33%) ~ 60.38s 60.92s p=0.810 n=6
Emit Time 16.25s (± 0.64%) 16.27s (± 0.40%) ~ 16.16s 16.34s p=0.574 n=6
Total Time 90.98s (± 0.16%) 91.03s (± 0.20%) ~ 90.80s 91.28s p=0.575 n=6
webpack - node (v18.15.0, x64)
Memory used 394,098k (± 0.02%) 394,186k (± 0.02%) ~ 394,132k 394,303k p=0.109 n=6
Parse Time 3.10s (± 0.70%) 3.12s (± 0.43%) ~ 3.11s 3.14s p=0.139 n=6
Bind Time 1.38s (± 1.07%) 1.39s (± 1.34%) ~ 1.36s 1.41s p=0.463 n=6
Check Time 14.04s (± 0.39%) 14.14s (± 0.41%) +0.10s (+ 0.70%) 14.06s 14.23s p=0.020 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 18.53s (± 0.26%) 18.65s (± 0.38%) +0.12s (+ 0.64%) 18.54s 18.75s p=0.024 n=6
xstate - node (v18.15.0, x64)
Memory used 513,367k (± 0.01%) 513,391k (± 0.01%) ~ 513,326k 513,469k p=0.521 n=6
Parse Time 3.28s (± 0.30%) 3.27s (± 0.12%) ~ 3.27s 3.28s p=0.071 n=6
Bind Time 1.55s (± 0.53%) 1.55s (± 0.75%) ~ 1.54s 1.57s p=1.000 n=6
Check Time 2.86s (± 1.16%) 2.85s (± 0.73%) ~ 2.83s 2.88s p=0.419 n=6
Emit Time 0.08s (± 0.00%) 0.07s (± 0.00%) 🟩-0.01s (-12.50%) 0.07s 0.07s p=0.001 n=6
Total Time 7.77s (± 0.46%) 7.74s (± 0.24%) ~ 7.72s 7.77s p=0.145 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)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@Zzzen
Copy link
Contributor Author

Zzzen commented Feb 18, 2024

Closed for housekeeping. To my surprise, the performance is worse. 😭

@Zzzen Zzzen closed this Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Narrow object property when key is a variable
4 participants