-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
remove sliced #11238
remove sliced #11238
Conversation
Closing for now, as I am unsure if this has a perf impact. |
Here some benchmarks on comparing spread-operator, Array.from and sliced var sliced = require('./')
var Bench = require('benchmark');
var s = new Bench.Suite;
var slice = [].slice;
var testArray = new Array(10).fill("a")
s.add('[...]', function () {
[...testArray];
}).add('Array.from', function () {
Array.from(testArray);
}).add('Array.prototype.slice.call(testArray)', function () {
Array.prototype.slice.call(testArray);
}).add('cached slice.call', function () {
slice.call(testArray)
}).add('sliced', function () {
sliced(testArray)
}).on('cycle', function (evt) {
console.log(String(evt.target));
}).on('complete', function () {
console.log('fastest is %s', this.filter('fastest').pluck('name'));
})
.run(); with 10 elements
with 1000 elements:
So [...] and Array.from are about the same speed. slice.call is not usable because arguments is not an array but an iteratable. But in any case sliced is slower than the built-ins. |
@@ -173,7 +172,7 @@ function iter(i) { | |||
if (debug) { | |||
if (typeof debug === 'function') { | |||
debug.apply(_this, | |||
[_this.name, i].concat(sliced(args, 0, args.length - 1))); | |||
[_this.name, i].concat(args.slice(0, args.length - 1))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here i used normal slice, as I think debug is not used in production?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug can be used in production. I'm sure some people use it for logging or perf. Shouldn't assume that certain features are never used in prod.
Also var sliced = require('./')
var Bench = require('benchmark');
var s = new Bench.Suite;
var slice = [].slice;
var testArray = new Array(3).fill("a")
s.add('direct', function () {
[testArray[0], testArray[1], testArray[2]];
}).add('[...]', function () {
[...testArray];
}).add('Array.from', function () {
Array.from(testArray);
}).add('Array.prototype.slice.call(testArray)', function () {
Array.prototype.slice.call(testArray);
}).add('cached slice.call', function () {
slice.call(testArray)
}).add('sliced', function () {
sliced(testArray)
}).on('cycle', function (evt) {
console.log(String(evt.target));
}).on('complete', function () {
console.log('fastest is %s', this.filter('fastest').pluck('name'));
})
.run();
So if we know that the length of the array is 2 or 3, than directly assigning the values to the new Array is much faster... |
@@ -5,7 +5,7 @@ | |||
|
|||
'use strict'; | |||
|
|||
const utils = require('./utils'); | |||
const utils = require('./utils'); // eslint-disable-line no-unused-vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont know why removing this line makes the unit tests fail.
I checked mongoose sourcecode. It seems that the spread operator is not used usually, but everywhere Array.from. I can change the occurences of the spread operator to Array.from if it is desired. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, thanks 👍 I like removing the extra dependency, especially if the benchmarks that justified using sliced are no longer accurate.
Nice work, thanks @Uzlopak 👍 |
Remove the package "sliced" in favor of native functions.