Skip to content

Conversation

@aearly
Copy link
Collaborator

@aearly aearly commented Jan 20, 2015

From the discussion in #696 , this PR removes most internal setImmediates to remove the unnecessary performance hit when you're using asynchronous functions. It leaves it to the user to guard against stack overflows with async.setImmediate or async.nextTick if they need to callback synchronously. This is a breaking change since there are many people relying on automatic deferral in waterfall and auto.

I had to refactor the logic in the functions that used setImmediate because the internal logic was relying on those deferrals. The timing of certain callbacks and events has also changed, so there will be further subtle breaking changes there. If the functions you pass to async were actually asynchronous, things should work as they always have.

I left the deferrals in for priorityQueue and cargo, because those seemed to be needed for them to actually work. They need to defer in order to become saturated so the priorities and payload sizes can take effect.

I also see that queue, priorityQueue, and cargo could likely be refactored to use the same underlying logic, albeit a bit more complicated. queue would have a payload size as well as a concurrency, and q.push would have an optional priority parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

In order to match the example above, I think this should be changed to:

if (matchesSomething(item)) {

@aearly aearly added this to the 2.0 milestone Jul 2, 2015
@aearly
Copy link
Collaborator Author

aearly commented Oct 25, 2015

Closing this for now. The doc updates were already added. Things have diverged so much this PR isn't useful -- we'll track this with #696 for v2.0.

@aearly aearly closed this Oct 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants