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.

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

fix(performance): Eliminate use of ES6 generators #4075

Closed
wants to merge 1 commit into from

Conversation

avelad
Copy link
Collaborator

@avelad avelad commented Mar 29, 2022

@@ -96,11 +98,17 @@ shaka.util.Iterables = class {
* - The previous item in the list, if it exists.
*
* @param {!Iterable.<T>} iterable
* @return {!Iterable.<
* {i: number, item: T, prev: (T|undefined), next: (T|undefined)}>}
* @return {!Array.<{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a drop-in replacement that doesn't require the call sites to change, which is why I did it this way. It was the easiest change to make. But if a big part of our performance problem is GC, we should probably avoid creating additional arrays.

In some cases, we could rewrite the call sites if a simple for loop would suffice. Or if we want to keep a functional style, we could accept a callback to be invoked on each item in the enumeration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new PR from my repo #4092

@avelad avelad closed this Mar 31, 2022
@joeyparrish joeyparrish deleted the no-generators branch April 21, 2022 15:55
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants