bug(perf): angular2/upgrade keeps running $rootScope.apply() if watchers call setTimeout

[andreialecu]

I'm trying to upgrade an existing angular1 app to angular2 using the angular2/upgrade functionality.

I have noticed that after using angular2/upgrade to bootstrap the app, I'm now getting angular1 digests on every single tick, and it tracks down to this line (discovered using the CPU profiler):

this.componentScope.$watch(() => this.changeDetector && this.changeDetector.detectChanges());

angular/modules/angular2/src/upgrade/downgrade_ng2_adapter.ts

Line 103 in 56a254e

this.componentScope.$watch(() => this.changeDetector && this.changeDetector.detectChanges());

I have an angular2 pipe that is configured as pure: false and its transform method seems to be executed on every single tick of the page. And since I'm using it inside an *ngFor and there are a lot of them on the page, the browser becomes unusable.

Here's a trace using Chrome's profiler

Am I doing something wrong or is this a bug?

EDIT: Repro at #6245 (comment)

[0x-r4bbit]

@andreialecu can you provide a plunk that demonstrates the issue. I have a guess but I'd like to double check first.

[andreialecu]

@PascalPrecht I'm trying to come up with a repro but I cannot seem to do it outside of my app.

I noticed that this is where the repeated angular1 digests start from, which in turn cause the cascading effect:

async_1.ObservableWrapper.subscribe(ngZone.onTurnDone, function (_) { ngZone.run(function () { return rootScope.$apply(); }); });

Transpiled from:

angular/modules/angular2/src/upgrade/upgrade_adapter.ts

Line 345 in 1b78342

ngZone.overrideOnTurnDone(() => rootScope.$apply());

I'm not sure what that subscription is supposed to do, but it keeps triggering every tick of the app. I may have something somewhere that is interfering, as this is a relatively big app, but I'm not sure what to look for.

Any clues as to what may be triggering that?

[0x-r4bbit]

So ngZone is the thing that monkey patches your environment and notifies Angular when an asynchronous operation has happened. It basically allows us to get rid off sth. like $rootScope.$apply.

However, when we upgrade apps using ngUpgrade we're running both frameworks side by side. Which means, there is an Angular 1 app that needs $rootScope.$apply in case something happened. This is not needed as long as you stick to ng1 world, however, as soon as you run an ng2 component inside your ng1 app, you need something that notifies A1 (ngZone) and cause it to trigger change detection ($rootScope.$apply()).

That's basically what this line of code does. It's hard to say without a plunk why this ends in an infinite loop in your case, but it sounds like there are changes happening over and over again out of each performed change detection. Since you're using a pipe, which is not stateless (pure:false) I'd guess that it returns a new collection every time it's executed, which is a change for Angular. But as mentioned, this is really just a guess, I think we need more of your code to verify that.

OTOH, Angular should actually throw if after the first change detection another changes happened to your expression. This also depends on the version you're using.

I think we really need a plunk that demonstrates your issue =/

[0x-r4bbit]

BTW: this is the error I'm talking about: #6005

[andreialecu]

@PascalPrecht after a lot of trial and error, I finally figured out what is causing this. It's related to setTimeout calls that trigger within watchers. I have a lot of third party angular1 components in this project which all have various timeouts like these.

Here's a repro:

http://plnkr.co/edit/rHGx6CoX2lKy6WVFMkFI?p=preview

Open the developer console to see the console.log inside the transform function go nuts.

[andreialecu]

Note how removing this block completely solves the problem:

    $rootScope.$watch(function() {
          _debounce || (_debounce = setTimeout(function() {
            _debounce = null;
          }, 100));
      });

[andreialecu]

Well, this seems to be completely blocking the upgrade for us. I'd attempt to submit a PR, but it seems like the core of ng-zone is affected. Might be an easy fix though... @mhevery ? :)

[andreialecu]

Changing upgrade_adapter.ts line 341:

ObservableWrapper.subscribe(ngZone.onTurnDone,
                                        (_) => { ngZone.run(() => rootScope.$apply()); });

To

ObservableWrapper.subscribe(ngZone.onTurnDone,
                                        (_) => { ngZone.runOutsideAngular(() => rootScope.$apply()); });

Fixes the infinite loop.

[andreialecu]

Looks like there are still various timers outside of the angular2 zone which get caught in the setTimeout hooks installed by zone.js and result in onTurnDone triggering and that block of code being executed, causing excessive angular 1 digests.

runOutsideAngular in that block of code above makes it slightly better, but does not resolve it completely.

This makes upgrading any non trivial app impossible for production purposes.

Guess everyone is on vacation still, but some acknowledgement here would be nice. The repro should be pretty clear.

[andreialecu]

Closing in favor or #6385 which has simpler repro.

[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.}