Performance issues because of _ngZone.onStable.subscribe

[cwayfinder]

Bug description:

There are a few components in this library with a line

_ngZone.onStable.subscribe(() => this._updatePopupPosition());

in the constructor (dropdown, tooltip, popover, typeahead, datepicker).

_ngZone.onStable is fired when the microtask queue is empty and Angular does not plan to enqueue any more. In my application, this happens too many times and too frequent and leads to freezes in Safari and Firefox.

Let's think about a more sophisticated approach to reposition a popup in order to optimise performance.

The minimal example to reproduce the issue

import { Component, OnInit } from '@angular/core';

@Component({
    selector: 'app-root',
    template: `
        <div class="dropdown" ngbDropdown>
            <a class="dropdown-toggle" ngbDropdownToggle>111</a>
            <div ngbDropdownMenu>
                <div>111</div>
                <div>222</div>
                <div>333</div>
            </div>
        </div>
    `,
    styleUrls: ['./app.component.scss']
})
export class AppComponent implements OnInit {

    ngOnInit(): void {
        setInterval(() => {
            console.log('interval');
        }, 100);
    }
}

When you open the dropdown menu it starts doing repositioning every 100 ms.

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 7.2.15
ng-bootstrap: 4.1.3

[cwayfinder]

I found how the same thing is done in Angular Material https://github.com/angular/components/blob/master/src/cdk/overlay/overlay-ref.ts#L136
They use take(1).

[benouat]

Hey @cwayfinder !

Would you mind providing a reproducible example via Stackblitz that demonstrate the performance issue ?

[cwayfinder]

Hey @benouat, I added an example. It's quite easy to reproduce.

[maxokorokov]

@cwayfinder I think Material has a dedicated PositionStrategy for repositioning (not 100% sure), what you're referencing is only for initial positioning. We have a more naive, but generic implementation at the moment, exactly as you're saying.

Firstly, regardless of this issue, I guess, you shouldn't be triggering change detection that often.

#3206 might improve things a little in your case for Safari and Firefox, I've seen quite a lot of improvement.

To position the popup we're trying things from [placement] one-by-one, something like:

for each placement 'p' in [top, left, right, bottom, etc] {
   1. apply appropriate css class for 'p' to the popup
   2. calculate position, apply 'translate'
   3. check positioned popup fits in the viewport ← this forces style recalculation (and might trigger layout)
   4. stop if it fits
}

I'm not aware of another way of checking positioning at the moment. Also in my simple tests repositioning is pretty fast, and I don't see this as a bottleneck at the moment. It never takes longer than a millisecond, unless you're trying many positions before you finding one that fits and doing style recalculation several times

---

So, could you please provide a performance timeline from Chrome as JSON for your use case (if possible to share for your application) to see the effects of positining ? If not at least a screenshot would be helpful.

---

Ideally we should allow customizing it:

• position only ONCE when opening
• reposition after this using various strategies → ex. never, on stable, every XX milliseconds, when parent scrolls, etc.

cc @fbasso

[cwayfinder]

@maxokorokov it is a bottleneck if you use some third-party things (e.g. https://github.com/tangrams/tangram) which runs some task 60 times per second.

I decorated the original _positionMenu method this way

@Directive({
	selector: '[ngbDropdown]'
})
export class NgbDropdownMonkeyPatchDirective {
	constructor(@Host() @Self() @Optional() public ngbDropdown: NgbDropdown) {
		const originalMethod = ngbDropdown['_positionMenu'].bind(ngbDropdown);

		ngbDropdown['_positionMenu'] = () => {
			console.log('_positionMenu');
			originalMethod();
		};
	}
}

And I get such situation in Chrome

That's crazy!

[fbasso]

Hi @cwayfinder, you have a component triggering the change detection 60 times per second, sure it causes some performance issues. Wouldn't be possible to run your third party library outside the Angular zone (https://angular.io/api/core/NgZone#description) ?

[ymeine]

Hello @cwayfinder,

As mentioned by @fbasso, have you tried instantiating this third-party library outside of the Angular zone?

More precisely, you should ensure than any intensive (frequent) processing is scheduled outside of the Angular zone (I don't know the details of the API of this library): runOutsideAngular.

If you ever need to react to some updates done by the library, use its API (plug into some events or whatever it is) to update your application data model, or anything, inside of the Angular zone again: run.

It is true that our positioning is an expensive task, but there should not be so many change detection cycles triggered, this is a more general concern for any Angular application. EVERYTHING depending on change detection is recomputed every time. This includes other parts of our code but also your code.

The proposed solution of caching (#3214) is an idea of improvement, but proper caching needs proper cache invalidation, and in this case, to be properly done in a generic way, this would still require to calculate positions and sizes. #3214 makes some strong hypotheses to avoid doing so, but I would not be in favor of a restricted solution for an issue which could be circumvented more properly.