-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Use iterators to avoid calculating all properties of UnionOrIntersectionType #53346
base: main
Are you sure you want to change the base?
Use iterators to avoid calculating all properties of UnionOrIntersectionType #53346
Conversation
…er can exit early
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at c53294b. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - main..53346
System
Hosts
Scenarios
TSServerComparison Report - main..53346
System
Hosts
Scenarios
StartupComparison Report - main..53346
System
Hosts
Scenarios
Developer Information: |
@typescript-bot perf test this faster |
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 8a6ba6d. You can monitor the build here. |
@typescript-bot perf test this faster |
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at b8d3664. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:Comparison Report - main..53346
System
Hosts
Scenarios
Developer Information: |
My latest version, while clearer, turns out to be worse than the previous iteration, so I guess I'll have to revert it. Really weird. |
@typescript-bot perf test this faster |
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 21ccbb9. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:Comparison Report - main..53346
System
Hosts
Scenarios
Developer Information: |
@typescript-bot test this |
Heya @jakebailey, I've started to run the extended test suite on this PR at 21ccbb9. You can monitor the build here. |
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 21ccbb9. You can monitor the build here. Update: The results are in! |
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 21ccbb9. You can monitor the build here. Update: The results are in! |
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 21ccbb9. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:Comparison Report - main..53346
System
Hosts
Scenarios
Developer Information: |
So overall it seems like Compiler-Unions is consistently faster and uses less memory (expected), but mui and xstate get slower. Not sure what to make of that. I'm going to hold off on this until I (hopefully soon) get better benchmarks rolling. |
Drafting this for now until I can see where the slowdown is and if it can be avoided. |
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the regular perf test suite on this PR at 391f090. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
StartupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@typescript-bot perf test this faster |
Heya @jakebailey, I've started to run the tsc-only perf test suite on this PR at 6148cde. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@jakebailey should this still be a draft PR? Do you need another look at it before it's actually ready to merge? |
I need to do a perf investigation to see if it's possible to avoid the negative performance hit in some benchmarks. That and I think I'm still on the fence as to whether or not this is too clever or if I can get some of the same benefits for the benchmark I was using another way. |
@typescript-bot perf test this faster |
Heya @jakebailey, I've started to run the faster perf test suite on this PR at 1e89756. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@typescript-bot perf test this faster |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
Wow, now this PR gets a 10% speedup in mui-docs. |
For #51188
Profiling gave me this big chunk:
getPropertiesOfUnionOrIntersectionType
is the biggest chunk. What I noticed was thatgetReducedType
didn't actually need all of the properties; all it wants to know is whether or not some property satisfiesisNeverReducedProperty
.By fiddling with the code a bit, we can convert
getPropertiesOfUnionOrIntersectionType
into an iterator, allowing callers to stop calculating properties early if that benefits them. And, it turns out that most callers ofgetPropertiesOfUnionOrIntersectionType
(all but one!) indeed don't actually care about every property, only whichever one matches some predicate.After, the same code looks like:
hyperfine, for this test case:
aka 44% faster aka 1.44x faster aka 31% less time.