Rewrite pop() to be non-recursive? #4

Closed
ZJONSSON opened this Issue Aug 10, 2012 · 3 comments

Comments

Projects
None yet
2 participants

I'm a "long-time" user of Queue and think it's a very elegant framework to manage control over multiple callbacks.
I did two modifications which I wanted to share. I'm not sure of the pros and cons, but they seem to work.

The first one allows me to specify the "await" before any "defer" without risking premature zero results.

Line 33:
if (!remaining) await(error, results);
->
setTimeout(function() { if (!remaining) await(error, results) },0)

And the second change is to circumvent any potential Maximum Recursions by putting a similar timeout on the pop() in defer:
Line 26:
pop()
->
setTimeout(pop,0);

Owner

mbostock commented Jan 23, 2013

Seems like a good idea to rewrite pop() to not be recursive, but I don’t have the spare brain power to think through at the moment how that would be done. I’d rather not resort to setTimeout(pop, 0) because that is quite expensive. On Node.js, it’d be easy to switch to process.nextTick, but I’d like Queue to function consistently in the browser.

Also, I think it’s a feature (although a debatable one) that queue.await invokes immediately if all deferred tasks have completed. In theory you could reuse the same queue multiple times, although I'm not sure how well that is supported at the moment. Related: I way of canceling deferred tasks that have not started and inserting deferred tasks at the head of the queue rather than the tail would be helpful.

ZJONSSON commented Feb 5, 2013

Agreed. There is a hack (using windows.postmessage) that delivers a more efficient solution but I'm unsure if it's fit to be a part of a standard library or not: http://jsperf.com/nexttick-vs-setzerotimeout-vs-settimeout/ An efficient processNext() in a browser would nevertheless open up a whole new range of interesting code patterns, i.e. "execute this, when the chain is done" type of logic.

ZJONSSON closed this Feb 5, 2013

Owner

mbostock commented Mar 31, 2013

This is fixed in 7bd5059.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment