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

Fix memory leak in `Element.removeData()` #1077

Merged
merged 1 commit into from Mar 24, 2019

Conversation

@jvilk
Copy link

@jvilk jvilk commented Feb 19, 2017

Prior to this change, Element.removeData() replaces an element's data with an empty object. This behavior results in a slow memory leak for applications that do not re-use IDs, as Raphael continues to add new keys and objects to eldata over time.

This memory leak turns out to be a problem for Piwik, which uses
kartographer.js (which uses raphael) to draw an SVG image of the United States
in its dashboard (see demo).

Every time the dashboard is redrawn, it spawns ~700 new Elements with data. I've submitted a pull request that appropriately cleans up this data when the user navigates away from the page using removeData, but Raphael leaves an empty object in the eldata map keyed on the element's old ID. Since kartographer/Piwik does not reuse IDs, eldata grows continually on each redraw until the application is out of memory.

Without this change, it is very easy for an application to slowly leak memory as Raphael continues to add new keys to `eldata`.

This memory leak turns out to be a problem for [Piwik](https://piwik.org/), which uses
kartographer.js (which uses raphael) to draw an SVG image of the United States
in its analytics dashboard ([see demo](https://demo.piwik.org/index.php?module=CoreHome&action=index&idSite=3&period=day&date=yesterday)).

Every time the dashboard is redrawn, it spawns ~700 new Elements with data. [I've submitted a pull request that appropriately cleans up this data when the user navigates away from the page using `removeData`](matomo-org/matomo#11350), but Raphael leaves an empty object in the `eldata` map keyed on the element's old ID. Since kartographer/Piwik does not reuse IDs, `eldata` will grow continually on each redraw until the application is out of memory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.