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

Optimized waterfall, parallel, et al. #1395

Merged
merged 12 commits into from Apr 7, 2017
14 changes: 7 additions & 7 deletions lib/auto.js
Expand Up @@ -4,7 +4,7 @@ import indexOf from 'lodash/_baseIndexOf';
import isArray from 'lodash/isArray';
import okeys from 'lodash/keys';
import noop from 'lodash/noop';
import rest from './internal/rest';
import slice from 'lodash/slice';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you use ./internal/slice here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, looks like my auto-import plugin picked up the wrong slice.


import once from './internal/once';
import onlyOnce from './internal/onlyOnce';
Expand Down Expand Up @@ -192,26 +192,26 @@ export default function (tasks, concurrency, callback) {
function runTask(key, task) {
if (hasError) return;

var taskCallback = onlyOnce(rest(function(err, args) {
var taskCallback = onlyOnce(function(err, result) {
runningTasks--;
if (args.length <= 1) {
args = args[0];
if (arguments.length > 2) {
result = slice(arguments, 1);
}
if (err) {
var safeResults = {};
forOwn(results, function(val, rkey) {
safeResults[rkey] = val;
});
safeResults[key] = args;
safeResults[key] = result;
hasError = true;
listeners = Object.create(null);

callback(err, safeResults);
} else {
results[key] = args;
results[key] = result;
taskComplete(key);
}
}));
});

runningTasks++;
var taskFn = wrapAsync(task[task.length - 1]);
Expand Down
9 changes: 5 additions & 4 deletions lib/doDuring.js
@@ -1,5 +1,5 @@
import noop from 'lodash/noop';
import rest from './internal/rest';
import slice from './internal/slice';
import onlyOnce from './internal/onlyOnce';
import wrapAsync from './internal/wrapAsync';

Expand Down Expand Up @@ -28,11 +28,12 @@ export default function doDuring(fn, test, callback) {
var _fn = wrapAsync(fn);
var _test = wrapAsync(test);

var next = rest(function(err, args) {
if (err) return callback(err);
function next(err/*, args...*/) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

mind changing comments to /*, ...args*/ (a bit better semantically)

if (err) return callback(err);
var args = slice(arguments, 1);
args.push(check);
_test.apply(this, args);
});
};

function check(err, truth) {
if (err) return callback(err);
Expand Down
7 changes: 4 additions & 3 deletions lib/doWhilst.js
@@ -1,5 +1,5 @@
import noop from 'lodash/noop';
import rest from './internal/rest';
import slice from './internal/slice';

import onlyOnce from './internal/onlyOnce';
import wrapAsync from './internal/wrapAsync';
Expand Down Expand Up @@ -29,10 +29,11 @@ import wrapAsync from './internal/wrapAsync';
export default function doWhilst(iteratee, test, callback) {
callback = onlyOnce(callback || noop);
var _iteratee = wrapAsync(iteratee);
var next = rest(function(err, args) {
var next = function(err/*, args...*/) {
if (err) return callback(err);
var args = slice(arguments, 1);
if (test.apply(this, args)) return _iteratee(next);
callback.apply(null, [null].concat(args));
});
};
_iteratee(next);
}
12 changes: 6 additions & 6 deletions lib/internal/parallel.js
@@ -1,20 +1,20 @@
import noop from 'lodash/noop';
import isArrayLike from 'lodash/isArrayLike';
import rest from './rest';
import slice from './slice';
import wrapAsync from './wrapAsync';

export default function _parallel(eachfn, tasks, callback) {
callback = callback || noop;
var results = isArrayLike(tasks) ? [] : {};

eachfn(tasks, function (task, key, callback) {
wrapAsync(task)(rest(function (err, args) {
if (args.length <= 1) {
args = args[0];
wrapAsync(task)(function (err, result) {
if (arguments.length > 2) {
result = slice(arguments, 1);
}
results[key] = args;
results[key] = result;
callback(err);
}));
});
}, function (err) {
callback(err, results);
});
Expand Down
9 changes: 9 additions & 0 deletions lib/internal/slice.js
@@ -0,0 +1,9 @@
export default function slice(arrayLike, start) {
start = start|0;
var newLen = Math.max(arrayLike.length - start, 0);
var newArr = Array(newLen);
for(var idx = 0; idx < newLen; idx++) {
newArr[idx] = arrayLike[start + idx];
}
return newArr;
}
14 changes: 5 additions & 9 deletions lib/reflect.js
@@ -1,5 +1,5 @@
import initialParams from './internal/initialParams';
import rest from './internal/rest';
import slice from './internal/slice';
import wrapAsync from './internal/wrapAsync';

/**
Expand Down Expand Up @@ -44,23 +44,19 @@ import wrapAsync from './internal/wrapAsync';
export default function reflect(fn) {
var _fn = wrapAsync(fn);
return initialParams(function reflectOn(args, reflectCallback) {
args.push(rest(function callback(err, cbArgs) {
args.push(function callback(err, cbArg) {
if (err) {
reflectCallback(null, {
error: err
});
} else {
var value = null;
if (cbArgs.length === 1) {
value = cbArgs[0];
} else if (cbArgs.length > 1) {
value = cbArgs;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I prefer this condition, except removing the else if and replacing with an else

var value = cbArg;
if (arguments.length > 2) value = slice(arguments, 1);
reflectCallback(null, {
value: value
});
}
}));
});

return _fn.apply(this, args);
});
Expand Down
6 changes: 4 additions & 2 deletions lib/seq.js
@@ -1,5 +1,6 @@
import noop from 'lodash/noop';
import rest from './internal/rest';
import slice from 'lodash/slice';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as before, sorry, is it possible to use ./internal/slice here?

import reduce from './reduce';
import wrapAsync from './internal/wrapAsync';
import arrayMap from 'lodash/_arrayMap';
Expand Down Expand Up @@ -55,9 +56,10 @@ export default rest(function seq(functions) {
}

reduce(_functions, args, function(newargs, fn, cb) {
fn.apply(that, newargs.concat(rest(function(err, nextargs) {
fn.apply(that, newargs.concat(function(err/*, nextargs...*/) {
var nextargs = slice(arguments, 1);
cb(err, nextargs);
})));
}));
},
function(err, results) {
cb.apply(that, [err].concat(results));
Expand Down
11 changes: 6 additions & 5 deletions lib/waterfall.js
@@ -1,7 +1,7 @@
import isArray from 'lodash/isArray';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really nice cleanup of waterfall ⭐

import noop from 'lodash/noop';
import once from './internal/once';
import rest from './internal/rest';
import slice from './internal/slice';

import onlyOnce from './internal/onlyOnce';
import wrapAsync from './internal/wrapAsync';
Expand Down Expand Up @@ -74,12 +74,13 @@ export default function(tasks, callback) {
return callback.apply(null, [null].concat(args));
}

var taskCallback = onlyOnce(rest(function(err, args) {
var taskCallback = onlyOnce(function(err/*, cbArgs...*/) {
var cbArgs = slice(arguments, 1);
if (err) {
return callback.apply(null, [err].concat(args));
return callback.apply(null, [err].concat(cbArgs));
}
nextTask(args);
}));
nextTask(cbArgs);
});

args.push(taskCallback);

Expand Down
8 changes: 4 additions & 4 deletions lib/whilst.js
@@ -1,5 +1,5 @@
import noop from 'lodash/noop';
import rest from './internal/rest';
import slice from './internal/slice';

import onlyOnce from './internal/onlyOnce';
import wrapAsync from './internal/wrapAsync';
Expand Down Expand Up @@ -42,10 +42,10 @@ export default function whilst(test, iteratee, callback) {
callback = onlyOnce(callback || noop);
var _iteratee = wrapAsync(iteratee);
if (!test()) return callback(null);
var next = rest(function(err, args) {
var next = function(err/*, args...*/) {
if (err) return callback(err);
if (test()) return _iteratee(next);
callback.apply(null, [null].concat(args));
});
callback.apply(null, [null].concat(slice(arguments, 1)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I long for splats 💨

};
_iteratee(next);
}
22 changes: 22 additions & 0 deletions perf/suites.js
Expand Up @@ -233,6 +233,28 @@ module.exports = [{
fn: function(async, done) {
async.waterfall(tasks, done);
}
}, {
name: "auto",
args: [
[5],
[10],
[100]
],
setup: function setup(count) {
tasks = {
dep1: function (cb) { cb(null, 1); }
};
_.times(count, function(n) {
var task = ['dep' + (n+1), function(results, cb) {
setImmediate(cb, null, n);
}];
if (n > 2) task.unshift('dep' + n);
tasks['dep' + (n+2)] = task;
});
},
fn: function(async, done) {
async.auto(tasks, done);
}
}, {
name: "queue",
args: [
Expand Down