Flex-layout: Possible performance issue with double forced reflow

Created on 5 Jun 2018  路  5Comments  路  Source: angular/flex-layout

Bug, feature request, or proposal:

Bug/performance proposal

What is the expected behavior?

Not having two forced reflows on directive initialisation

What is the current behaviour?

Two (or more) forced reflows on directive initialisation

What are the steps to reproduce?

One example is in the fxLayoutDirective. You can see the _updateWithDirection being called inside the ngOnChanges and after that in the ngOnInit.

I might be wrong, but from what I can tell from the profiler this calls the heavy _updateWithDirection twice on initalisation. Because you initialise the directive by using the layout input, it will always go through the ngOnChanges hook anyways. Which makes this line obsolete, or better yet, causing unnecessary recalculations.

I noticed in the profiler that these updates are pretty intensive for the cpu, by causing forced reflows

Other examples:

FlexAlignDirective._updateWithValue
FlexOrderDirective._updateWithValue
FlexDirective._updateStyle
FlexLayoutAlignDirective._updateWithValue

Instead of removing the corresponding lines, you could also check inside the ngOnChanges if the ngOnInit hook has already ran. And if so, execute the function. This needs to be done for the LayoutGapDirective anyways, because it needs to be executed inside the ngAfterViewInit.

Another thing that I noticed is the cpu hungry _getDisplayStyle inside every directive that extends the BaseDirective. If the current element does not use the media queries .xs, .xl ..., how necessary is this call?

What is the use-case or motivation for changing an existing behaviour?

To have a faster flex framework

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

All

Is there anything else we should know?

Like I said, my assumptions might be wrong, and there is actually a valid reason for executing it twice.

P3 has pr performance

Most helpful comment

Nice, just an easy benchmark shows that the flex-layout library is now 75% faster 馃憤

All 5 comments

@PierreDuc Thank you for tracking this down. I've removed the duplicate function calls in #773. As for the calls to _getDisplayStyle, can you provide a profiling breakdown of where it's called, and how it's affecting performance? It's called primarily to determine show/hide or if there is a flex-direction present on a parent container.

@CaerusKaru It's seems like that _display is only used in the ShowHideDirective, but it's set in the ngOnInit of every directive that extends the BaseDirective, by calling _getDisplayStyle. So perhaps it's better to move this call to just ShowHideDirective.

Excellent suggestion, it's been moved in the latest commit on the PR.

Nice, just an easy benchmark shows that the flex-layout library is now 75% faster 馃憤

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._

Was this page helpful?
0 / 5 - 0 ratings