On Sunday, April 6, 2014 12:23:56 PM UTC-7, Nathann Cohen wrote:
>
> > As you can see for n=6, all the 318
> sage.graphs.linearextensions.LinearExtensions instances are still in
> memory. I suspect it's something like this:
> > - The "linear_extension" method is cached. This means that the poset
> will keep the linear_extension alive
> > - The unique_parent of linear_extension property means that its
> construction arguments (which includes the poset!) are used as a strong key
> in a weak_value_dictionary. This means that as long as the linear_extension
> is alive, the key will remain and hence the category of linear_extension
> will keep the poset alive.
> >
> > Voila, a classic memory leak. "cached_methods" and "unique_parent"
> combined are basically designed to invite memory leaks like this. Don't use
> them together unless you're prepared to think very hard about basically all
> the objects that might have a glancing interaction with the objects you're
> implementing.
>
> Are you serious ? I'm ready to bet that a LOT of classes use those two
> things at the same time. But there is a way to identify them automatically,
> isn't it ?
>
If you mean by "automatic": write a script to create a lot of them and
throw them away and see if it leaks then, yes. Otherwise: this really is a
non-local program logic problem, so detecting it is hard.
Yep. The problem is: UniqueRepresentation implements necessarily a GLOBAL
cache and hence will hold strong references to the creation parameters.
It's very unfortunate and I think if we had thought about the consequences
of that a little more, we wouldn't have gone with this design choice at
all. It's just too hard to work with. But now we're stuck with it.
UniqueRepresentation does try to alleviate the problems a little bit by
using a WeakValueDictionary for its cache, but as you see: Construction
parameter passed to a UniqueRepresentation constructor should NEVER hold a
strong reference to the resulting object, because that creates a memory
leak. Ideally, construction parameters should just not hold references at
all, but that's unfortunately infeasible.
Caching a routine that's simply a wrapper around a UniqueRepresentation
constructor is nearly useless anyway: UniqueRepresentation is supposed to
be fast and cached anyway! (it does have to do some argument processing,
though, so it is a little slower that a straight cached routine). However,
I can imagine that similar scenarios occur in less obvious ways and yes,
they all leak.
We can clean things up a little bit in this example by clearing the cache
ourselves, i.e., run
for p in P:
dummy=p.linear_extensions()
p.linear_extensions.clear_cache()
but you'll find that you need to do multiple gc.collect() afterwards to get
rid of all the crud: the garbage is so complicated that the collector can't
clean it up in one go (the objects that get released from being
WeakValueDictionary keys due to weakref callbacks are still part of
reference cycles, and it takes a fresh gc run to recognize them as such)
--
You received this message because you are subscribed to the Google Groups
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to sage-devel+unsubscr...@googlegroups.com.
To post to this group, send email to sage-devel@googlegroups.com.
Visit this group at http://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.
