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.

Sign up for GitHub

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

Jump to bottom

Fix multiple memory leaks in UserCountryMap #11350

Merged
merged 3 commits into from Mar 27, 2017
Merged

Fix multiple memory leaks in UserCountryMap #11350

merged 3 commits into from Mar 27, 2017

Commits on Feb 22, 2017

  1. Upgrade to Raphael 2.2.7 to fix serious memory leak in UserCountryMap 

    Kartograph stores data on SVG paths using Raphael:

https://github.com/kartograph/kartograph.js/blob/f70bd295fa8b763d771f923a91b09564040eada3/src/core/maplayerpath.coffee#L30

Raphael stores this data in a *global* map, keyed on the path's ID:

https://github.com/DmitryBaranovskiy/raphael/blob/fe8e591e1c86b5aeb4c252b33c08e647434504c5/raphael.js#L3168

https://github.com/DmitryBaranovskiy/raphael/blob/fe8e591e1c86b5aeb4c252b33c08e647434504c5/raphael.js#L306

UserCountryMap does the right thing and calls clear() on the Kartograph map to reclaim memory...:

https://github.com/piwik/piwik/blob/6fab62e2229f9efadbcf43ff541b46f007fc6323/plugins/UserCountryMap/javascripts/visitor-map.js#L1332

...which eventually calls `remove` on each of these SVG paths:

https://github.com/kartograph/kartograph.js/blob/f70bd295fa8b763d771f923a91b09564040eada3/src/core/maplayerpath.coffee#L70

However, previous versions of Raphael (including 2.1.0) fail to call `removeData` to clear out the data associated with the SVG path in the global map,
leading to a serious memory leak (heap grows by ~10MB each time the map refreshes):

https://github.com/DmitryBaranovskiy/raphael/blob/7ba1a8258be64fdb517bc78fde44ff0e6188ca05/raphael.js#L4475

Later versions, including the latest 2.2.1, properly call `removeData` when `remove` is called:

https://github.com/DmitryBaranovskiy/raphael/blob/master/raphael.js#L6842
    John Vilk committed Feb 22, 2017
    Copy the full SHA
    6afe92a View commit details
    Browse the repository at this point in the history

  2. Properly remove window resize event listener for UserCountryMap 

    The previous version of the code inappropriately tried to remove the event listener;
the `function` object for `onResizeLazy` is different across maps.
    John Vilk committed Feb 22, 2017
    Copy the full SHA
    1733efe View commit details
    Browse the repository at this point in the history

  3. Prevent visitor-map from inserting a new CSS document into DOM every … 

    …refresh.
    John Vilk committed Feb 22, 2017
    Copy the full SHA
    3325609 View commit details
    Browse the repository at this point in the history