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

feat: Allow max concurrency for parallel map with option #15

Merged
merged 5 commits into from Jun 25, 2022

Conversation

weswigham
Copy link
Contributor

@weswigham weswigham commented Jul 23, 2019

Makes map aware of a NAL_MAX_CONCURRENCY environment variable which, when set to a positive integer, controls the maximum number of concurrent iterations which may be in-flight at once.

My usecase here is that I'd like to see a deterministic build log output order from a build I don't really control, which is run with either gulp or just (both of which use undertaker, which uses bach which uses this for its parallel mode). To do so, I'd like to set the maximum allowable concurrency to 1 concurrent operation at a time, this way the output log order isn't affected by the runtime of various operations. lerna and rush both allow doing this at the monorepo level, however there's no way to do it down at the individual task-runner level yet.

If this would be a welcome change to now-and-later, I can add some tests and documentation, or, if you think a "no concurrency" type feature would be better served in undertaker itself, I could look there, too (although at any layer above this you pretty much just get a yes/no, not a configurable maximum count, unless you amend now-and-later's API to take an optional concurrency limit parameter).

@weswigham
Copy link
Contributor Author

Huh. I am unsure what I am doing that would only fail on node 0.10... Is there some old v8 bug I'm triggering?

@phated
Copy link
Member

phated commented Jul 28, 2019

@weswigham I think it's useful to support max concurrency in here but I don't want to use an environment variable. Can you change the PR to pass in maxConcurrency as an option?

lib/map.js Show resolved Hide resolved
@weswigham
Copy link
Contributor Author

@phated should the option be a member of the extensions object?

@phated
Copy link
Member

phated commented Jul 28, 2019

@weswigham yes, I think we can include it in the extensions object and then I will rename them in the docs.

@phated phated added this to To do in v5 Apr 28, 2020
@phated phated moved this from To do to In progress in v5 Oct 21, 2020
weswigham and others added 5 commits June 24, 2022 18:17
Makes `map` aware of a `NAL_MAX_CONCURRENCY` environment variable which, when set to a positive integer, controls the maximum number of concurrent iterations which may be in-flight at once.
Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

This PR seemed to have stalled, so I updated it with my desired API, changed extensions to options throughout, added tests and docs.

I also rebased this on the new project template updates.

If everything is green, I'll get this merged 🎉 Thanks for sending over the proposal @weswigham

@phated phated changed the title Proposal: Configurable maximum concurrency via NAL_MAX_CONCURRENCY feat: Allow max concurrency for parallel map with option Jun 25, 2022
@phated phated merged commit 92ae356 into gulpjs:master Jun 25, 2022
@weswigham weswigham deleted the patch-1 branch June 25, 2022 04:53
@phated phated moved this from In progress to Done in v5 Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v5
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants