FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

events: optimize listener array cloning by mscdex · Pull Request #1050 · nodejs/node · GitHub

/ node Public

events: optimize listener array cloning - #1050

Closed
mscdex wants to merge 1 commit into
nodejs:v1.xfrom
mscdex:perf-events-part4
ConversationCommits (1)ChecksFiles changed
Closed

events: optimize listener array cloning#1050
mscdex wants to merge 1 commit into
nodejs:v1.xfrom
mscdex:perf-events-part4

Conversation

mscdex commented Mar 4, 2015

Copy link
Copy Markdown
Contributor

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().

events: optimize listener array cloning
b58a7d5 
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()`.
bnoordhuis reviewed Mar 4, 2015
View reviewed changes
Comment thread lib/events.js

bnoordhuis Mar 4, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Curious, is iterating backwards faster than forward?

mscdex Mar 4, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Fishrock123 Mar 5, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Mar 4, 2015

Copy link
Copy Markdown
Member

LGTM

silverwind commented Mar 4, 2015

Copy link
Copy Markdown
Contributor

+1, kill all the magic numbers.

julianduque commented Mar 5, 2015

Copy link
Copy Markdown
Contributor

LGTM

mscdex added a commit that referenced this pull request Mar 5, 2015
events: optimize listener array cloning
555a7c4 
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 <info@bnoordhuis.nl>
Reviewed-By: Julian Duque <julianduquej@gmail.com>

mscdex commented Mar 5, 2015

Copy link
Copy Markdown
Contributor Author

Thanks, landed in 555a7c4.

mscdex closed this Mar 5, 2015
rvagg mentioned this pull request Mar 5, 2015
Closed
mscdex mentioned this pull request Nov 23, 2015
Closed
mscdex deleted the perf-events-part4 branch February 7, 2016 19:06
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
snyk-bot mentioned this pull request Apr 24, 2023
Open
aliscco mentioned this pull request Nov 30, 2023
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL