Fix plotting memory leak #9307
Fix plotting memory leak #9307
Conversation
12dee0b to
1a9769a
Compare
|
@jreback I think this patch is ready for review. Please let me know if you have any questions or suggestions. |
I understand you're referring to this. If so, only |
|
@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. |
|
Understood. What I'm caring is current impl makes it more difficult to understand the logic... How about:
|
|
@sinhrks @TomAugspurger can you have a quick look? |
|
@qwhelan can you squash to a single commit? and pls add a release note referncing the bug? |
|
@TomAugspurger can you have a look? |
1 similar comment
|
@TomAugspurger can you have a look? |
|
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.
|
|
@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. |
1a9769a to
795d566
Compare
|
whats new is now available for 0.16.1 5ebf521 |
795d566 to
2336fb3
Compare
|
Closing due to better version available in #9814 |
This PR resolves #9003 (and explains matplotlib/matplotlib#3892). The root cause of the memory leak is a reference cycle between
MPLPlotobjects and theAxesSubplotobjects they create.Specifically, a
plotffunction object is stored inax._plot_datafor 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 ofself. This means that byplotfpulls inself.axandself.axes, which point to theAxesSubplotthatplotfis being attached to. We therefore have a reference cycle: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
AxesSubplotandMPLPlothave the same lifetime. Just not using_plot_dataprevents the leak but breaks functionality.The final option as I see it is to change
plotfto not depend onself. This works but involves a fair amount of modifications. I elected to make each of theplotfs top-level functions to make the lack ofself-dependency explicit. This also required making several other functionsclassmethodsand moving some data fromMPLPlotto theAxesSubplotobject.The key assumption being made by this change is that either
MPLPlotobjects are discarded immediately after use or we don't want any modifications to theMPLPlot(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 ifMPLPlotobjects 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.