Skip to content

Defer processing of nested generic calls that return constructor types #54813

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

Conversation

Andarist
Copy link
Contributor

I'm trying a lot of things to benefit from contextual inference like this for nested calls. The only thing that works is if the nested call returns a function but at that position in our library regular functions can also one received.

We need a way between differentiating them at runtime so I tried to return a constructable type from our factories instead - that didn't pan out. It didn't help my inference problems but I was quickly able to find out why... so I tried this as an experiment and since all tests pass, I think it's worth proposing this.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jun 28, 2023
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@Andarist Andarist force-pushed the defer-resolving-generic-functions-that-return-constructors branch from 2f8d596 to 8a8b22e Compare June 28, 2023 15:38
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

IMO, what's good for the generic function is good for the generic constructor - it being missing was probably just an oversight.

@weswigham
Copy link
Member

@typescript-bot test this
@typescript-bot run dt
@typescript-bot test top100
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 13, 2023

Heya @weswigham, I've started to run the regular perf test suite on this PR at e0a1e0d. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 13, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 13, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Compiler

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 300,530k (± 0.00%) 300,528k (± 0.00%) ~ 300,514k 300,536k p=0.575 n=6
Parse Time 3.01s (± 0.25%) 3.01s (± 0.18%) ~ 3.00s 3.01s p=0.476 n=6
Bind Time 0.93s (± 0.44%) 0.93s (± 0.00%) ~ 0.93s 0.93s p=0.405 n=6
Check Time 9.34s (± 0.24%) 9.33s (± 0.29%) ~ 9.28s 9.36s p=0.517 n=6
Emit Time 7.64s (± 0.29%) 7.64s (± 0.21%) ~ 7.62s 7.66s p=0.622 n=6
Total Time 20.93s (± 0.16%) 20.90s (± 0.15%) ~ 20.85s 20.93s p=0.145 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 194,238k (± 0.02%) 194,274k (± 0.01%) ~ 194,236k 194,314k p=0.066 n=6
Parse Time 1.58s (± 0.00%) 1.58s (± 0.26%) ~ 1.58s 1.59s p=0.405 n=6
Bind Time 0.79s (± 0.69%) 0.80s (± 0.65%) ~ 0.79s 0.80s p=0.640 n=6
Check Time 9.96s (± 0.65%) 10.01s (± 0.47%) ~ 9.96s 10.08s p=0.376 n=6
Emit Time 2.74s (± 0.33%) 2.74s (± 0.33%) ~ 2.73s 2.75s p=1.000 n=6
Total Time 15.08s (± 0.49%) 15.12s (± 0.31%) ~ 15.07s 15.19s p=0.469 n=6
Monaco - node (v16.17.1, x64)
Memory used 347,287k (± 0.01%) 347,303k (± 0.00%) ~ 347,293k 347,314k p=0.521 n=6
Parse Time 2.69s (± 0.20%) 2.68s (± 0.19%) ~ 2.68s 2.69s p=0.640 n=6
Bind Time 0.99s (± 0.00%) 0.99s (± 0.00%) ~ 0.99s 0.99s p=1.000 n=6
Check Time 7.94s (± 0.20%) 7.95s (± 0.33%) ~ 7.91s 7.98s p=0.406 n=6
Emit Time 4.26s (± 0.35%) 4.26s (± 0.19%) ~ 4.25s 4.27s p=0.448 n=6
Total Time 15.87s (± 0.16%) 15.88s (± 0.12%) ~ 15.86s 15.90s p=0.466 n=6
TFS - node (v16.17.1, x64)
Memory used 301,408k (± 0.00%) 301,406k (± 0.01%) ~ 301,377k 301,445k p=0.810 n=6
Parse Time 2.18s (± 0.69%) 2.18s (± 0.75%) ~ 2.15s 2.20s p=0.491 n=6
Bind Time 1.11s (± 0.46%) 1.11s (± 0.00%) ~ 1.11s 1.11s p=0.174 n=6
Check Time 7.24s (± 0.32%) 7.21s (± 0.25%) ~ 7.18s 7.23s p=0.157 n=6
Emit Time 3.98s (± 0.50%) 3.98s (± 0.38%) ~ 3.97s 4.01s p=0.868 n=6
Total Time 14.52s (± 0.13%) 14.48s (± 0.25%) ~ 14.43s 14.54s p=0.076 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,886k (± 0.00%) 479,877k (± 0.00%) ~ 479,856k 479,895k p=0.335 n=6
Parse Time 3.15s (± 0.13%) 3.15s (± 0.28%) ~ 3.14s 3.16s p=0.787 n=6
Bind Time 0.91s (± 0.00%) 0.91s (± 0.00%) ~ 0.91s 0.91s p=1.000 n=6
Check Time 17.88s (± 0.43%) 17.88s (± 0.43%) ~ 17.80s 17.98s p=0.936 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 21.94s (± 0.35%) 21.94s (± 0.36%) ~ 21.85s 22.04s p=0.872 n=6
xstate - node (v16.17.1, x64)
Memory used 542,848k (± 0.02%) 542,889k (± 0.02%) ~ 542,783k 543,065k p=0.378 n=6
Parse Time 3.68s (± 0.14%) 3.68s (± 0.17%) ~ 3.67s 3.69s p=0.386 n=6
Bind Time 1.40s (± 4.69%) 1.46s (± 0.28%) ~ 1.45s 1.46s p=0.178 n=6
Check Time 3.25s (± 3.09%) 3.17s (± 0.46%) ~ 3.15s 3.19s p=0.514 n=6
Emit Time 0.08s (± 4.99%) 0.08s (± 6.19%) ~ 0.08s 0.09s p=0.595 n=6
Total Time 8.42s (± 0.48%) 8.38s (± 0.24%) ~ 8.37s 8.42s p=0.122 n=6
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, 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 (v16.17.1, x64)
Req 1 - updateOpen 2,492ms (± 0.16%) 2,490ms (± 0.14%) ~ 2,484ms 2,493ms p=0.258 n=6
Req 2 - geterr 5,940ms (± 0.23%) 5,950ms (± 0.14%) ~ 5,944ms 5,964ms p=0.199 n=6
Req 3 - references 344ms (± 0.53%) 343ms (± 0.44%) ~ 341ms 345ms p=0.222 n=6
Req 4 - navto 275ms (± 0.15%) 277ms (± 0.76%) ~ 275ms 280ms p=0.074 n=6
Req 5 - completionInfo count 1,356 (± 0.00%) 1,356 (± 0.00%) ~ 1,356 1,356 p=1.000 n=6
Req 5 - completionInfo 83ms (±10.41%) 83ms (± 6.75%) ~ 76ms 93ms p=0.680 n=6
CompilerTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,621ms (± 0.56%) 2,632ms (± 0.86%) ~ 2,609ms 2,662ms p=0.375 n=6
Req 2 - geterr 4,792ms (± 0.14%) 4,785ms (± 0.17%) ~ 4,773ms 4,796ms p=0.127 n=6
Req 3 - references 351ms (± 0.30%) 350ms (± 0.18%) ~ 349ms 351ms p=0.388 n=6
Req 4 - navto 270ms (± 0.95%) 269ms (± 0.15%) ~ 268ms 269ms p=0.673 n=6
Req 5 - completionInfo count 1,518 (± 0.00%) 1,518 (± 0.00%) ~ 1,518 1,518 p=1.000 n=6
Req 5 - completionInfo 79ms (± 1.25%) 79ms (± 0.52%) ~ 78ms 79ms p=0.753 n=6
xstateTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,708ms (± 0.12%) 2,703ms (± 0.18%) ~ 2,698ms 2,711ms p=0.077 n=6
Req 2 - geterr 1,960ms (± 2.00%) 1,950ms (± 2.63%) ~ 1,882ms 1,991ms p=1.000 n=6
Req 3 - references 136ms (± 2.41%) 135ms (± 7.72%) ~ 114ms 140ms p=0.250 n=6
Req 4 - navto 358ms (± 0.55%) 359ms (± 0.65%) ~ 356ms 363ms p=0.683 n=6
Req 5 - completionInfo count 2,071 (± 0.00%) 2,071 (± 0.00%) ~ 2,071 2,071 p=1.000 n=6
Req 5 - completionInfo 323ms (± 1.77%) 324ms (± 1.34%) ~ 317ms 329ms p=0.872 n=6
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • CompilerTSServer - node (v16.17.1, x64)
  • Compiler-UnionsTSServer - node (v16.17.1, x64)
  • xstateTSServer - node (v16.17.1, 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 (v16.17.1, x64)
Execution time 156.25ms (± 0.17%) 156.51ms (± 0.18%) +0.26ms (+ 0.16%) 154.75ms 160.63ms p=0.000 n=600
tsserver-startup - node (v16.17.1, x64)
Execution time 231.13ms (± 0.15%) 231.90ms (± 0.13%) +0.76ms (+ 0.33%) 230.38ms 235.14ms p=0.000 n=600
tsserverlibrary-startup - node (v16.17.1, x64)
Execution time 235.63ms (± 0.13%) 236.13ms (± 0.16%) +0.50ms (+ 0.21%) 234.90ms 244.69ms p=0.000 n=600
typescript-startup - node (v16.17.1, x64)
Execution time 236.01ms (± 0.11%) 235.97ms (± 0.10%) ~ 234.85ms 237.92ms p=0.227 n=600
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • tsc-startup - node (v16.17.1, x64)
  • tsserver-startup - node (v16.17.1, x64)
  • tsserverlibrary-startup - node (v16.17.1, x64)
  • typescript-startup - node (v16.17.1, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

Hey @weswigham, it looks like the DT test run failed. Please check the log for more details.
You can check the log here.

@sandersn
Copy link
Member

@tyescript-bot run dt

@sandersn
Copy link
Member

@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 30, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

@sandersn sandersn merged commit 8d1fa44 into microsoft:main Nov 30, 2023
@Andarist Andarist deleted the defer-resolving-generic-functions-that-return-constructors branch March 11, 2024 19:36
sandersn added a commit to sandersn/TypeScript that referenced this pull request Mar 12, 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.

5 participants