Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

md-tabs causes many $digest executions #3101

Closed
stevenmiles opened this issue Jun 3, 2015 · 14 comments
Closed

md-tabs causes many $digest executions #3101

stevenmiles opened this issue Jun 3, 2015 · 14 comments
Assignees
Milestone

Comments

@stevenmiles
Copy link

I've been suffering some performance problems with a fairly complex application in FF and IE. I found that my application was continually executing $digest. It seems that the digests are caused by md-tabs.

As the watch expressions are evaluated during a digest, the watch expression created by md-tabs that executes shouldPaginate() is executed continuously. In some profiling I did, shouldPaginate() accounted for the majority of the time in FF and IE. shouldPaginate() seems to be a fairly slow function which is amplified when it is continually being executed.

To measure digests I put a watch on $rootSope and logged out a message when the watch was evaluated. My assumption is the each evaluation of the watch is a $digest execution. Please correct me if my assumptions are wrong.

I measured by monitoring console output in the dev tools of various browsers. I found the following behaviours.

Chrome 43.0.2357.81 m
25ish $digests executions on initialization then 4-5 per tab change

Firefox 38.0.5
25ish $digests executions on initialization then continually executes $digest after first tab change

IE 11
Continually executes $digest after initialization.

OS: Windows 8.1
Angular Material: 0.9.7

http://codepen.io/stevenmiles/pen/yNMPzV

@stevenmiles stevenmiles changed the title md-tab causes many $digest executions md-tabs causes many $digest executions Jun 4, 2015
@zwirn1981
Copy link

Same $digest error in IE 11 when using "md-stretch-tabs" and if there are more tabs then space left, so it should paginate but instead breaks with $digest execution error shouldPaginate().

@stevenmiles
Copy link
Author

I don't see a "$digest execution error". Also, what ever is causing the $digest executions isn't triggering the digest loop detection inside angularjs. I don't see any exceptions at all. What I'm seeing is just a lot of $digests, and in FF and IE, continuous $digests.

@ThomasBurleson
Copy link
Contributor

@robertmesserle - sounds like a priority issue.

@stevenmiles
Copy link
Author

This fix doesn't seem to have an effect on what I reported. I've' updated the code pen, and double checked that the changes from commit are present. I'm seeing the same behaviour in all browsers as in the first comment.

http://codepen.io/stevenmiles/pen/LVWgJy

Animated Gif of FF behavior.
24 digests after init then continuous digests after tab change.
tabdigests

In the codepen, I'm not setting md-stretch-tabs, so according to the docs this should default to auto. I've also changed this code pen so that the number of digests are shown on the page to save having to monitor console.

@rrajewski
Copy link

In IE 11 it seems that the updateInkBarStyles function is the culprit. Adding/Removing the classes (even if the end result is no change) is triggering the DOMSubtreeModified event and causing the infinite digest loop.

handler:
https://github.com/angular/material/blob/master/src/components/tabs/js/tabsController.js#L59

updateInkBarStyles:
https://github.com/angular/material/blob/master/src/components/tabs/js/tabsController.js#L535

@robertmesserle
Copy link
Contributor

Ah, this seemed to address one of the digest issues, but missed another. Pushing a fix right now for the remaining issue.

robertmesserle pushed a commit that referenced this issue Jun 10, 2015
@stevenmiles
Copy link
Author

Commit 781929d has fixed this issue for FF. FF now behaves the same as Chrome. IE behaviour has changed slightly in that it now behaves like FF before 781929d, which is an improvement in that it doesn't start repeatedly digesting immediately after application start.

Sorry for being rather persistent about this issue, it seemed as if this issue was closed prematurely and remained closed despite providing updated codepens that demonstrated the issue wasn't fixed after 5372710.

Thanks for the fix!

@robertmesserle
Copy link
Contributor

@stevenmiles No need to apologize, I appreciate the updates! I will try to look into this on IE today.

@robertmesserle
Copy link
Contributor

Also, I thought I had re-opened it before - leaving it closed was accidental. =p

@wenbolovesnz
Copy link

+1!
I have the same issue for IE11, shouldPaginate() get called constantly, I also noticed that shouldPaginate() get called even during the state transitions.

@robertmesserle robertmesserle added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Jun 29, 2015
robertmesserle pushed a commit that referenced this issue Jun 29, 2015
@hanthomas
Copy link

The fix is an improvement in IE11. However, there are still a lot of digests being fired per tab change - about 10 digests... seems high for what's actually happening. But the bigger problem is that swiping the tabs doesn't work in IE11 (though, the arrow buttons do). In Chrome, swiping the tabs has a rubberband effect, so I can't actually get to the other hidden tabs because it snaps right back to the first tab after momentarily moving to the right. Neither behavior works that way it's described in the MD specs. http://www.google.com/design/spec/components/tabs.html#tabs-types-of-tabs

@stevenmiles
Copy link
Author

@hanthomas there is a separate issue for digests during tab transitions #3299. That is probably the next major perf issue with tabs atm.

@robertmesserle Thanks for the fix. No more continuous digests!

@hanthomas
Copy link

@stevenmiles - understood. didn't realize there was another story to address tab performance issues. thanks for clarifying.
@robertmesserle - didn't mean to sound unappreciative. thanks this fix, and looking forward to the perf enhancements.

@XavCamp
Copy link

XavCamp commented Nov 24, 2015

@robertmesserle Hi,
I don't understand why you moved the listener for DOMSubtreeModified from elements.pagination in tabsController (which is the md-pagination-wrapper) to element in the link of the directive, which is md-tabs-template? These are completely different elements in the DOM.
Now the callback is called every time something changes in any tab content, which can happen a lot (like a hundred times per second in my app) so it freezes everything, even with the debounce.
Thanks for your help.

I'm on Chrome 46.0.2490.86 on OS X, with Angular Material v1.0.0-rc4.
I have this issue since the v0.10.1

@ThomasBurleson ThomasBurleson removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Jan 19, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants