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

Don't cache child lists for tokens #58233

Merged
merged 6 commits into from
Apr 19, 2024
Merged

Conversation

DanielRosenwasser
Copy link
Member

This PR always returns a shared frozen array from getNodeChildren when given a token (e.g. an identifier or any other keyword, punctuation, etc.). It also marks all child-lists as readonly Node[] instead of just Node[].

If we want, we can probably just use the internal emptyArray, but I don't think we've ever frozen that one.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 17, 2024
@DanielRosenwasser
Copy link
Member Author

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@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

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Apr 17, 2024

I guess an alternative path would be to have IdentifierObject, PrivateIdentifierObject, and TokenObject all have their own getChildren (and possibly getChildCount and getChildAt), but 🤷‍♂️.

const nodeChildren = new WeakMap<Node, Node[] | undefined>();
const nodeChildren = new WeakMap<Node, readonly Node[] | undefined>();

const emptyChildren: readonly Node[] = Object.freeze([]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use emptyArray?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confusingly, services defines its own mutable empty array (and none of them actually guard against modification)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this isn't even in services, yeah, emptyArray would be nice. (I used to have a PR that froze it but it didn't really seem to matter)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the description:

If we want, we can probably just use the internal emptyArray, but I don't think we've ever frozen that one.

Happy to switch over to that too, but Jake mentioned that he didn't want to make that change since getChildren() always was declared as a mutable array. Freezing this array (for runtime safety) and propagating the change (for static verification) helped validate that it's unlikely we mutate these arrays.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does freezing emptyArray affect our perf at all? If not, I'd just do that and use emptyArray.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried in #55755 but I think that was actually more of a problem with trying to make immutable empty maps/sets. Sent #58240 to try just arrays quick.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it sure looks like freezing is bad: #58240 (comment)

@DanielRosenwasser
Copy link
Member Author

@typescript-bot test top400

@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
test top400 ✅ Started ✅ Results

