Cache discriminated contextual types#58372
Conversation
|
@typescript-bot test it |
|
@typescript-bot test it Sorry, the new function app we had to deploy has some scaling issues being worked out. |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests comparing Everything looks good! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Wow, that's actually significant., 3% on our self test (expected), but also 1.5% in mui-docs. |
weswigham
left a comment
There was a problem hiding this comment.
Honestly surprised we call this with the same arguments so many times - but I guess at least twice with the same args is basically guaranteed, since signature inference is 2-pass.
|
@typescript-bot test it |
|
@ahejlsberg Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hmm, removing |
|
@jakebailey Here are the results of running the top 400 repos comparing Everything looks good! |
|
I bisected a perf regression typechecking https://github.com/arktypeio/arktype and found the following: {
"checkTime": 9.68,
"types": 483415,
"instantiations": 2629152
}
devDependencies:
- typescript 5.5.0-dev.20240429
+ typescript 5.5.0-dev.20240430
{
"checkTime": 12.2,
"types": 514846,
"instantiations": 2819487
}It seems likely this change was somehow involved? |
|
Guided by This PR isn't part of that. |
|
My mistake, I likely jumped to conclusions when I saw this in the release notes as a likely candidate since I use quite a lot of unions internally. @Andarist and I have been looking at which of the changes from that day could have caused such a large increase in type instantiations and check time and haven't been able to identify it yet. |
While researching another issue, @jakebailey noticed this statement takes an exceedingly long time to type check. With the caching added in this PR, the check time for the statement drops from 250ms to about zero on my machine.