#13394: Write a WeakValueDictionary with safer key removal
-------------------------------------+-------------------------------------
Reporter: nbruin | Owner: rlm
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-5.13
Component: memleak | Resolution:
Keywords: | Merged in:
Authors: Simon King | Reviewers:
Report Upstream: None of the above | Work issues:
- read trac for reasoning. | Commit:
Branch: | fab0ed4112b9f798e2690f4c885b57cd711ea698
u/SimonKing/ticket/13394 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by nbruin):
I have attached a !WeakValueDictionary based on your prototype, but
directly implemented on `dict`. It's a drop-in replacement, so you can use
it right away (and you can probably more easily test its performance). In
my preliminary checks it seems (much) faster than
`weakref.WeakValueDictionary` in all situations (not surprising since it's
cythonized) and I think also always a bit faster than your prototype (it
should be with one level of indirection less)
I have not made explicit modifications "safe" for dictionary iteration,
although in most cases they should be fairly safe: python's dict only
reshapes on ''adding'' entries, so enumerating entries shouldn't lead to
huge problems (except for non-determinism).
Notes I found when adapting your code:
- if we only use `pending_removals` for callbacks, there's no need to
check that list for retrieval etc. You can recognize these entries from
the dead weakref.
- in `_IterationContext.__exit__` you must check the guard level every
time. Key deletion could have any consequence, including creation of new
iterators.
- for now I have left `len` as the length of the dict itself. We could
make it `len(self)-len(self.pending_removals)`, but length of a
`WeakValueDictionary` doesn't say much anyway.
- there's a `PyObject_Hash` that's a little more efficient than calling
hash (you get a `long` immediately).
Interesting detail: when I put this file in place in by `sage-git`, your
branch worked! That indicates that the MRO error reported above is indeed
triggered by some changed memory management or dict order.
Debugging by seeing if dicts that have an active iterator get mutated
might be useful. But note that people can keep iterators around without
intention to consume them any more.
--
Ticket URL: <http://trac.sagemath.org/ticket/13394#comment:54>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
