perf(md-tooltip) use MutationObserver instead of watcher if available #7822
Conversation
| } | ||
|
|
||
| function configureWatchers () | ||
| { |
There was a problem hiding this comment.
Braces should be in the same line.
|
I believe I don't have to call scope.visibleWatcher() in onDestroy, but somebody correct me if I'm wrong. |
clshortfuse
force-pushed
the
md-tooltip-no-watcher
branch
from
2764ac8 to
1e0d5f6
Compare
use watcher on mdVisible only if being used braces on same line manually fire change if not watcher is present
clshortfuse
force-pushed
the
md-tooltip-no-watcher
branch
from
bde836c to
2bae1ad
Compare
| if (isVisible) showTooltip(); | ||
| else hideTooltip(); | ||
| }); | ||
| if (element[0] && 'MutationObserver' in $window) { |
There was a problem hiding this comment.
Why use MutationObserver instead of $attrs.$observe( ) ?
There was a problem hiding this comment.
While $observe is technically faster than $watch since it's a string comparison vs an object/expression evaluation, both are checked synchronously every $digest.
MutationObserver is handled by the browser and signaled asynchronously.
|
Solution appears to be complex. What are the justifications and perf gains? |
ThomasBurleson
added
the
needs: team discussion
|
I'll write a codepen to show the gains of this and the md-menu change. I use this in a production environment. With the solution I built, virtual repeat is not possible, and I have a table with 100s of rows and an md-tooltip on icons and an md-menu item on each row. Essentially, a 500 row table will have 1/4x the amount of watchers, going from 2000 down to 500 with no features lost. Watchers are processed every digest or apply and 2000 watchers will bring even an i7 to its knees. The important thing is to make sure that no features are lost, which I think the code is fine in this regard. |
|
A short explanation of the code is as follows: md-direction is a raw value tied to an attribute in the md-tooltip element. Since this variable is raw text, we can use MutationObserver to get a "push notification" from the browser when it changes instead of looping every digest to check if the attribute changed (what Angular does) md-visible is a reference value that needs to be checked by angular. Because tying md-visible to a value is optional in md-tooltip, the code will only use a watcher if the attribute is being used. If the attribute is not set at runtime, a MutationObserver is used to receive a "push notification" if the user decides to start using the attribute, which then creates the watcher. If MutationObserver is not available, the code will fallback to Angular's regular checks every digest loop. |
|
Okay, here are the code pens: Here's 1.0.7 which is the standard way of using two watchers per tooltip: http://codepen.io/shortfuse/pen/LNOdvW In my table of 1000 bind-once rows, it increments the watcher count from 13 to 4000 (2 per row). On my fork which doesn't use watchers, the watcher count stays the same at 13. I set it to a high number to illustrate the performance effects. Please disregard any CSS changes in my fork. |
|
@jelbourn - please review. Seems like a reasonable perf improvement for tooltips in large tables. |
| @@ -92,19 +92,42 @@ function MdTooltipDirective($timeout, $window, $$rAF, $document, $mdUtil, $mdThe | |||
| content.css('transform-origin', origin); | |||
| } | |||
|
|
|||
| function onVisibleChanged (isVisible) { | |||
| if (isVisible) showTooltip(); | |||
There was a problem hiding this comment.
Missing braces
(here and elsewhere)
… use watcher on mdVisible only if being used braces on same line manually fire change if not watcher is present Closes angular#7822
Fixes angular#10162 - When the position was changed dynamically the resulting tooltip's panel would retain the position classes on it's panelEl. Now when creating and recreating the tooltip with a different position class due to a position change, the panel will grab that tracked panel and update its configuration object so that the new position class can be used. Fixes angular#10157 - There was a `mdDirection` watcher that was deleted due to a rebase/timing issue when moving to the `mdPanel` API. Reference to previous PR: angular#7822.
use watcher on mdVisible only if being used
fixes #4033
improved version of #4441