@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,055k (± 0.01%) 297,021k (± 0.00%) ~ 297,001k 297,035k p=0.092 n=6
Parse Time 4.05s (± 0.29%) 4.04s (± 0.43%) ~ 4.02s 4.07s p=0.566 n=6
Bind Time 1.21s (± 0.52%) 1.21s (± 0.43%) ~ 1.21s 1.22s p=0.386 n=6
Check Time 12.11s (± 0.51%) 12.17s (± 0.48%) ~ 12.09s 12.23s p=0.127 n=6
Emit Time 10.51s (± 0.46%) 10.52s (± 0.33%) ~ 10.48s 10.56s p=0.744 n=6
Total Time 27.89s (± 0.39%) 27.95s (± 0.21%) ~ 27.88s 28.04s p=0.298 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 191,784k (± 0.02%) 191,817k (± 0.08%) ~ 191,708k 192,112k p=0.575 n=6
Parse Time 1.65s (± 1.11%) 1.64s (± 1.24%) ~ 1.61s 1.67s p=0.458 n=6
Bind Time 0.87s (± 1.19%) 0.87s (± 0.94%) ~ 0.86s 0.88s p=0.928 n=6
Check Time 11.33s (± 0.55%) 11.33s (± 0.31%) ~ 11.28s 11.38s p=0.872 n=6
Emit Time 3.15s (± 0.31%) 3.16s (± 0.52%) ~ 3.14s 3.18s p=0.505 n=6
Total Time 17.00s (± 0.23%) 16.99s (± 0.26%) ~ 16.94s 17.04s p=0.421 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,751,155k (± 0.00%) 1,751,107k (± 0.00%) ~ 1,751,068k 1,751,145k p=0.090 n=6
Parse Time 8.07s (± 0.78%) 8.06s (± 0.75%) ~ 7.99s 8.16s p=1.000 n=6
Bind Time 2.71s (± 0.38%) 2.71s (± 0.66%) ~ 2.68s 2.73s p=1.000 n=6
Check Time 66.98s (± 0.42%) 67.11s (± 0.20%) ~ 66.96s 67.28s p=0.296 n=6
Emit Time 0.16s (± 0.00%) 0.16s (± 0.00%) ~ 0.16s 0.16s p=1.000 n=6
Total Time 77.93s (± 0.42%) 78.05s (± 0.23%) ~ 77.82s 78.25s p=0.298 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,331,236k (± 2.57%) 2,307,748k (± 0.05%) ~ 2,306,637k 2,309,329k p=0.378 n=6
Parse Time 7.38s (± 0.64%) 7.41s (± 0.69%) ~ 7.32s 7.47s p=0.298 n=6
Bind Time 2.73s (± 1.22%) 2.72s (± 0.85%) ~ 2.69s 2.76s p=0.689 n=6
Check Time 49.35s (± 0.42%) 49.46s (± 0.78%) ~ 48.87s 49.95s p=0.378 n=6
Emit Time 3.93s (± 1.80%) 3.87s (± 2.95%) ~ 3.73s 4.01s p=0.470 n=6
Total Time 63.39s (± 0.28%) 63.47s (± 0.67%) ~ 62.83s 63.93s p=0.810 n=6
self-build-src-public-api - node (v18.15.0, x64)
Memory used 2,382,271k (± 0.03%) 2,382,704k (± 0.02%) ~ 2,381,917k 2,383,350k p=0.378 n=6
Parse Time 6.17s (± 0.80%) 6.16s (± 1.13%) ~ 6.09s 6.25s p=0.575 n=6
Bind Time 2.08s (± 0.66%) 2.09s (± 2.13%) ~ 2.04s 2.16s p=0.746 n=6
Check Time 40.12s (± 0.27%) 40.12s (± 0.19%) ~ 39.99s 40.22s p=0.872 n=6
Emit Time 3.16s (± 1.68%) 3.19s (± 2.77%) ~ 3.07s 3.31s p=0.521 n=6
Total Time 51.55s (± 0.34%) 51.57s (± 0.27%) ~ 51.36s 51.75s p=0.471 n=6
self-compiler - node (v18.15.0, x64)
Memory used 419,439k (± 0.00%) 419,433k (± 0.01%) ~ 419,386k 419,464k p=0.873 n=6
Parse Time 3.39s (± 0.56%) 3.39s (± 0.58%) ~ 3.37s 3.41s p=0.801 n=6
Bind Time 1.31s (± 1.08%) 1.32s (± 1.15%) ~ 1.29s 1.33s p=0.564 n=6
Check Time 18.05s (± 0.20%) 18.09s (± 0.33%) ~ 18.00s 18.19s p=0.228 n=6
Emit Time 1.36s (± 2.06%) 1.37s (± 1.79%) ~ 1.33s 1.40s p=0.809 n=6
Total Time 24.12s (± 0.14%) 24.16s (± 0.34%) ~ 24.07s 24.30s p=0.520 n=6
ts-pre-modules - node (v18.15.0, x64)
Memory used 369,005k (± 0.02%) 369,036k (± 0.03%) ~ 368,937k 369,158k p=0.575 n=6
Parse Time 2.44s (± 0.96%) 2.43s (± 1.15%) ~ 2.39s 2.46s p=0.935 n=6
Bind Time 1.33s (± 1.11%) 1.32s (± 1.65%) ~ 1.29s 1.34s p=0.246 n=6
Check Time 13.30s (± 0.32%) 13.33s (± 0.38%) ~ 13.27s 13.42s p=0.326 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 17.07s (± 0.36%) 17.07s (± 0.25%) ~ 17.01s 17.14s p=0.627 n=6
vscode - node (v18.15.0, x64)
Memory used 2,915,585k (± 0.00%) 2,915,564k (± 0.00%) ~ 2,915,509k 2,915,640k p=0.230 n=6
Parse Time 11.27s (± 0.40%) 11.24s (± 0.12%) ~ 11.22s 11.26s p=0.332 n=6
Bind Time 3.46s (± 2.60%) 3.46s (± 2.46%) ~ 3.40s 3.57s p=0.681 n=6
Check Time 62.64s (± 0.25%) 62.68s (± 0.46%) ~ 62.45s 63.21s p=0.689 n=6
Emit Time 17.12s (± 7.87%) 16.53s (± 0.67%) ~ 16.36s 16.64s p=0.296 n=6
Total Time 94.48s (± 1.35%) 93.91s (± 0.37%) ~ 93.45s 94.33s p=0.471 n=6
webpack - node (v18.15.0, x64)
Memory used 409,497k (± 0.02%) 409,503k (± 0.02%) ~ 409,416k 409,601k p=0.748 n=6
Parse Time 4.83s (± 0.26%) 4.84s (± 0.52%) ~ 4.82s 4.88s p=0.669 n=6
Bind Time 2.03s (± 0.67%) 2.03s (± 0.90%) ~ 2.01s 2.06s p=1.000 n=6
Check Time 21.02s (± 0.30%) 21.04s (± 0.36%) ~ 20.95s 21.11s p=0.573 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 27.87s (± 0.22%) 27.91s (± 0.33%) ~ 27.78s 28.03s p=0.574 n=6
xstate-main - node (v18.15.0, x64)
Memory used 459,018k (± 0.02%) 458,955k (± 0.01%) ~ 458,898k 459,012k p=0.230 n=6
Parse Time 4.00s (± 0.38%) 4.00s (± 0.29%) ~ 3.99s 4.02s p=0.363 n=6
Bind Time 1.48s (± 1.16%) 1.48s (± 0.99%) ~ 1.46s 1.50s p=0.625 n=6
Check Time 22.20s (± 0.74%) 22.25s (± 0.62%) ~ 22.08s 22.43s p=0.521 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 27.69s (± 0.54%) 27.72s (± 0.48%) ~ 27.56s 27.89s p=0.748 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,491ms (± 0.80%) 3,505ms (± 0.42%) ~ 3,487ms 3,532ms p=0.810 n=6
Req 2 - geterr 7,614ms (± 1.42%) 7,601ms (± 1.78%) ~ 7,496ms 7,863ms p=0.936 n=6
Req 3 - references 429ms (± 0.45%) 433ms (± 1.77%) ~ 424ms 446ms p=0.418 n=6
Req 4 - navto 338ms (± 0.94%) 340ms (± 1.30%) ~ 332ms 344ms p=0.226 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 115ms (± 9.28%) 117ms (± 8.91%) ~ 110ms 136ms p=0.858 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 3,657ms (± 0.48%) 3,683ms (± 1.05%) ~ 3,628ms 3,726ms p=0.230 n=6
Req 2 - geterr 5,700ms (± 1.59%) 5,723ms (± 1.32%) ~ 5,677ms 5,875ms p=0.065 n=6
Req 3 - references 453ms (± 1.47%) 454ms (± 1.20%) ~ 443ms 459ms p=0.806 n=6
Req 4 - navto 339ms (± 1.95%) 337ms (± 0.31%) ~ 335ms 338ms p=0.869 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 121ms (± 5.26%) 121ms (± 5.52%) ~ 107ms 124ms p=0.935 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 3,287ms (± 5.80%) 3,207ms (± 0.89%) ~ 3,158ms 3,240ms p=0.872 n=6
Req 2 - geterr 1,971ms (±12.18%) 1,820ms (± 9.40%) ~ 1,735ms 2,168ms p=0.128 n=6
Req 3 - references 173ms (± 8.09%) 166ms (±10.55%) ~ 152ms 195ms p=0.462 n=6
Req 4 - navto 530ms (± 0.86%) 480ms (±12.23%) ~ 421ms 543ms p=0.297 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 452ms (± 1.24%) 429ms (±11.33%) ~ 364ms 464ms p=0.575 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 224.23ms (± 0.15%) 224.37ms (± 0.18%) +0.14ms (+ 0.06%) 222.27ms 229.04ms p=0.001 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 343.12ms (± 0.30%) 342.83ms (± 0.31%) -0.29ms (- 0.09%) 334.10ms 351.80ms p=0.000 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 332.55ms (± 0.28%) 332.61ms (± 0.31%) ~ 324.56ms 358.49ms p=0.168 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 224.03ms (± 0.15%) 223.92ms (± 0.15%) -0.11ms (- 0.05%) 222.40ms 227.12ms p=0.004 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

