Work around memory leak in networkx #801
Conversation
| def __del__(self): | ||
| # Work around cyclic reference bugs in networkx.DiGraph | ||
| # See https://github.com/uber/pyro/issues/798 | ||
| self._graph.__dict__.clear() |
for the record i am opposed to such kludges and eagerly await this code's deletion |
|
You da boss, this fixed it.
…On Feb 21, 2018 6:29 PM, "Fritz Obermeyer" ***@***.***> wrote:
@fritzo <https://github.com/fritzo> singlehandedly
for the record i am opposed to such kludges and eagerly await this code's
deletion
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#801 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABVhL9qAKDGwcAFjynTOIHtbuHYvNsWtks5tXNEBgaJpZM4SOpO1>
.
|
| graph_type = kwargs.pop("graph_type", "flat") | ||
| assert graph_type in ("flat", "dense"), \ | ||
| "{} not a valid graph type".format(graph_type) | ||
| self.graph_type = graph_type | ||
| super(Trace, self).__init__(*args, **kwargs) | ||
|
|
||
| def __del__(self): |
There was a problem hiding this comment.
When is this override required? At least in the tests, it looks like the circular references are resolved with the other changes you made.
There was a problem hiding this comment.
nx.DiGraph has a circular reference and would not be gc'd when the Trace is collected. This override is needed to cut the circular references in the nx.DiGraph object so that it can be immediately gc'd. If you were to remove this line, tests would still pass but DiGraph objects would leak. I guess we could strengthen the test to check counts for both Trace and nx.DiGraph objects, but @karalets already confirmed that things are working correctly.
There was a problem hiding this comment.
I see. I suppose the gc will detect those cycles at some point, but this should immediately fix the issue. I'm still not sure why the Digraph object should have circular references though, will pick your brains on this later.
There was a problem hiding this comment.
The cycles arise because nx.DiGraph stores view objects and those view objects have references back to the graph, e.g. here. I've filed networkx/networkx#2885 with a suggested fix.
There was a problem hiding this comment.
The cycles arise because nx.DiGraph stores view objects and those view objects have references back to the graph
Kudos to discovering this, and filing the issue! :) Thanks for explaining.
Warning: NSFW
Fixes #798