#12313: Fix yet another memory leak caused by caching of coercion data
--------------------------------------------------+-------------------------
Reporter: SimonKing | Owner:
Type: defect | Status: needs_work
Priority: major | Milestone: sage-5.3
Component: memleak | Resolution:
Keywords: coercion weak dictionary | Work issues: Rebase, and
take into account Nils' comments
Report Upstream: N/A | Reviewers: Simon King,
Jean-Pierre Flori, John Perry
Authors: Simon King, Jean-Pierre Flori | Merged in:
Dependencies: #11521, #11599, #12969, #12215 | Stopgaps:
--------------------------------------------------+-------------------------
Comment (by nbruin):
Replying to [comment:183 SimonKing]:
> Why? References to `TripleDictEraser` come from the `TripleDict` it
points to, and from the weak references that are stored in said
`TripleDict`. Would this be more than having a reference cycle that only
involves `KeyedRef`, `TripleDict` and `TripleDictEraser`? None of them has
a `__del__` method, isn't it?
Hm ... For MonoDictEraser you used a weakref. That's mainly what led me to
believe it's better. I don't know what's more costly. Weakrefs make
individual deletions more expensive because of the callbacks. Cyclic
garbage buildup adds cost in that the GC is triggered more frequently. It
depends a bit on the connectivity of the graph if it's worthwhile
introducing enough weakrefs to break cycles.
The following check suggests that weakrefs can sometimes be cheaper:
{{{
sage: class refstore(object):
....: def __init__(self,ref):
....: self.ref=ref
....:
sage: class cyclic(object):
....: def __init__(self):
....: self.me=refstore(self)
....:
sage: N=0
sage: def callback(c):
....: global N
....: N+=1
....:
sage: import weakref
sage:
sage: class weakcyc(object):
....: def __init__(self):
....: self.me=refstore(weakref.ref(self,callback))
....:
sage: #import gc
sage: #gc.set_debug(gc.DEBUG_STATS)
sage: %time for i in range(80000): c = cyclic()
CPU times: user 0.33 s, sys: 0.01 s, total: 0.35 s
Wall time: 0.36 s
sage: %time for i in range(80000): c = weakcyc()
CPU times: user 0.20 s, sys: 0.00 s, total: 0.20 s
Wall time: 0.21 s
sage: %time for i in range(80000): c = cyclic()
CPU times: user 0.39 s, sys: 0.00 s, total: 0.39 s
Wall time: 0.40 s
sage: %time for i in range(80000): c = weakcyc()
CPU times: user 0.20 s, sys: 0.00 s, total: 0.20 s
Wall time: 0.21 s
sage: %time for i in range(80000): c = cyclic()
CPU times: user 0.33 s, sys: 0.00 s, total: 0.33 s
Wall time: 0.35 s
sage: %time for i in range(80000): c = weakcyc()
CPU times: user 0.20 s, sys: 0.00 s, total: 0.20 s
Wall time: 0.21 s
sage: N
239999
}}}
(it's fun to turn on gc stats and see that the weakrefs trigger no GC!)
> Hence, if you have a working iterator at #13387 then we should use that.
Note that [attachment:idkey_dict] has a different iterator, thanks to
Cython now supporting "yield". Do you think that iterator is safe?
Apart from the fact that it's returning garbage keys at the moment:
absolutely! See #13387. Or otherwise just do a nested while loop, storing
the position using bucket number and index in bucket. That's easier logic
than the recursive approach. But of course validate the weakrefs.
> In the next few days, my resources for working will be a bit limited,
but I'll try to get my patches from #715 and here in a better state,
according to your remarks.
Have fun!
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12313#comment:188>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
