remove sliced package in favor of native Array.prototype.slice.call

[Uzlopak]

After some investigations, I conclude, that sliced-package is actually massively slower than native Array.prototype.slice. It is only faster, if you have super small arrays, like an array with 6 to 10 entries. Bigger Arrays are much faster processed with native Array.prototype.slice.call.

The benchmark of sliced is actually just testing an array with 1 entry. Thats why it seems much faster than the built-in-methods. But as soon as you use a benchmark with bigger slices, it gets slower and slower and slower.

See corresponding issue

aheckmann/sliced#11

[AbdelrahmanHafez]

The issue points out that Array.prototype.slice is faster than sliced when working with large arrays, which doesn't impact how mongoose is utilizing sliced (with a quick search, mostly with arguments, which are usually less than 5).

That being said, I think that importing an external package for such micro-optimization increases the overhead for mongoose development, and would rather leave optimization for the PhDs at Google to figure out along the way on the V8 engine.

This is especially true for a package that works with databases such as mongoose, where a good DB call will take ~10 ms, trying to save microseconds is a waste of effort.

[Uzlopak]

Yeah, well. Partly agree with your beginner article on Dev. But this is a framework. The framework should have more optimizations, as it is heavy used by 3rd party. So trying to save milliseconds is worth the shot. But sliced in its current implementation is for sure slower than native functions.

To be honest, I always benchmark my code for fastest implementation and checking for v8 deoptimizations. Not because it is a waste of effort, but because there is already too much bad code on npm, which results in the assumption, that javascript is a piss poor performance language, despite the fact, that it can do better, but people implement always the "more convenient" or "more readable" solution, neglecting the fact that it needs to be fast and using as less as possible RAM to improve the enduser experience. The enduser gives a damn about how readable your code is. If it is slow, it is slow.

[AbdelrahmanHafez]

That's an interesting insight.

It seems to me with your points that you're in fact supporting using sliced over Array.prototype.slice in mongoose, rather than removing it. Since it's, AFAIK, always used with arguments which are always less than 10, a number where sliced performs better.

While I understand your point of view, I think better readability is more important than having software run a few microseconds faster. Of course, if it's an issue that the end-user can notice, we need to do something about it, which I doubt will be the case for sliced vs Array.prototype.slice in the context of mongoose.

Do you happen to have a benchmark that provides a number on which of both solutions is better in the context of mongoose?

[Uzlopak]

I hardly doubt that sliced is faster in those cases. If you run benchmarks based on sliced, you would see, that sliced is about 70 % slower in those cases. I doubt, that sliced would approve my changes, as the project seems to be basically dead?. And I have no intent to implement a sliced2 package. :D

You dont even need to "slice" in some cases actually.
e.g. in query.js
before

  if (arguments.length === 1) {
    this._ensurePath('mod');
    val = arguments[0];
    path = this._path;
  } else if (arguments.length === 2 && !Array.isArray(arguments[1])) {
    this._ensurePath('mod');
    val = slice(arguments);
    path = this._path;
  } else if (arguments.length === 3) {
    val = slice(arguments, 1);
    path = arguments[0];
  } else {
    val = arguments[1];
    path = arguments[0];
  }

after

  if (arguments.length === 1) {
    this._ensurePath('mod');
    val = arguments[0];
    path = this._path;
  } else if (arguments.length === 2 && !Array.isArray(arguments[1])) {
    this._ensurePath('mod');
    val = [arguments[0], arguments[1]];
    path = this._path;
  } else if (arguments.length === 3) {
    val = [arguments[1], arguments[2]];
    path = arguments[0];
  } else {
    val = arguments[1];
    path = arguments[0];
  }

So I would still recommend to rip out in the case of mongoose like I proposed in the initial post.

[Uzlopak]

scratch that. It should be actually something like:

  if (arguments.length === 1) {
    this._ensurePath('mod');
    val = arguments[0];
    path = this._path;
  } else if (arguments.length === 2 && !Array.isArray(arguments[1])) {
    this._ensurePath('mod');
    val = arguments[0].slice(arguments[1]);
    path = this._path;
  } else if (arguments.length === 3) {
    val = arguments[0].slice(arguments[1], arguments[2]);
    path = arguments[0];
  } else {
    val = arguments[1];
    path = arguments[0];
  }

But it seems, that sliced is not behaving like expected because it can also process Objects and strings and creates from them Arrays???

[Uzlopak]

#11238 got merged few days ago