Skip to content

Conversation

bnoordhuis
Copy link
Member

Replace the call to Array#splice() with a faster open-coded version
that creates less garbage.

Add a new benchmark to prove it. With the change applied, it scores
a whopping 40% higher.

R=@chrisdickinson

@sonewman
Copy link
Contributor

👍

@algesten
Copy link

That's super sweet!

Perhaps a pedantic point, but shouldn't we call it something else but splice though? I mean since it does one very specific thing. shiftOne?

Don't use Number#toPrecision(), it switches to scientific notation for
numbers with more digits than the precision; use Number#toFixed().

PR-URL: nodejs#185
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Replace the call to Array#splice() with a faster open-coded version
that creates less garbage.

Add a new benchmark to prove it.  With the change applied, it scores
a whopping 40% higher.

PR-URL: nodejs#185
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
@chrisdickinson
Copy link
Contributor

LGTM. I looked into splitting splice into spliceFront and spliceBack (copying backwards or forwards from the splice position, respectively) but since the usual number of listeners is <= 10, it didn't make too much of a difference. I also tried specializing position == 0 and position == list.length, but again, no difference!

@bnoordhuis bnoordhuis force-pushed the micro-optimize-ee-remove-listener branch 2 times, most recently from 1ee720b to d3f8db1 Compare December 20, 2014 12:38
@bnoordhuis bnoordhuis closed this Dec 20, 2014
@bnoordhuis bnoordhuis deleted the micro-optimize-ee-remove-listener branch December 20, 2014 12:38
@bnoordhuis bnoordhuis merged commit d3f8db1 into nodejs:v0.12 Dec 20, 2014
@bnoordhuis
Copy link
Member Author

Cheers, fixed up the indentation and (partially) incorporated @algesten's feedback by renaming the function to spliceOne(). Landed in d3f8db1.

@chrisdickinson Using Array#shift() to remove the first element is a bit faster, but only for larger lists. The lower bound is probably at least 16 elements but there doesn't seem to be an appreciable difference until the array length is in the low hundreds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants