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

events: optimize listener array cloning #1050

Closed
wants to merge 1 commit into from

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Mar 4, 2015

This both switches to a single algorithm for array cloning and also
speeds up (by ~100% in the ee-listeners-many benchmark) the
"many elements" case that was previously handled by
array.slice().

This both switches to a single algorithm for array cloning and also
speeds up (by ~100% in the ee-listeners-many benchmark) the
"many elements"  case that was previously handled by
`array.slice()`.
return ret;
function arrayClone(arr, i) {
var copy = new Array(i);
while (i--)
Copy link
Member

Choose a reason for hiding this comment

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

Curious, is iterating backwards faster than forward?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to this jsperf and this jsperf it appears so, at least in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's really micro, but I also always do this when possible/sane, idk I guess I just like the way it looks. XD

@bnoordhuis
Copy link
Member

LGTM

@silverwind
Copy link
Contributor

+1, kill all the magic numbers.

@julianduque
Copy link
Contributor

LGTM

mscdex added a commit that referenced this pull request Mar 5, 2015
This both switches to a single algorithm for array cloning and also
speeds up (by ~100% in the ee-listeners-many benchmark) the
"many elements"  case that was previously handled by
`array.slice()`.

PR-URL: #1050
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Julian Duque <[email protected]>
@mscdex
Copy link
Contributor Author

mscdex commented Mar 5, 2015

Thanks, landed in 555a7c4.

@mscdex mscdex closed this Mar 5, 2015
@rvagg rvagg mentioned this pull request Mar 5, 2015
@mscdex mscdex deleted the perf-events-part4 branch February 7, 2016 19:06
@ChALkeR ChALkeR added performance Issues and PRs related to the performance of Node.js. events Issues and PRs related to the events subsystem / EventEmitter. labels Feb 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
events Issues and PRs related to the events subsystem / EventEmitter. performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants