Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MatPaginatedTabHeader has performance problems (forced synchronous layout). #17317

Closed
pkozlowski-opensource opened this issue Oct 7, 2019 · 2 comments · Fixed by #17409 or hrueger/AGLight#112
Closed

MatPaginatedTabHeader has performance problems (forced synchronous layout). #17317

pkozlowski-opensource opened this issue Oct 7, 2019 · 2 comments · Fixed by #17409 or hrueger/AGLight#112
Assignees
@crisbeto
Labels
P2 The issue is important to a large percentage of users, with a workaround perf This issue is related to performance

Comments

@pkozlowski-opensource
Copy link
Member

pkozlowski-opensource commented Oct 7, 2019 •
edited
Loading

Reproduction

Steps to reproduce:

  1. open Chrome dev tools / Performance
  2. start recording a profile and go to https://next.angular.io/guide/router

Expected Behavior

MatPaginatedTabHeader should not be the perf hot spot / bottleneck for AIO

Actual Behavior

The _checkPaginationEnabled methods triggers a forced synchronous layout (by doing const isEnabled = this._tabList.nativeElement.scrollWidth > this._elementRef.nativeElement.offsetWidth;.

This forced synchronous layout accounts for ~38% of the entire JS processing time on the critical TTI path (see attached recording). Similar bottlenecks were spotted on other profiled sites (ex. https://www.mustakbil.com/)

Dev tools profile recording
@Splaktar Splaktar added the perf This issue is related to performance label Oct 7, 2019
@crisbeto
Copy link
Member

crisbeto commented Oct 7, 2019

I spent some time profiling in against our dev app and it's not just _checkPaginationEnabled that's causing the issues, there's also _checkScrollingControls and _alignInkBarToSelectedTab that trigger similar reflows. I don't think we can do much about it without breaking existing use cases. Our best bet might be to allow consumers to opt out of pagination, if they know that they won't need it.

@gkalpak gkalpak mentioned this issue Oct 9, 2019
Closed
3 tasks
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 15, 2019
@crisbeto
feat(tabs): add input to opt out of pagination
38f5300 
Currently the tabs pagination works automatically by measuring the size of the tab header to figure out whether to show pagination. This measuring can be expensive because it triggers a page layout and might not necessarily be required if the page won't have enough tabs to paginate through.

These changes add an input and an option to the injection token to allow consumers to opt out of the pagination, if they know that they won't need it.

Fixes angular#17317.
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 15, 2019
@crisbeto
feat(tabs): add input to opt out of pagination
943afd1 
Currently the tabs pagination works automatically by measuring the size of the tab header to figure out whether to show pagination. This measuring can be expensive because it triggers a page layout and might not necessarily be required if the page won't have enough tabs to paginate through.

These changes add an input and an option to the injection token to allow consumers to opt out of the pagination, if they know that they won't need it.

Fixes angular#17317.
@crisbeto crisbeto mentioned this issue Oct 15, 2019
Merged
@crisbeto crisbeto self-assigned this Oct 15, 2019
@crisbeto crisbeto added has pr P2 The issue is important to a large percentage of users, with a workaround labels Oct 15, 2019
@surajtambe surajtambe mentioned this issue Oct 16, 2019
Closed
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 16, 2019
@crisbeto
feat(tabs): add input to opt out of pagination
98d4e80 
Currently the tabs pagination works automatically by measuring the size of the tab header to figure out whether to show pagination. This measuring can be expensive because it triggers a page layout and might not necessarily be required if the page won't have enough tabs to paginate through.

These changes add an input and an option to the injection token to allow consumers to opt out of the pagination, if they know that they won't need it.

Fixes angular#17317.
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 16, 2019
@crisbeto
feat(tabs): add input to opt out of pagination
23ac073 
Currently the tabs pagination works automatically by measuring the size of the tab header to figure out whether to show pagination. This measuring can be expensive because it triggers a page layout and might not necessarily be required if the page won't have enough tabs to paginate through.

These changes add an input and an option to the injection token to allow consumers to opt out of the pagination, if they know that they won't need it.

Fixes angular#17317.
mmalerba pushed a commit that referenced this issue Oct 16, 2019
@crisbeto @mmalerba
feat(tabs): add input to opt out of pagination
800d663 
Currently the tabs pagination works automatically by measuring the size of the tab header to figure out whether to show pagination. This measuring can be expensive because it triggers a page layout and might not necessarily be required if the page won't have enough tabs to paginate through.

These changes add an input and an option to the injection token to allow consumers to opt out of the pagination, if they know that they won't need it.

Fixes #17317.
mmalerba pushed a commit that referenced this issue Oct 17, 2019
@crisbeto @mmalerba
feat(tabs): add input to opt out of pagination (#17409)
bb9a3a8 
Currently the tabs pagination works automatically by measuring the size of the tab header to figure out whether to show pagination. This measuring can be expensive because it triggers a page layout and might not necessarily be required if the page won't have enough tabs to paginate through.

These changes add an input and an option to the injection token to allow consumers to opt out of the pagination, if they know that they won't need it.

Fixes #17317.
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@crisbeto crisbeto

Labels
P2 The issue is important to a large percentage of users, with a workaround perf This issue is related to performance
Projects
None yet
Milestone
No milestone
3 participants
@pkozlowski-opensource @Splaktar @crisbeto