-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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.
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
Tooltip Memory Leak #4499
Comments
Hey guys, any movement on this one? I have an app in production getting killed by this right now. |
Working on a PR now that should fix this by removing the logic of dynamically adding/removing the tooltip. Instead it'll sit in the DOM for the life of the host element. |
@andrewseguin I'm not certain that is the cause, as the leak can be observed without ever actually hovering over the buttons that have the tooltip applied to them. In the plunker, you can run your mouse up and down the left side of the list items, without ever hovering over the buttons on the right that trigger the tooltips to show, |
Can you confirm that removing HammerJS from the plunker helps resolve the issue? Looks to me that the CPU usage returns to normal if it's not imported. |
@andrewseguin I can confirm that removing hammer makes a significant, measurable difference. Removing it fixes the DOM node leaks, cpu usage, and the effectiveness of garbage collection. Nice find...where do we go from here? |
The tooltip component starts listening for mouseenter and mouseleave events on construction. When hammerjs is present there will be a reference between the tooltip and hammerjs because of this (see screenshots of the heap snapshots). Due to this reference the tooltip cannot be garbage collected after the destruction of the component and memory will be leaked. To let the tooltip be garbage collected on destruction either hammerjs should not be used or (preferably) the reference between the tooltip and hammerjs should be removed. Removing the reference is done by stop listening to the mouseenter and mouseleave events when the tooltip is destroyed (see my PR of May 26th). |
Just want to add that this affects md-slide-toggle as well. |
Component should clean up events it has registered to. Closes #4499
* fix(tooltip): remove native event listener on component destroy Component should clean up events it has registered to. Closes #4499 * Break at higher syntactic level * Undo accidental formatting
…lar#5144) * fix(tooltip): remove native event listener on component destroy Component should clean up events it has registered to. Closes angular#4499 * Break at higher syntactic level * Undo accidental formatting
I have the same issue (Angular and Material v6.x) and removing HummerJS from my project solve the memory leak in my case. However not I keep getting warning messages in the log: |
@mmalerba @andrewseguin Is it possible that the PR did not actually resolve this issue? |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Bug, feature request, or proposal:
Bug
What is the expected behavior?
For tooltip to not leak memory.
What is the current behavior?
Tooltip seems to leak memory.
What are the steps to reproduce?
Plunker with tooltip:
https://plnkr.co/edit/IFpa3W9EXGG1Mch7TBmm?p=preview
Plunker without tooltip
https://plnkr.co/edit/vFdyEmedUYGCavMl8eEN?p=preview
Which versions of Angular, Material, OS, browsers are affected?
Used plunker template provided in issue template, so latest.
Is there anything else we should know?
I performed a devtools timeline recording and tried to keep it apples-to-apples. I performed GC at the start and end of each recording.
Strangely, both recordings seem to indicate leaking DOM nodes, which aren't cleaned up after a GC. The version with the tooltip however, doesn't clean up after itself nearly as nicely as the version without it.
Timeline Without Tooltip
Timeline With Tooltip
The text was updated successfully, but these errors were encountered: