Skip to content

[RFC]: add utils/async/parallel #1811

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

Closed
3 tasks done
nightknighto opened this issue Mar 9, 2024 · 11 comments · Fixed by #1896
Closed
3 tasks done

[RFC]: add utils/async/parallel #1811

nightknighto opened this issue Mar 9, 2024 · 11 comments · Fixed by #1896
Assignees
Labels
Accepted RFC feature request which has been accepted. difficulty: 3 Likely to be challenging but manageable. Feature Issue or pull request for adding a new feature. JavaScript Issue involves or relates to JavaScript. priority: Normal Normal priority concern or feature request. RFC Request for comments. Feature requests and proposed changes. Utilities Issue or pull request concerning general utilities.

Comments

@nightknighto
Copy link
Contributor

nightknighto commented Mar 9, 2024

Description

This RFC proposes adding utils/async/parallel to run a set of asynchronous code in parallel, corresponding to async.js parallel and async.js parallelLimit

Related Issues

Related to the project idea stdlib-js/google-summer-of-code#9

Questions

No.

Other

The implementation will be inspired by utils/async/map-values.

The API interface would be parallel( functionArray [, options] [, done] ).

Params:

  • functionArray: an array of functions to be executed in parallel. The functions should be async, or else they would run sequentially.
  • options will accept:
    • thisArg: execution context
    • limit: maximum number of pending invocations with default infinity. (Will replace the need for a separate function for parallelLimit)
  • done: a callback function with (err, result) parameters that is called when the execution is finished.
    • err: will be truthy if an error happened, for error-handling.
    • result: will be an array of the return values of each function.

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@stdlib-bot
Copy link
Contributor

👋 Hi there! 👋

And thank you for opening your first issue! We will get back to you shortly. 🏃 💨

@nightknighto
Copy link
Contributor Author

I'd like to work on it ASAP as part of my proposal for the mentioned project.

@kgryte kgryte added RFC Request for comments. Feature requests and proposed changes. Feature Issue or pull request for adding a new feature. difficulty: 3 Likely to be challenging but manageable. Utilities Issue or pull request concerning general utilities. JavaScript Issue involves or relates to JavaScript. priority: Normal Normal priority concern or feature request. labels Mar 9, 2024
@kgryte
Copy link
Member

kgryte commented Mar 9, 2024

@Deadreyo This is looking good. I suggest making done a mandatory argument. In your OP, you have it as optional, based on the proposed signature.

The functions should be async, or else they would run sequentially.

I don't think this should be required, and I don't think we have a way to enforce this. So long as each provided function calls a provided callback function and we invoke done on completion, that is the best we can do. Up to consumers to avoid releasing the zalgo.

options.limit

This makes sense to me. I don't see an imperative to have two separate APIs.

functionArray

Should we also support providing an object, similar to async.js or only an array-like object containing functions? I can see the argument for supporting both.

@kgryte kgryte added the Needs Discussion Needs further discussion. label Mar 9, 2024
@nightknighto
Copy link
Contributor Author

I agree with all your points.

Should we also support providing an object, similar to async.js or only an array-like object containing functions? I can see the argument for supporting both.

Comparing with waterfall as I consider these two to be of the same class, waterfall only supports the array. I suggest we have them consistent in this aspect, and if needed later on we can add support for both together.

@kgryte
Copy link
Member

kgryte commented Mar 9, 2024

For waterfall, it makes sense to only support an input array, as there's no "results" value. It's different with parallel, where you want the results of all async functions. And there I can see an argument for "naming" the results. That same argument does not apply to waterfall.

@nightknighto
Copy link
Contributor Author

nightknighto commented Mar 9, 2024

Alright I now understand what you mean, this example:

async.parallel({
    one: function(callback) {
        setTimeout(function() {
            callback(null, 1);
        }, 200);
    },
    two: function(callback) {
        setTimeout(function() {
            callback(null, 2);
        }, 100);
    }
}, function(err, results) {
    console.log(results);
    // results is equal to: { one: 1, two: 2 }
});

It's fine by me either of the decisions, implementing both at this moment, or implementing only the array now and pushing the object to a separate issue/task, where we can discuss the arguments for both sides more freely.

@nightknighto
Copy link
Contributor Author

For the functions in the functionArray, they should have a resolve or callback function which acts like return like the example above.
To ensure some kind of proper use, we can do some checks on each of provided functions:

  • If number of parameters is less than 1, throw an error
  • If number of parameters is 1 or more, pass the callback in the first param. An alternative would be to throw an error if more than 1 param but I think it should be more flexible.

@kgryte
Copy link
Member

kgryte commented Mar 10, 2024

I don't think we should require a particular arity for functions in the function array. Users should be able to provide a function of any arity; we don't really care. We always pass in a clbk function to each function in the function array which has the signature:

// Callback function provided by the `parallel` implementation to each function in the function array...
function clbk( error, result ) {
	if ( error ) {
		// ... handle error
		return;
	}
	// ...handle result
}

The onus is on users to ensure they call the callback. And if the arity is zero, they are responsible for extracting the callback from arguments.

@nightknighto
Copy link
Contributor Author

Great. I think everything is set. I will start on it

@kgryte
Copy link
Member

kgryte commented Mar 10, 2024

Awesome. Thanks!

@kgryte kgryte added Accepted RFC feature request which has been accepted. and removed Needs Discussion Needs further discussion. labels Mar 10, 2024
@nightknighto
Copy link
Contributor Author

Finished and opened the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted RFC feature request which has been accepted. difficulty: 3 Likely to be challenging but manageable. Feature Issue or pull request for adding a new feature. JavaScript Issue involves or relates to JavaScript. priority: Normal Normal priority concern or feature request. RFC Request for comments. Feature requests and proposed changes. Utilities Issue or pull request concerning general utilities.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants