-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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 native generators/iterables, remove helper cruft #51921
Conversation
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at 67025d7. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - main..51921
System
Hosts
Scenarios
TSServerComparison Report - main..51921
System
Hosts
Scenarios
StartupComparison Report - main..51921
System
Hosts
Scenarios
Developer Information: |
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at dbf27e9. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - main..51921
System
Hosts
Scenarios
TSServerComparison Report - main..51921
System
Hosts
Scenarios
StartupComparison Report - main..51921
System
Hosts
Scenarios
Developer Information: |
Running this through benchstat in its current state, it appears to all be noise and some wins that could be chance:
|
@@ -2903,15 +2690,6 @@ export function enumerateInsertsAndDeletes<T, U>(newItems: readonly T[], oldItem | |||
return hasChanges; | |||
} | |||
|
|||
/** @internal */ | |||
export function fill<T>(length: number, cb: (index: number) => T): T[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one's the same as arrayOf
.
hasDecorators, | ||
HasDecorators, | ||
hasDecorators, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Annoying :(
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at 09f17c6. You can monitor the build here. Update: The results are in! |
This should be ready for review, though likely I can split out the Collection removal into a different PR. (The other PRs I've sent for cleanups had been pulled out of this one.) |
This reverts commit 4d68782.
@jakebailey Here they are:
CompilerComparison Report - main..51921
System
Hosts
Scenarios
TSServerComparison Report - main..51921
System
Hosts
Scenarios
StartupComparison Report - main..51921
System
Hosts
Scenarios
Developer Information: |
Seems like it's still noise.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sourcemap stuff could probably be rewritten to be a generator, provided we'd be okay with some API changes.
Yeah, I just wasn't confident enough to attempt that in a cleanup PR; it seemed like a bunch of perf testing had gone into producing it, so, I'd want to make sure whatever it ends up as behaves how people expect. |
This PR:
core.ts
.getEntries
, which with our new target was always equal toObject.entries
for-of
,yield
,yield*
, etc.arrayFrom(it).map(fn)
to the more efficient but less discoverablearrayFrom(it, fn)
.RemovesSplit out into Completely remove Collection/ReadonlyCollection types #52134 with Completely remove Push type #52133 for Push too.Collection
andReadonlyCollection
entirely, as they were only used in a couple of places which could be more specific.sum
, which was a weird function which arbitrarily added up values off of properties of objects.This PR doesn't remove
arrayFrom
, as it turns out to be much faster thanArray.from
. It also doesn't removeisArray
pending #17002.There's only one main instance where we don't use a native generator, and that's
decodeMappings
, which seems difficult to change.