@jvilk opened this Pull Request on February 17th 2017 Contributor

See commits for details. The Raphael upgrade fixes the most serious memory leak.

You can verify the problem(s) by following these steps.

Prerequisites: The map is visible on the dashboard (i.e., piwik has collected data recently)

Steps:

  1. Navigate to the dashboard.
  2. Take a heap snapshot in Chrome, and note the size.
  3. Click on the "Dashboard" link in the left menu to reload the dashboard without refreshing the page.
  4. Take another heap snapshot. Note that it has grown by ~10MB!

After about 150 dashboard navigations, Piwik will crash the tab in Chrome because it has filled the heap. (Chrome has a JavaScript heap size of 1.4GB)

In addition, every time you click on the "Dashboard" link in the left menu, Piwik appends a link element to the end of the body containing the CSS for the map. You can view this in the "Elements" tab of Chrome's development tools.

@mattab commented on February 18th 2017 Owner

Thank you @jvilk for the nice Pull request! We will review this for the next Piwik release 3.0.2

@jvilk commented on February 19th 2017 Contributor

Note that there is still a tiny small memory leak caused by Raphael's behavior. I've submitted a PR to Raphael that eliminates this tiny memory leak.

Regardless, this PR alone turns the large memory leak into a tiny memory leak, so it is worth considering / merging before Raphael considers/merges the other fix.

@sgiehl commented on March 27th 2017 Member

@jvilk thanks for that PR. Seems to fix the memory leak in this case.

I'm gonna merge this now.

While testing I maybe found another memory leak. When clicking the reload button for the usercountry widget the required memory still increases a lot, guess the widget is not destroyed before it's reloaded or something similar. Would you maybe be keen to have a look at that, too?

This Pull Request was closed on March 27th 2017
Powered by GitHub Issue Mirror