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.

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

JavaScript Memory Leak: widgetContent $destroy handlers #12059

Open
jvilk opened this issue Sep 14, 2017 · 0 comments
Open

JavaScript Memory Leak: widgetContent $destroy handlers #12059

jvilk opened this issue Sep 14, 2017 · 0 comments
Labels
c: Performance For when we could improve the performance / speed of Matomo.

Comments

@jvilk
Copy link

jvilk commented Sep 14, 2017

This is a somewhat complex memory leak made even more complex by my only vague familiarity with AngularJS. I will do my best to explain what is going on!

Steps to Reproduce

  • Visit the Dashboard of a Piwik installation.
  • Open up the devtools console, and inspect the length of the following property:
    • window.$widgetContent[0].jQuery[long number].$scope.$$listeners.$destroy
    • Note: jQuery randomly generates the long number.
    • Note 2: You have two instances of jQuery on the page, which both use different numbers in their properties. Typically, the larger of the two has the $scope property.
  • Click on the "Dashboard" link in the menu to refresh the dashboard.
  • Re-inspect the property, noting how it has doubled in length.

Cause

compileAngularComponents is to blame; specifically, this line:

https://github.com/piwik/piwik/blob/9c7d7ff1c842132bc8f3f0ff8c296eb7d3959c30/plugins/Morpheus/javascripts/piwikHelper.js#L123

The call to $compile causes AngularJS to register the $destroy handler on the element's scope. However, the scope is never destroyed -- it is re-used across dashboard visits.

Solution

Is it possible to destroy and re-create the scope when you redraw the dashboard? If not, then perhaps you can remove all of these listeners as a hackfix when the dashboard is redrawn?

Alternatively, are you supposed to re-compile these widgets when you revisit the dashboard? I honestly don't know. :-)

Impact

To measure the impact of this leak on Piwik's heap size, I hackfixed the leak by simply removing the line in Angular JS that adds the listener. Here is the result:

piwik_angular_leak

@sgiehl sgiehl added the c: Performance For when we could improve the performance / speed of Matomo. label Sep 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Performance For when we could improve the performance / speed of Matomo.
Projects
None yet
Development

No branches or pull requests

3 participants