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.

Sign up for GitHub

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

Jump to bottom

Minor performance issue #758

Closed
DotsElshad opened this issue May 19, 2015 · 2 comments
Closed

Minor performance issue #758

DotsElshad opened this issue May 19, 2015 · 2 comments
Labels
bug performance

Comments

@DotsElshad
Copy link

Hello,

when going from 0.9.0 to 0.9.2 I noticed the following change: f5d804f

The problem here, is that q.process() is called "q.concurrency" times. But it should suffice to call it Math.min(q.tasks.length, q.concurrency) times.

OK, this is a minor performance problem. But if q.concurrency happens to be Infinity then it actually produces an endless loop. Which it didn't before. Arguably, "concurrency = Infinity" is not a very sensible case.
@aearly
Copy link
Collaborator

aearly commented May 19, 2015

Thanks for the code review. We'll fix this shortly

@DotsElshad
Copy link
Author

Thank you very much!
It lead to a bug in our software, but it was wrong usage of "async" on our side ("concurrency = Infinity" really doesn't make much of a sense).

@aearly aearly closed this as completed in 91f6fb3 May 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug performance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@aearly @DotsElshad