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

Replace slice with restParam #818

Merged
merged 2 commits into from
Jul 1, 2015
Merged

Replace slice with restParam #818

merged 2 commits into from
Jul 1, 2015

Conversation

megawac
Copy link
Collaborator

@megawac megawac commented Jul 1, 2015

This has been known to greatly simplify code (heavily used in underscore and lodash) and be a great optimization (slicing arguments is sloooooooooooow).

If interested, refer to these threads for documentation, benchmarks, arguments, naming, etc:-)

args[index] = arguments[index];
}
args[startIndex] = rest;
return func.apply(this, args);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Line 156-161 is currently unused. Thoughts? I was thinking about just commenting it out as it handles cases not handled by the switch statement

@aearly
Copy link
Collaborator

aearly commented Jul 1, 2015

Interesting -- simplifies the code a bit. Is this actually faster, though? What do the benchmarks in perf/ say?

@megawac
Copy link
Collaborator Author

megawac commented Jul 1, 2015

The bench results are essentially the same in all benchmarks +- a couple
milliseconds which makes sense because this is a synch part of a async
test.

On Wed, Jul 1, 2015 at 1:10 PM, Alexander Early notifications@github.com
wrote:

Interesting -- simplifies the code a bit. Is this actually faster, though?
What do the benchmarks in perf say?


Reply to this email directly or view it on GitHub
#818 (comment).

@aearly
Copy link
Collaborator

aearly commented Jul 1, 2015

Ok, that's fine then. When we switched from Array.prototype.slice to _baseSlice, there was a measurable performance increase! This gives us the same performance with cleaner code, so 👍

aearly added a commit that referenced this pull request Jul 1, 2015
Replace slice with restParam
@aearly aearly merged commit c312a05 into caolan:master Jul 1, 2015
megawac pushed a commit to megawac/async that referenced this pull request Jul 2, 2015
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.

None yet

2 participants