#12313: Fix yet another memory leak caused by caching of coercion data
--------------------------------------------------+-------------------------
Reporter: SimonKing | Owner:
Type: defect | Status:
needs_review
Priority: major | Milestone: sage-5.3
Component: memleak | Resolution:
Keywords: coercion weak dictionary | Work issues:
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 SimonKing):
Replying to [comment:154 nbruin]:
> Sorry, the edited comment
http://trac.sagemath.org/sage_trac/ticket/12313#comment:148 probably fell
out of scope for you. In short: do away with the `_refcache` and put the
weakref directly in the bucket. That saves storage and a superfluous
lookup. That way `MonoDict` and `TripleDict` should still be a bit faster
than normal `dicts` (because key comparison is trivial).
I think that would not work, for two reasons:
1. You seem to assume that all keys of `MonoDict` allow weak references.
Is that granted? I am not sure. Perhaps this is a non-issue.
2. In a very early stage of my work, I ''did'' put the weak references
into the buckets. However, my impression was that that would create a
massive slow-down, as I explain now.
With the current patch, the buckets are of the form
`[h1,v1,h2,v2,h3,v3,...]`, where h1,h2,h3 are the memory addresses of the
keys, and v1,v2,v3 are the values. Now assume that we would put the (weak)
references r1,r2,r3 to the keys into the bucket:
`[r1,v1,r2,v2,r3,v3,...]`.
The weak references should have a callback function removing entries from
the dictionary. Hence, they are not just `weakref.ref`, but
`weakref.KeyedRef`.
Let K be some weak-reffable key and f be some callback function. While
`weakref.ref(K) is weakref.ref(K)`, we unfortunately have
`weakref.KeyedRef(K,f,id(K)) is not weakref.KeyedRef(K,f,id(K))`.
We want to compare K by identity. Hence, either do `if r1() is K` or `if
r1.key == <size_t><void *>K and r1() is not None`. Both tests are slow
(r1.key is a Python attribute lookup). Even worse: They are slow,
regardless whether K is in the dictionary or not.
With the new code, we would first test `h1==<size_t><void *>K` and then
`self._refcache[h1]() is not None`. The second test is slow, but
apparently we need it, or some doctests will fail. But the point is: If K
is not in the dictionary then ''except in very rare cases'' the first test
will recognise it. And the first test is indeed fast. Hence, non-existing
keys are fast to test, existing keys are probably not worse to test than
with a usual `WeakKeyDictionary`. Will test it soon...
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12313#comment:157>
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.
