-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Use for-loop, instead of slice #3489
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
Conversation
Hrm. I don't know if 8% is worth it for that gross code ;) |
@jashkenas It would definitely be worth it to me as |
Oh no, this is a 2000% (not a typo!) boost. I'm saying my linked-list code should be 8% slower, but it's the exact same speed because this slice is bottle-necking |
2000% boost? Now I'm confused. But holy hell. Would this be worth pulling out as a little |
Also, that JSPerf looks questionable. Why would three arguments be so much faster than no arguments? Let's be careful here, before mucking things about. |
That would help, but it would still de-opt because of leaking arguments. We could just use
I'm just as confused as you are. I think it's because the method isn't getting jit-d since there's not a hot loop. |
Gah, one of these days I'm going to spot the errors before I post them in a PR. Here's an corrected jsperf, showing 500% increases. (I was calling |
Ping jashkenas/underscore#1758. Mind testing with a |
I'll do that shortly. |
One would think that would hit your "leaking arguments" problem, but it's worth a shot. |
@megawac: http://jsperf.com/trigger-with-for-loop-vs-slice/3 Using |
First, thanks @jridgewell for finding this perf boost! While I agree that slice is more elegant the one-line for loop seems worth it in this case, since it's in such a hot path. |
Would it be more appropriate to optimize Talk to me on gitter, ill update a jsperf tonight |
But I think he's saying that it'll deopt if we pass arguments anywhere. So even passing |
A solid rest implementation will give this a run for its money http://jsperf.com/trigger-with-for-loop-vs-slice/5 |
That jsperf was super convoluted, I cleaned it up. @jridgewell is right, passing In most cases I convert the |
👍 |
Use for-loop, instead of slice
http://jsperf.com/trigger-with-for-loop-vs-slice
One of the optimizations I've found in the linked-list events branch is using a for-loop to grab the trigger arguments. Perf testing
#trigger
on that branch (with slice) against master doesn't tell the whole truth, since this slice call is the performance bottleneck. My linked-list trigger should be roughly ~8% slower than a simple array, but it's a dead heat as long as slice is in there.