#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 nbruin):
> 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.
`<size_t><object>...` has two effects: It does de refcount dance (mostly
superfluous) and it inserts PyInt_AsSsize_t call (necessary). By
introducing the right header template you can get the second and avoid the
first. See the code at #13387.
> > - 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.
OK, perhaps a little cryptic indeed. What I meant was that for a monodict
the bucket entries are really of the form
{{{
( key_id, ref, value )
}}}
which one could store as
{{{
(<size_t>, <PyObject*>, <PyObject*>)
}}}
but we're storing it as (Lists accept nothing else)
{{{
(<PyObject*>, <PyObject*>, <PyObject*>)
}}}
which means that the first entry gets stored by a pointer to a system-
word-sized allocated entry (a PyInt to be precise) instead of stored
directly in the field that would be big enough anyway. That's wasting
memory locality and hence CPU cache space on the bit we use the most.
(weakref and the values obviously have to be pointers).
Perhaps JP has a point -- the main concern on this ticket is to get a
''correct'' `MonoDict`. Discussions about optimizing it should go on
#13387. The memory leaks desperately need solving. Why don't you move
`IdKeyDict` there? If you can get it to perform sufficiently well, we
could drop it in. Having the code a little more structured would be a
benefit from a maintenance point of view.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12313#comment:173>
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.