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.

Sign up for GitHub

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

Jump to bottom

Prefer non-partial signature match in non-generic case #55447

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Prefer non-partial signature match in non-generic case #55447

merged 2 commits into from
Nov 30, 2023

Conversation

Andarist
Copy link
Contributor

fixes #55203

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Aug 21, 2023
@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 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 13, 2023 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 13, 2023 •
edited
Loading

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

Update: The results are in!

weswigham
weswigham approved these changes Sep 13, 2023
View reviewed changes
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.

I think this is good, but since it affects overload resolution (admittedly in an edge case when we're talking about union overloads), it might have knock-on effects in the wild, so we'll have to make sure the extended test suites come back OK.

@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,257k (± 0.01%) 300,273k (± 0.01%) ~ 300,263k 300,308k p=0.373 n=6
Parse Time 3.00s (± 0.17%) 3.01s (± 0.18%) ~ 3.00s 3.01s p=0.640 n=6
Bind Time 0.93s (± 0.00%) 0.93s (± 0.00%) ~ 0.93s 0.93s p=1.000 n=6
Check Time 9.30s (± 0.09%) 9.33s (± 0.36%) ~ 9.29s 9.37s p=0.289 n=6
Emit Time 7.61s (± 0.43%) 7.64s (± 0.30%) ~ 7.60s 7.66s p=0.169 n=6
Total Time 20.85s (± 0.15%) 20.90s (± 0.21%) ~ 20.83s 20.94s p=0.064 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 194,362k (± 0.53%) 194,474k (± 0.67%) ~ 193,923k 197,118k p=1.000 n=6
Parse Time 1.58s (± 0.00%) 1.58s (± 0.00%) ~ 1.58s 1.58s p=1.000 n=6
Bind Time 0.79s (± 0.69%) 0.80s (± 0.51%) ~ 0.79s 0.80s p=0.282 n=6
Check Time 9.96s (± 0.48%) 9.95s (± 0.23%) ~ 9.93s 9.99s p=0.871 n=6
Emit Time 2.74s (± 0.33%) 2.74s (± 0.15%) ~ 2.73s 2.74s p=0.787 n=6
Total Time 15.07s (± 0.30%) 15.07s (± 0.16%) ~ 15.04s 15.10s p=0.936 n=6
Monaco - node (v16.17.1, x64)
Memory used 347,201k (± 0.01%) 347,172k (± 0.01%) ~ 347,130k 347,218k p=0.173 n=6
Parse Time 2.68s (± 0.30%) 2.69s (± 0.45%) ~ 2.67s 2.70s p=0.652 n=6
Bind Time 0.99s (± 0.41%) 0.99s (± 0.00%) ~ 0.99s 0.99s p=0.405 n=6
Check Time 7.93s (± 0.10%) 7.93s (± 0.26%) ~ 7.91s 7.96s p=0.805 n=6
Emit Time 4.26s (± 0.35%) 4.27s (± 0.41%) ~ 4.25s 4.30s p=0.328 n=6
Total Time 15.86s (± 0.13%) 15.88s (± 0.15%) ~ 15.84s 15.91s p=0.222 n=6
TFS - node (v16.17.1, x64)
Memory used 301,188k (± 0.01%) 301,178k (± 0.00%) ~ 301,172k 301,186k p=0.128 n=6
Parse Time 2.19s (± 0.75%) 2.17s (± 0.45%) ~ 2.16s 2.18s p=0.111 n=6
Bind Time 1.08s (± 6.41%) 1.11s (± 0.46%) ~ 1.11s 1.12s p=0.114 n=6
Check Time 7.22s (± 0.35%) 7.21s (± 0.20%) ~ 7.19s 7.23s p=0.514 n=6
Emit Time 3.99s (± 0.54%) 3.99s (± 0.20%) ~ 3.97s 3.99s p=0.932 n=6
Total Time 14.47s (± 0.51%) 14.48s (± 0.14%) ~ 14.46s 14.52s p=0.573 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,547k (± 0.00%) 479,555k (± 0.01%) ~ 479,483k 479,589k p=0.297 n=6
Parse Time 3.15s (± 0.24%) 3.15s (± 0.20%) ~ 3.14s 3.16s p=0.718 n=6
Bind Time 0.91s (± 0.00%) 0.91s (± 0.45%) ~ 0.90s 0.91s p=0.405 n=6
Check Time 17.83s (± 0.28%) 17.87s (± 0.26%) ~ 17.81s 17.92s p=0.295 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 21.89s (± 0.23%) 21.93s (± 0.23%) ~ 21.87s 21.99s p=0.336 n=6
xstate - node (v16.17.1, x64)
Memory used 542,960k (± 0.03%) 542,955k (± 0.01%) ~ 542,880k 543,044k p=0.298 n=6
Parse Time 3.70s (± 0.24%) 3.68s (± 0.33%) -0.02s (- 0.45%) 3.67s 3.70s p=0.039 n=6
Bind Time 1.38s (± 4.31%) 1.46s (± 0.28%) +0.08s (+ 5.80%) 1.45s 1.46s p=0.018 n=6
Check Time 3.27s (± 2.62%) 3.18s (± 0.31%) ~ 3.17s 3.19s p=0.169 n=6
Emit Time 0.08s (± 6.19%) 0.09s (± 6.44%) ~ 0.08s 0.09s p=0.640 n=6
Total Time 8.44s (± 0.40%) 8.40s (± 0.18%) ~ 8.38s 8.42s p=0.123 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,488ms (± 0.15%) 2,490ms (± 0.05%) ~ 2,488ms 2,491ms p=0.459 n=6
Req 2 - geterr 5,942ms (± 0.36%) 5,947ms (± 0.24%) ~ 5,929ms 5,962ms p=0.378 n=6
Req 3 - references 344ms (± 0.37%) 344ms (± 0.34%) ~ 343ms 346ms p=0.801 n=6
Req 4 - navto 277ms (± 0.98%) 276ms (± 0.30%) ~ 275ms 277ms p=0.548 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 80ms (± 4.13%) 82ms (± 8.08%) ~ 76ms 94ms p=0.739 n=6
CompilerTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,643ms (± 1.73%) 2,610ms (± 0.61%) ~ 2,585ms 2,631ms p=0.173 n=6
Req 2 - geterr 4,780ms (± 0.18%) 4,769ms (± 0.22%) ~ 4,759ms 4,783ms p=0.109 n=6
Req 3 - references 351ms (± 0.16%) 351ms (± 0.16%) ~ 350ms 351ms p=1.000 n=6
Req 4 - navto 268ms (± 0.51%) 269ms (± 0.00%) ~ 269ms 269ms p=0.293 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.04%) 79ms (± 0.52%) ~ 78ms 79ms p=1.000 n=6
xstateTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,707ms (± 0.20%) 2,708ms (± 0.16%) ~ 2,702ms 2,714ms p=0.747 n=6
Req 2 - geterr 1,968ms (± 1.47%) 1,975ms (± 0.66%) ~ 1,952ms 1,987ms p=0.936 n=6
Req 3 - references 135ms (± 1.87%) 138ms (± 2.73%) ~ 131ms 141ms p=0.107 n=6
Req 4 - navto 358ms (± 0.45%) 361ms (± 0.87%) ~ 357ms 365ms p=0.081 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 320ms (± 1.54%) 328ms (± 1.30%) +8ms (+ 2.40%) 319ms 330ms p=0.049 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.55ms (± 0.21%) 156.68ms (± 0.19%) +0.12ms (+ 0.08%) 154.72ms 160.63ms p=0.006 n=600
tsserver-startup - node (v16.17.1, x64)
Execution time 230.91ms (± 0.12%) 231.61ms (± 0.19%) +0.70ms (+ 0.30%) 230.05ms 245.94ms p=0.000 n=600
tsserverlibrary-startup - node (v16.17.1, x64)
Execution time 235.51ms (± 0.13%) 236.09ms (± 0.15%) +0.58ms (+ 0.24%) 234.72ms 243.24ms p=0.000 n=600
typescript-startup - node (v16.17.1, x64)
Execution time 235.63ms (± 0.14%) 235.71ms (± 0.13%) +0.08ms (+ 0.03%) 234.29ms 244.51ms p=0.001 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/55447/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 sandersn merged commit 0a87761 into microsoft:main Nov 30, 2023
19 checks passed
@acutmore acutmore mentioned this pull request Feb 21, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weswigham weswigham weswigham approved these changes

@gabritto gabritto Awaiting requested review from gabritto

Assignees

@gabritto gabritto

Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Andarist @weswigham @typescript-bot @sandersn @gabritto