[perf] NgZone: runaway change detections

[mleibman]

Summary

Using NgZone.run() can in some cases result in a large number of change detections performed synchronously, creating a serious performance issue.

Repro

https://stackblitz.com/edit/angular-5zht6w

this.ngZone.runOutsideAngular(() => {
  setTimeout(() => {

    // This code results in one change detection occurring per
    // ngZone.run() call. This is entirely feasible, and can be a serious
    // performance issue.
    for (let i = 0; i < 100; i++) {
      this.ngZone.run(() => {
        // do something
      });
    }

  });
});

Description

NgZone runs change detection as soon as the execution leaves the top-most NgZone call and all the microtasks there have finished, but if there are more than one top-level calls to NgZone.run() in a single VM turn, a synchronous change detection is run after each one.

The example above may seem somewhat contrived, but this situation is quite easy to get into w/o even noticing. For example, you may have a window.onmessage event handler that receives data from a worker or an IFRAME. The handler then iterates through the array of received objects, invoking various callbacks or emitting observable values for each one. If one of those callbacks or subscribers is part of an Angular app and needs to update something that should be picked up by Angular's change detection, they would wrap it in the NgZone.run() call.

[JiaLiPassion]

@mleibman , I am not sure I understand the issue completely. So the case may look like.

window.ommessage(data => {
  data.forEach(d => {
    subject.emit(d); // this will trigger ngZone.run() in the angular app.
  });
});

1. And if the ngZone.run() is nested, the ngZone.run() in the loop will not really trigger the change detection.
   In this specified case, the window.onmessage is also patched by zone, so the callback of window.onmessage is also a ngZone.run(), so the ngZone.run() in the data loop will not trigger the change detection.

2. In other case without the top ngZone.run, you are suggesting we run a top ngZone.run() to prevent this situation?

[mleibman]

1. Is window.onmessage really patched? IIRC, it isn't since there's no way to match sent and received messages... Anyway, let's assume it isn't for this particular example. If the subject subscriber works around that by doing its own NgZone.run(), this will indeed result in one CD per iteration. If window.onmessage is patched, then yes, this won't result in any extra CDs.

2. That's not always possible or desirable... What if we don't want the top-level call to be run in NgZone, and only lower-level functions are responsible for deciding that? We could first run in NgZone and then immediately run it outside of it, but that would result in a CD happening even if it wouldn't otherwise (i.e. if there are no nested NgZone.run() calls).

We've actually hit this issue several times in different scenarios that are too specific to describe here, so I tried to come up with something simple and realistic. Ideally, this could be fixed in NgZone itself.

Does CD have to be synchronous, or can it be scheduled to run at the end of a VM turn in a microtask?

[JiaLiPassion]

Does CD have to be synchronous, or can it be scheduled to run at the end of a VM turn in a microtask?

Currently there is an option ngZoneEventCoalescing, if you set it to true when bootstrap

platformBrowserDynamic().bootstrapModule(AppModule, {ngZoneEventCoalescing: true})

then all change detections caused by the event tasks (event listeners and onxxx callbacks) will run once as a animationFrame tick.

[mleibman]

I tried ngZoneEventCoalescing, but, as expected, it has no effect in this case.
That option is actually enabled in the linked repro on StackBlitz.
Keep in mind, we're executing a single ZoneJS task here. We're simply executing it in the root zone, and then entering NgZone multiple times. But perhaps a similar approach (scheduling a CD as a microtask) could be used?

[JiaLiPassion]

Yes, you are right, and sure I think we can use the similar approach, since currently markDirty() is scheduling and coalescing the CD in the animationFrame.

[Achilles1515]

I raised a similar question in the Angular Discord a couple weeks ago, but didn't get a good response. My question was-

If I am running outside the Angular zone and want to re-enter Angular zone, say to just attach an event listener, how can this be done without triggering change detection? Because in this case, all I want to do is dynamically attach an event listener to an element inside Angular zone and there is absolutely no need for change detection to run immediately afterward (i.e. after calling ngZone.run()).

I ended up writing this dirty helper function that patches out appRef.tick() for a moment:

runWithoutTick<T>(fn: (...args: any[]) => T, appRef: ApplicationRef, ngZone: NgZone, applyThis?: any, applyArgs?: any[]): T {
    const originalTick = appRef.tick;

    try {
      const patchedFunction = () => {
        // First, call user function.
        const x = fn.call(applyThis, applyArgs);

        // Next, disable change detection after running user function, effectively making sure
        // `ngZone.run()` does not trigger change detection.
        appRef.tick = () => {};
        return x;
      };
      return ngZone.run(patchedFunction, applyThis, applyArgs);
    } finally {
      // Finally, restore change detection.
      appRef.tick = originalTick;
    }
  }

@JiaLiPassion Is there a better or existing way to solve this problem?

[mleibman]

@Achilles1515 if all you want to do is attach an event listener that will run in NgZone, you can do that outside of NgZone, and just wrap the listener:

// some code running outside of NgZone

// this will result in a change detection
this.ngZone.run(() => {
  this.element.addEventListener('click', this.onClick);
});

// this does not result in a change detection, but the listener still runs in NgZone
this.element.addEventListener('click', () => {
  this.ngZone.run(() => this.onClick);
});

[Achilles1515]

@mleibman For that example, that is a good solution, assuming I am in charge of the callback code. But it was more just a general statement - when going from outside Angular zone to inside Angular zone and back out, why is a change detection cycle always necessary (and forced)?

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}