#11342: Make getattr faster on parents and elements
---------------------------+------------------------------------------------
Reporter: SimonKing | Owner: tbd
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.7.1
Component: performance | Keywords: getattr parent element
Work_issues: | Upstream: N/A
Reviewer: | Author: Simon King
Merged: | Dependencies: #9944
---------------------------+------------------------------------------------
Changes (by newvalueoldvalue):
* status: new => needs_review
* dependencies: => #9944
* author: => Simon King
Comment:
Here are my ideas to make attribute access faster.
'''1. Make raising an attribute error faster'''
It is very common that one tries to get an attribute, but it does not
exist - attribute error. Since it occurs so frequently, it is essential
that the error be raised as quickly as possible. Raising the error is
therefore the job of a Cython function `raise_attribute_error` in
`sage.structure.parent`. However, that function can be slightly improved.
1.a) The second argument to that function always is a string: The
attribute name. Thus, why not explicitly state it in the Cython code?
1.b) It internally operates with the type of the first argument. So, why
not explicitly state it in the Cython code?
1.c) The error message is formed from the name of the class and the name
of the attribute. There is one complication: If one has an extension type
then its name shall be prepended by its module name. There is a Cython
function `is_extension_type` for telling whether it is an extension type
or not. But the function is small, and a tiny bit of time can be saved by
doing the test inside `raise_attribute_error`.
1.d) How shall one create the error message? In unpatched Sage, it is done
by a formatted string that is formed by the class name (perhaps together
with the name of the module of the class) and the attribute name. However,
it turned out in some timings that I did that composing the error message
by addition of strings is faster than the formatted string.
1.e) If one has an extension type then it turns out to be faster to get
module name plus class name from the string representation of that type,
rather than from addition of strings.
Hence, the new `raise_attribute_error` looks like this:
{{{
cdef inline raise_attribute_error(self, str name):
cdef type cls = type(self)
cdef int dictoff
try:
dictoff = cls.__dictoffset__
except AttributeError:
raise AttributeError, "'"+cls.__name__+"' object has no attribute
'"+name+"'"
if dictoff:
raise AttributeError, "'"+cls.__name__+"' object has no attribute
'"+name+"'"
raise AttributeError, repr(cls)[6:-1] + " object has no attribute
'"+name+"'"
}}}
__Timings for `raise_attribute_error`__
I compare the timings of unpatched and patched sage-4.7.alpha5, stating
the patched version first. We have to distinguish extension types and
others:
{{{
sage: cython("""
....: from sage.structure.parent cimport raise_attribute_error
....: def test(self, name, long m):
....: cdef long i
....: for i from 0<=i<m:
....: try:
....: raise_attribute_error(self,name)
....: except AttributeError:
....: pass
....: """)
sage: P.<a,b,c> = PolynomialRing(QQ)
sage: P.__class__.__dictoffset__
0
sage: QQ.__class__.__dictoffset__
288
sage: %time test(P,'bla',10^7)
CPU times: user 19.99 s, sys: 0.00 s, total: 19.99 s
Wall time: 19.99 s
# unpatched:
# CPU times: user 21.28 s, sys: 0.00 s, total: 21.28 s
# Wall time: 21.28 s
sage: %time test(QQ,'bla',10^7)
CPU times: user 17.66 s, sys: 0.01 s, total: 17.67 s
Wall time: 17.67 s
# unpatched:
# CPU times: user 19.92 s, sys: 0.01 s, total: 19.93 s
# Wall time: 19.93 s
}}}
'''2. Raise attribute errors earlier'''
An attribute of a parent can be obtained from the category only if its
category is properly initialised. Otherwise an attribute error is raised.
In order to test whether it is properly initialised, the method
`_is_category_initialized` is called. However, that method merely tests
whether the cdef attribute `_category` of the parent is None.
Hence, first suggestion: Directly test whether `self._category` is None,
if `self` is a parent.
Up to now, an attribute of an element can be obtained from the category of
its parent, even if the category of the parent is ''not'' properly
initialised. I doubt that this is a good idea. In fact, there is only a
single doctest error if one requests the parent's category to be
initialised, and this error vanishes if #9944 is applied.
Hence, second suggestion: If self is an element, then try to get an
attribute from the category only if self._parent._category is not None.
These changes have a nice effect on the performance. Again, I state the
timings for the patched version first.
Recall that by #10467, if the `__getattr__` method of a parent (or
element) is asked for a private attribute (one that starts with double
underscore but does not end with an underscore), it immediately raises an
error, since by convention private attributes can not be inherited from
the category (or they wouldn't be private).
Hence, we have to consider the following cases:
1. non-existing private attributes
{{{
sage: a = 1
sage: timeit('try: QQ.__bla\nexcept: pass')
625 loops, best of 3: 13.8 µs per loop
# unpatched: 625 loops, best of 3: 15.3 µs per loop
sage: timeit('try: a.__bla\nexcept: pass')
625 loops, best of 3: 13.3 µs per loop
# unpatched: 625 loops, best of 3: 14 µs per loop
}}}
2. non-existing non-private attributes
{{{
sage: timeit('try: QQ.__bla_\nexcept: pass')
625 loops, best of 3: 17.2 µs per loop
# unpatched: 625 loops, best of 3: 23.4 µs per loop
sage: timeit('try: a.__bla_\nexcept: pass')
625 loops, best of 3: 21.6 µs per loop
# unpatched: 625 loops, best of 3: 28.2 µs per loop
sage: timeit('try: QQ.bla\nexcept: pass')
625 loops, best of 3: 16.7 µs per loop
# unpatched: 625 loops, best of 3: 22.9 µs per loop
sage: timeit('try: a.bla\nexcept: pass')
625 loops, best of 3: 20.8 µs per loop
# unpatched: 625 loops, best of 3: 27.2 µs per loop
}}}
3. existing attributes inherited from the category
{{{
sage: timeit('try: QQ.sum\nexcept: pass',number=10^5)
100000 loops, best of 3: 740 ns per loop
# unpatched: 625 loops, best of 3: 744 ns per loop
sage: timeit('try: a.cartesian_product\nexcept: pass',number=10^5)
100000 loops, best of 3: 1.67 µs per loop
# unpatched: 100000 loops, best of 3: 3.89 µs per loop
}}}
Long doctests pass for me - #9944 is required for one test in
`sage.category.primer`. So: Needs review!
Depends on #9944
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11342#comment:1>
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.
