Fix plotting memory leak

[qwhelan]

This PR resolves #9003 (and explains matplotlib/matplotlib#3892). The root cause of the memory leak is a reference cycle between MPLPlot objects and the AxesSubplot objects they create.

Specifically, a plotf function object is stored in ax._plot_data for the purposes of potentially redrawing if the data needs to be resampled. This would be fine if this were a top-level function; however, these are all nested functions that make use of self. This means that by plotf pulls in self.ax and self.axes, which point to the AxesSubplot that plotf is being attached to. We therefore have a reference cycle:

AxesSubplot -> AxesSubplot._plot_data -> plotf -> self -> self.ax -> AxesSubplot

In order to make the objects collectable, we need to either explicitly break a link or replace it with a weakref. Weakrefs don't work as AxesSubplot and MPLPlot have the same lifetime. Just not using _plot_data prevents the leak but breaks functionality.

The final option as I see it is to change plotf to not depend on self. This works but involves a fair amount of modifications. I elected to make each of the plotfs top-level functions to make the lack of self-dependency explicit. This also required making several other functions classmethods and moving some data from MPLPlot to the AxesSubplot object.

The key assumption being made by this change is that either MPLPlot objects are discarded immediately after use or we don't want any modifications to the MPLPlot (e.g., adding errors post-plotting) to be reflected if redrawing. I believe both cases are true but this patch has the potential for behavioral changes if MPLPlot objects are regularly being retained and modified.

I've also added a memory-leak test to prevent a regression; the test fails as expected if applied without the other commits in this patch.

[qwhelan]

@jreback I think this patch is ready for review. Please let me know if you have any questions or suggestions.

[jorisvandenbossche]

cc @TomAugspurger @sinhrks

[sinhrks]

Specifically, a plotf function object is stored in ax._plot_data

I understand you're referring to this. If so, only line and area are affected and no need to change others?

[qwhelan]

@sinhrks Yep, it's a subtle bug so I thought it'd be better to convert everything to a safer approach to prevent a reoccurrence.

[sinhrks]

Understood. What I'm caring is current impl makes it more difficult to understand the logic...

How about:

• Change closure to normal function (not return a function, but uses a function defined in the module directly)
• Make mapping from class to each plot function and unify separated _get_plot_function.

class LinePlot(MPLPlot):
    plotf = _lineplot_plotf
# or 
_plotf_mapping = {LinePlot: _lineplot_plotf, ...}

[jreback]

@sinhrks @TomAugspurger can you have a quick look?

[jreback]

@qwhelan can you squash to a single commit? and pls add a release note referncing the bug?

[jreback]

@TomAugspurger can you have a look?

[jreback]

@TomAugspurger can you have a look?

[TomAugspurger]

I've looked at this a bit. Haven't had a chance to fully digest it since I've never had to deal with this kind of problem before. I'll look more and let you know. It may be something to push for another release.

On Mar 10, 2015, at 18:20, jreback [email protected] wrote:

@TomAugspurger can you have a look?

—
Reply to this email directly or view it on GitHub.

[qwhelan]

@jreback Hey, sorry about the delay - made the mistake of letting Inbox decide what emails I see. I'll squash it now. Let me know what version I should put it under for the release notes.

[qwhelan]

@jreback Given we're at a rc1, I'm going to wait on adding to the docs until 0.16.1 is in development. As @sinhrks pointed out, it'd be possible to trim this down to just LinePlot and AreaPlot if that makes including this in a point-release more palatable.

[jreback]

whats new is now available for 0.16.1 5ebf521

[qwhelan]

Closing due to better version available in #9814