@typescript-bot
Copy link
Collaborator

@DanielRosenwasser Here are the results of running the top 400 repos comparing main and refs/pull/58233/merge:

Everything looks good!

@DanielRosenwasser
Copy link
Member Author

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 19, 2024

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

Command Status Results
perf test this ✅ Started 👀 Results

@DanielRosenwasser DanielRosenwasser changed the title Return a shared empty list for tokens in getNodeChildren Don't catch child lists for tokens Apr 19, 2024
@DanielRosenwasser DanielRosenwasser changed the title Don't catch child lists for tokens Don't cache child lists for tokens Apr 19, 2024
@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,153k (± 0.01%) 297,159k (± 0.01%) ~ 297,112k 297,205k p=0.689 n=6
Parse Time 4.04s (± 0.50%) 4.05s (± 0.27%) ~ 4.03s 4.06s p=0.247 n=6
Bind Time 1.19s (± 0.46%) 1.21s (± 1.13%) +0.02s (+ 1.67%) 1.20s 1.23s p=0.015 n=6
Check Time 12.16s (± 0.25%) 12.14s (± 0.32%) ~ 12.10s 12.20s p=0.627 n=6
Emit Time 10.50s (± 0.33%) 10.53s (± 0.34%) ~ 10.47s 10.57s p=0.107 n=6
Total Time 27.89s (± 0.19%) 27.94s (± 0.12%) ~ 27.88s 27.97s p=0.147 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 194,821k (± 0.74%) 192,976k (± 0.96%) ~ 191,716k 195,391k p=0.065 n=6
Parse Time 2.01s (± 1.13%) 2.01s (± 1.12%) ~ 1.98s 2.03s p=1.000 n=6
Bind Time 1.07s (± 0.97%) 1.06s (± 0.77%) ~ 1.05s 1.07s p=0.546 n=6
Check Time 13.99s (± 0.55%) 14.02s (± 0.40%) ~ 13.95s 14.11s p=0.687 n=6
Emit Time 3.88s (± 0.56%) 3.87s (± 0.58%) ~ 3.85s 3.91s p=0.623 n=6
Total Time 20.95s (± 0.49%) 20.97s (± 0.25%) ~ 20.90s 21.04s p=0.810 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,752,854k (± 0.00%) 1,752,891k (± 0.00%) ~ 1,752,834k 1,752,964k p=0.521 n=6
Parse Time 9.96s (± 0.50%) 9.94s (± 0.52%) ~ 9.89s 10.04s p=0.470 n=6
Bind Time 3.34s (± 1.11%) 3.32s (± 0.41%) ~ 3.31s 3.35s p=0.413 n=6
Check Time 81.46s (± 0.20%) 81.85s (± 0.48%) ~ 81.35s 82.38s p=0.092 n=6
Emit Time 0.20s (± 2.06%) 0.19s (± 2.81%) ~ 0.19s 0.20s p=0.282 n=6
Total Time 94.96s (± 0.21%) 95.30s (± 0.40%) ~ 94.79s 95.83s p=0.128 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,309,049k (± 0.03%) 2,308,967k (± 0.05%) ~ 2,308,148k 2,310,974k p=0.575 n=6
Parse Time 7.41s (± 1.28%) 7.38s (± 0.76%) ~ 7.30s 7.43s p=0.422 n=6
Bind Time 2.74s (± 0.71%) 2.73s (± 0.64%) ~ 2.71s 2.76s p=0.807 n=6
Check Time 49.34s (± 0.71%) 49.45s (± 0.54%) ~ 49.03s 49.77s p=0.810 n=6
Emit Time 3.90s (± 1.65%) 4.03s (± 0.87%) +0.13s (+ 3.20%) 3.97s 4.06s p=0.013 n=6
Total Time 63.39s (± 0.60%) 63.60s (± 0.47%) ~ 63.12s 63.96s p=0.471 n=6
self-build-src-public-api - node (v18.15.0, x64)
Memory used 2,381,823k (± 0.03%) 2,382,337k (± 0.03%) ~ 2,381,588k 2,383,370k p=0.173 n=6
Parse Time 5.15s (± 0.62%) 5.16s (± 1.04%) ~ 5.08s 5.22s p=0.422 n=6
Bind Time 1.78s (± 5.69%) 1.74s (± 0.48%) ~ 1.72s 1.74s p=1.000 n=6
Check Time 34.18s (± 0.56%) 34.35s (± 0.15%) ~ 34.30s 34.44s p=0.078 n=6
Emit Time 2.73s (± 1.10%) 2.69s (± 2.83%) ~ 2.57s 2.77s p=0.296 n=6
Total Time 43.85s (± 0.28%) 43.95s (± 0.18%) ~ 43.85s 44.08s p=0.128 n=6
self-compiler - node (v18.15.0, x64)
Memory used 419,245k (± 0.01%) 419,301k (± 0.03%) ~ 419,213k 419,521k p=0.378 n=6
Parse Time 4.21s (± 0.19%) 4.17s (± 0.68%) -0.04s (- 0.99%) 4.12s 4.20s p=0.006 n=6
Bind Time 1.56s (± 0.41%) 1.56s (± 1.02%) ~ 1.54s 1.58s p=1.000 n=6
Check Time 22.27s (± 0.31%) 22.28s (± 0.39%) ~ 22.15s 22.36s p=0.810 n=6
Emit Time 1.71s (± 1.46%) 1.73s (± 1.77%) ~ 1.69s 1.77s p=0.465 n=6
Total Time 29.76s (± 0.27%) 29.74s (± 0.35%) ~ 29.59s 29.85s p=0.810 n=6
ts-pre-modules - node (v18.15.0, x64)
Memory used 368,984k (± 0.03%) 368,954k (± 0.02%) ~ 368,858k 369,091k p=0.810 n=6
Parse Time 2.46s (± 0.91%) 2.45s (± 0.48%) ~ 2.44s 2.47s p=0.683 n=6
Bind Time 1.32s (± 1.12%) 1.30s (± 1.06%) ~ 1.29s 1.32s p=0.192 n=6
Check Time 13.29s (± 0.20%) 13.32s (± 0.19%) ~ 13.28s 13.35s p=0.090 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 17.07s (± 0.28%) 17.08s (± 0.23%) ~ 17.03s 17.13s p=0.573 n=6
vscode - node (v18.15.0, x64)
Memory used 2,919,709k (± 0.00%) 2,919,641k (± 0.00%) ~ 2,919,476k 2,919,812k p=0.471 n=6
Parse Time 16.57s (± 0.50%) 16.54s (± 0.24%) ~ 16.50s 16.60s p=0.572 n=6
Bind Time 4.96s (± 0.24%) 4.97s (± 0.56%) ~ 4.95s 5.02s p=0.934 n=6
Check Time 88.30s (± 0.31%) 88.25s (± 0.48%) ~ 87.55s 88.72s p=0.936 n=6
Emit Time 26.09s (± 9.03%) 26.81s (± 8.34%) ~ 23.88s 28.54s p=0.689 n=6
Total Time 135.93s (± 1.77%) 136.59s (± 1.50%) ~ 133.91s 138.16s p=0.575 n=6
webpack - node (v18.15.0, x64)
Memory used 409,545k (± 0.01%) 409,545k (± 0.02%) ~ 409,478k 409,698k p=0.471 n=6
Parse Time 3.92s (± 1.06%) 3.90s (± 1.12%) ~ 3.83s 3.94s p=0.628 n=6
Bind Time 1.64s (± 0.75%) 1.65s (± 0.84%) ~ 1.63s 1.67s p=0.247 n=6
Check Time 16.96s (± 0.17%) 16.89s (± 0.50%) ~ 16.74s 16.99s p=0.064 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.52s (± 0.14%) 22.44s (± 0.27%) -0.08s (- 0.36%) 22.34s 22.50s p=0.030 n=6
xstate-main - node (v18.15.0, x64)
Memory used 459,152k (± 0.03%) 459,111k (± 0.01%) ~ 459,062k 459,190k p=0.575 n=6
Parse Time 3.99s (± 0.89%) 4.00s (± 0.58%) ~ 3.97s 4.03s p=0.809 n=6
Bind Time 1.47s (± 0.43%) 1.47s (± 1.95%) ~ 1.43s 1.50s p=0.677 n=6
Check Time 22.36s (± 0.37%) 22.19s (± 0.66%) -0.17s (- 0.78%) 22.02s 22.44s p=0.045 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 27.82s (± 0.35%) 27.66s (± 0.45%) ~ 27.53s 27.84s p=0.065 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,478ms (± 0.88%) 3,498ms (± 1.06%) ~ 3,436ms 3,546ms p=0.471 n=6
Req 2 - geterr 7,586ms (± 0.63%) 7,626ms (± 0.84%) ~ 7,548ms 7,732ms p=0.378 n=6
Req 3 - references 434ms (± 1.75%) 435ms (± 2.26%) ~ 426ms 452ms p=0.810 n=6
Req 4 - navto 338ms (± 0.79%) 339ms (± 1.03%) ~ 333ms 343ms p=0.328 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 112ms (± 1.66%) 116ms (± 7.72%) ~ 111ms 134ms p=0.677 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen 3,669ms (± 0.77%) 3,691ms (± 0.98%) ~ 3,653ms 3,741ms p=0.298 n=6
Req 2 - geterr 5,718ms (± 1.47%) 5,676ms (± 0.42%) ~ 5,638ms 5,700ms p=0.378 n=6
Req 3 - references 456ms (± 1.43%) 449ms (± 0.34%) ~ 447ms 450ms p=0.061 n=6
Req 4 - navto 340ms (± 1.74%) 346ms (± 1.94%) ~ 339ms 357ms p=0.061 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 123ms (± 3.59%) 124ms (± 0.88%) ~ 122ms 125ms p=0.730 n=6
xstate-main-1-tsserver - node (v18.15.0, x64)
Req 1 - updateOpen 7,790ms (± 0.41%) 7,777ms (± 0.23%) ~ 7,751ms 7,795ms p=0.575 n=6
Req 2 - geterr 1,682ms (± 0.91%) 1,696ms (± 1.22%) ~ 1,678ms 1,724ms p=0.173 n=6
Req 3 - references 138ms (± 1.88%) 135ms (± 3.14%) ~ 126ms 137ms p=0.114 n=6
Req 4 - navto 607ms (± 0.79%) 595ms (± 0.79%) -12ms (- 1.98%) 589ms 600ms p=0.005 n=6
Req 5 - completionInfo count 3,413 (± 0.00%) 3,413 (± 0.00%) ~ 3,413 3,413 p=1.000 n=6
Req 5 - completionInfo 1,306ms (± 1.46%) 1,293ms (± 1.93%) ~ 1,254ms 1,315ms p=0.376 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 154.87ms (± 0.15%) 154.63ms (± 0.17%) -0.24ms (- 0.15%) 153.46ms 158.38ms p=0.000 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time 342.35ms (± 0.30%) 342.16ms (± 0.31%) -0.19ms (- 0.06%) 334.16ms 353.31ms p=0.001 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time 332.12ms (± 0.31%) 332.15ms (± 0.28%) ~ 323.73ms 343.45ms p=0.886 n=600
typescript-startup - node (v18.15.0, x64)
Execution time 271.23ms (± 0.30%) 271.34ms (± 0.28%) ~ 264.65ms 277.68ms p=0.219 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

@DanielRosenwasser
Copy link
Member Author

It turns out that we always made this optimization - the only difference with this PR is that now

  • we no longer stick that emptyArray in a WeakMap
  • every list is marked as readonly

I don't know if there is a difference practically between !isNode(...) and isToken(...).

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me

@DanielRosenwasser DanielRosenwasser added Breaking Change Would introduce errors in existing code API Relates to the public API for TypeScript labels Apr 19, 2024
@jakebailey
Copy link
Member

jakebailey commented Apr 19, 2024

I don't know if there is a difference practically between !isNode(...) and isToken(...).

They're equivalent in behavior, but isNode only makes one comparison so I'd just keep that. Or maybe that's just speculative optimization 😄

But yes, better to not stick that empty array into the weak map over and over.

@DanielRosenwasser DanielRosenwasser merged commit e8f2225 into main Apr 19, 2024
25 checks passed
@DanielRosenwasser DanielRosenwasser deleted the sharedTokenChildren branch April 19, 2024 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Relates to the public API for TypeScript Author: Team Breaking Change Would introduce errors in existing code For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants