-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Improve __spreadArray perf, and other fixes related to SpreadElement #44527
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
@typescript-bot perf test |
Heya @rbuckton, I've started to run the perf test suite on this PR at ad979d0. You can monitor the build here. Update: The results are in! |
@rbuckton Here they are:Comparison Report - master..44527
System
Hosts
Scenarios
Developer Information: |
I don't expect to see much of a perf change since our own compiler doesn't use spread very often. There may be a bump due to adding a new literal value argument to each spread. If so I can investigate adding a reusable and immutable @typescript-bot perf test |
Heya @rbuckton, I've started to run the perf test suite on this PR at 40fa251. You can monitor the build here. Update: The results are in! |
@rbuckton Here they are:Comparison Report - master..44527
System
Hosts
Scenarios
Developer Information: |
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
@typescript-bot perf test |
Heya @rbuckton, I've started to run the perf test suite on this PR at 9964d8b. You can monitor the build here. Update: The results are in! |
@rbuckton Here they are:Comparison Report - master..44527
System
Hosts
Scenarios
Developer Information: |
That result seems fairly reasonable. |
Running one more set of perf tests. The previous test was against @typescript-bot perf test |
Heya @rbuckton, I've started to run the perf test suite on this PR at 9964d8b. You can monitor the build here. Update: The results are in! |
@rbuckton Here they are:Comparison Report - main..44527
System
Hosts
Scenarios
Developer Information: |
That's much better. I was concerned that the material-ui tests looked slower, but it was due to diffing against an old baseline. We need to merge and ship microsoft/tslib#151 before I can merge this, unless I remove the checker restriction for I'd prefer the restriction remain so that we have a way to indicate to the user they need to update their copy of |
This changes
__spreadArray
to useArray.prototype.concat
and avoids explicitly converting a sparse array to a packed array when such a conversion is unnecessary or unexpected. This also fixes an issue where[1, , ...a]
would drop the missing element at index 1.Before this can be merged I need to verify backwards compatibility with
__spreadArray
for older versions of TS. I suspect I may need to change theif
condition to be backwards compatible (where we always packed the result).Fixes #44287