Permalink
Browse files

Tweaking `nextTick` implementation.

  • Loading branch information...
1 parent d27c0fb commit 3175d1cfa2596ab2caf9c562f395b15377fcddf6 @domenic domenic committed Apr 25, 2012
Showing with 21 additions and 12 deletions.
  1. +21 −12 q.js
View
33 q.js
@@ -35,6 +35,14 @@
})(function (exports) {
"use strict";
+var taskQueue = [];
@kriskowal

kriskowal Apr 25, 2012

Owner

I am curious whether an array or a linked list would run faster here. My theory is that array.shift() is O(array.length) and that a linked list would get JITed down to structs.

@domenic

domenic Apr 25, 2012

Collaborator

Good point, hmm. Seems very JITter dependent. Wonder if this is decidable with jsperf.

@Yaffle

Yaffle Apr 25, 2012

for Chrome shift+push works fast,
but it is implementation dependent, while linked list is fast everywhere

+function runQueuedTasks() {
+ var task;
+ while ((task = taskQueue.shift())) {
+ task();
@kriskowal

kriskowal Apr 25, 2012

Owner

Would an exception thrown here halt the processing of subsequent tasks?

@domenic

domenic Apr 25, 2012

Collaborator

Indeed it would; that's bad. I wonder if the deoptimization from try/catch would overwhelm the benefit of batching, at least in the MessageChannel case. In the setTimeout(, 0) case I'm sure this would still come out ahead.

But hmm, then you have to re-throw the errors in the next tick, so ugh, this is getting worse and worse.

@kriskowal

kriskowal Apr 25, 2012

Owner

Good point about MessageChannel. Batching might not benefit that approach. Of course everything in a Q callback is already suffering the try/catch penalty once. I wonder whether they compound. In any case, perhaps it is not worth pursuing after all.

@domenic

domenic Apr 25, 2012

Collaborator

That's true, and they might not compound. (In my uninformed mental model, it just flips the "be exception-aware" bit.) I think for setTimeout it's definitely worth pursuing. It does make the existing nextTick(function () { throw error; }) code a bit comical, though.

@kriskowal

kriskowal Apr 25, 2012

Owner

Rethrowing exceptions in future turns to avoid being caught…is already a bit of dark humor. To be caught and rethrown in that future turn just makes it darker and more humorous.

@domenic

domenic Apr 25, 2012

Collaborator

Hah, well put. I'll go for it. The next big thing I want to tackle is #57 anyway, so that would hide all the dark humor far, far from the light of day.

@domenic

domenic Apr 25, 2012

Collaborator

Bah. Now the following code is out of order:

nextTick2(console.log.bind(console, "1"));
nextTick2(function () { throw new RangeError("ahsdhfkdlf"); });
nextTick2(console.log.bind(console, "3"));

With the current implementation in master, you get 1, RangeError, 2 in the console. With a version of this that involves catching and rethrowing, you get 1, 3, error.

Think it's time to abandon this approach :(

@kriskowal

kriskowal Apr 25, 2012

Owner

Yeah, I am far more anxious to see #57. It is a game changer.

+ }
+}
+
var nextTick;
if (typeof process !== "undefined") {
// node
@@ -49,22 +57,23 @@ if (typeof process !== "undefined") {
// modern browsers
// http://www.nonblocking.io/2011/06/windownexttick.html
var channel = new MessageChannel();
- // linked list of tasks (single, with head node)
- var head = {}, tail = head;
- channel.port1.onmessage = function () {
- var next = head.next;
- var task = next.task;
- head = next;
- task();
- };
+ channel.port1.onmessage = runQueuedTasks;
+
nextTick = function (task) {
- tail = tail.next = {task: task};
- channel.port2.postMessage(0);
+ taskQueue.push(task);
+
+ if (taskQueue.length === 1) {
+ channel.port2.postMessage(0);
+ }
};
} else {
- // old browsers
+ // old browsers, with a trick to batch tasks if called multiple times in the same turn
nextTick = function (task) {
- setTimeout(task, 0);
+ taskQueue.push(task);
+
+ if (taskQueue.length === 1) {
+ setTimeout(runQueuedTasks, 0);
+ }
};
}

0 comments on commit 3175d1c

Please sign in to comment.