#14214: Cythoned homsets
----------------------------------------------+-----------------------------
Reporter: SimonKing | Owner: tbd
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-5.9
Component: performance | Resolution:
Keywords: Hom, cython, cached method | Work issues: Do not change
cmp too much
Report Upstream: N/A | Reviewers:
Authors: Simon King | Merged in:
Dependencies: #14159, #12951, #13184 | Stopgaps:
----------------------------------------------+-----------------------------
Comment (by nbruin):
Replying to [comment:28 SimonKing]:
> That's interesting. I can reproduce it, but it totally contradicts all
my previous experience with cached methods.
I think we got to the root of this: up to 0.18, cython would compile its
classes without "attribute lookup cache support". Some unfortunate
semantics in python mean that for instance attributes, the whole MRO needs
to be walked, so they form the worst case. I think that's why you were
finding `cached_method` faster sometimes: That's a class attribute lookup
so likely succeeds a little earlier. With a deep MRO that might be enough
to offset the overhead from calling that's involved in a `cached_method`.
In all likelihood from 0.19 onwards (or earlier if we patch cython in
sage), will enable "attribute lookup cache support" on the classes it
defines, in which case the result from a full MRO lookup will be cached
(also when it doesn't find anything), so after the first access, we don't
have to walk the MRO anymore. This speeds up all attribute access
considerably (sage doctesting seems to speed up something like 5-10% on
average) and it makes normal instance attribute access faster than a
`cached_method` call.
Given that, I think we SHOULD generally prefer instance attributes over
no-arg cached_method.
In fact, with the attribute lookup cache in place, the difference between
a property an a cached_method becomes even more pronounced:
{{{
sage: cython("""
....: include "../ext/stdsage.pxi"
....: include "../ext/python_bool.pxi"
....: from sage.misc.cachefunc import cached_method
....: from sage.structure.parent cimport Parent
....: cdef class T(Parent):
....: cdef public int D
....: @cached_method
....: def C(self):
....: return 10
....: property B:
....: def __get__(self):
....: return self.__cached_methods['c']
....: def __set__(self,value):
....: self.__cached_methods['c']=value
....:
....: """)
....:
sage: A=T()
sage: A.C()
10
sage: A.B=10
sage: timeit('A.C()',number=800000,repeat=30)
800000 loops, best of 30: 183 ns per loop
sage: timeit('A.B',number=800000,repeat=30)
800000 loops, best of 30: 58.1 ns per loop
sage: timeit('A.D',number=800000,repeat=30)
800000 loops, best of 30: 47 ns per loop
}}}
For reference, without attribute cache this is:
{{{
sage: timeit('A.C()',number=800000,repeat=30)
800000 loops, best of 30: 195 ns per loop
sage: timeit('A.B',number=800000,repeat=30)
800000 loops, best of 30: 68.5 ns per loop
sage: timeit('A.D',number=800000,repeat=30)
800000 loops, best of 30: 60.7 ns per loop
}}}
So in order of decreasing precedence, storage should be:
- instance attribute (cdef public or `__dict__` based if available)
- `__cached_methods` dict based data descriptor (really, that's just an
instance attribute dictionary, so we could probably do away with that if
we can have an instance `__dict__`. There's no inherent reason why cython
cdef class instances can't have a `__dict__`. I guess the cython people
are holding off on implementing it because they want it to play nice with
the cython level too (which we wouldn't particularly care about)
- cached_method.
The main advantage of cached_method is its spelling. So if we could make a
version of `@cached_method` (say `@computed_property`) that produces a
data descriptor (based on `self.__cached_methods`), where the `__get__`
can basically be what is now the `__call__` you'd get a different spelling
and better performance out of noarg cached "methods". If you make sure
that `__set__` is `NULL` you could even use `self.__dict__` to cache the
result and never get called on `__get__` again (you'd provide a nondata
descriptor, so you'd get shadowed by the instance attribute once
installed)
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/14214#comment:30>
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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
