#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:162 nbruin]:
> OK! example implementation done. A couple of things I found:
>
> - Some of your doctests for `MonoDict` test `TripleDict`!
Oops.
> - your iterators can recurse quite deeply if there are lots of empty
buckets in
> a row. That can easily happen.
Yes? I thought that the number of buckets is quite small (so, no deep
recursion) and the whole thing is designed to distribute the items
uniformly over the buckets
> - Casting to `<object>` in Cython indeed gives you a kind of `<PyObject
*>`
> where Cython takes care of intelligent casting and refcounting for you.
That
> comes at some cost. Especially for `tmp` I think we can largely do away
with
> that for a little gain in speed. This might apply more widely.
I wrote some general code for what I now call `IdKeyDict`: A weak key
dictionary (that also accepts items that aren't weak-reffable) with keys
of prescribed length, keys being compared by identity. There, the tmp is
not used.
> - It's a shame we can't store our `h` values immediately in the list
but that
> we have to store them in an integer, which means we're storing a
pointer to
> that integer in the list.
I don't understand that comment.
> - What happens if we find a dead entry in setitem? How can we be sure
the
> pending callback for that doesn't happen on our new value?
My new code takes care of that. The methods are a lot more modularised.
There is one method that simply finds the correct bucket (according to the
hash) and another method that finds the position of an item on that
bucket. The second method will automatically delete a dead item from the
bucket. And of course, that method is both used in `__setitem__`,
`__getitem__` and `__contains__`.
> I think explicitly
> deleting the weakref should do the trick, but then we should ensure
that nobody
> else gets a chance to keep those weakrefs alive!
What would be the problem if the weakrefs survive? OK, at some point the
callback will start working, but would that be a problem? I mean, if the
callback tries to remove an item that has already been deleted, then the
callback would simply do nothing
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12313#comment:164>
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.
