postDigestQueue poor performances

[AntoineEsteve]

One of our page is very heavy. To decrease our watcher count and to accelerate the angular digest cycle we are using a lot the On-Time-Binding syntax ::. We are also using angular-bind-notifier to avoid unnecessary watchs over our expressions on this page.

This strategy allowed us to reduce the angular digest cycle considerably.

But this strategy has one down side: It uses the $$postDigest (postDigestQueue) to unwatch the expression after it has been evaluated successfully.

So what ?

At the end of the digest, angular will run through the postDigestQueue. As we used a lot of On-Time-Binding expressions, our postDigestQueue can grow up to more than 100 000 queued tasks.

The problem is that angular uses the following code to loop over the queue:

while (postDigestQueue.length) {
  try {
    postDigestQueue.shift()();
  } catch (e) {
    $exceptionHandler(e);
  }
}

The shift method removes the element at the zeroeth index and shifts
the values at consecutive indexes down, then returns the removed
value.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/shift

Yes, Array.prototype.shift() is a very expensive call when there is a lot of elements in the array.

Our digest cycle will sometimes take more than 20 seconds because of that.

When we change the previous code with the following one, it is way faster:

for (var i = 0; i < postDigestQueue.length; i++) {
  try {
    postDigestQueue[i]();
  } catch (e) {
    $exceptionHandler(e);
  }
}
postDigestQueue.length = 0;

Is there a reason why they did that ?
Shouldn't we use that much the one time binding ?

I could see one reason: if a task add itself a need task in the queue. Is is possible ? ($$postDigest is a private queue) An answer would be to use popinstead of shift if the order of execution is not important, but is it ?

Edit: The order seems important because the postDigestQueue is used with the animations.

[Narretz]

One way to see if a change to the code causes any problems is to make the change in the code and the run the tests to see if any of them fail. You can open a PR with the change, and Travis will run the tests if you can't do it locally.

Generally I'd say if you have 100.000 queued tasks then even one-time binding won't help much. I'm not sure though if you really have 100.000 one-time bindings on the page. But that's definitely a lot.

[gkalpak]

The order of execution is definitely important. I guess the purpose of using shift is exactly to be able to add more items in the $$postDigest queue that will be executed before other tasks. Since this is a private API, I would be OK changing this "contract" if we are not using it internally.

If this provides any noticeable speedup, I beieve it is a change worth making. It is indeed a shame that one-time bindings cause a slow-down (even if just a tiny one).

_{FWIW, I agree with @Narretz, that 100K tasks sound like too many. But we shouldn't need 20s either 😃}

[thorn0]

Tests fail if I use this code:

for (var i = 0; i < postDigestQueue.length; i++) {
  try {
    postDigestQueue[i]();
  } catch (e) {
    $exceptionHandler(e);
  }
}
postDigestQueue.length = 0;

It happens because $digest can be invoked recursively inside a $$postDigest callback. In particular, it happens in the flushOnChangesQueue function in $compile. Everything works fine if a queue is used (see #14545).

[Narretz]

We merged a perf improvement that will be in the next 1.5.x release, see cb2f8c0 I don't have numbers but it should speed up large post digest queues