#11342: Make getattr faster on parents and elements
------------------------------+---------------------------------------------
Reporter: SimonKing | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.7.2
Component: performance | Keywords: getattr parent element
Work_issues: | Upstream: N/A
Reviewer: Jeroen Demeyer | Author: Simon King
Merged: | Dependencies: #9944
------------------------------+---------------------------------------------
Old description:
> If an attribute of a parent or element can not be found by inspecting the
> method resolution order, `__getattr__` is invoked. It tries to obtain the
> attribute from the parent class or the element class of the category of
> the parent. If it can not be found, the attribute error is raised using a
> Cython function `sage.structure.parent.raise_attribute_error`.
>
> I see several ways to make both `__getattr__` and
> `sage.structure.parent.raise_attribute_error` a lot faster. Details will
> be given in the comments.
>
> Because of one doctest in `sage.categories.primer`, it is needed that the
> category of polynomial rings is properly initialised. Therefore:
>
> Depends on #9944
>
> Apply
>
> * [attachment:trac11342-attribute_error_message.rebased.patch]
> * [attachment:trac_11342_fix_pari_initialization.patch]
> * [attachment:trac_11342_fix_pari_ring.patch]
New description:
If an attribute of a parent or element can not be found by inspecting the
method resolution order, `__getattr__` is invoked. It tries to obtain the
attribute from the parent class or the element class of the category of
the parent. If it can not be found, the attribute error is raised using a
Cython function `sage.structure.parent.raise_attribute_error`.
I see several ways to make both `__getattr__` and
`sage.structure.parent.raise_attribute_error` a lot faster. Details will
be given in the comments.
Because of one doctest in `sage.categories.primer`, it is needed that the
category of polynomial rings is properly initialised. Therefore:
Depends on #9944
Apply
* [attachment:trac11342-attribute_error_message.rebased.patch]
* [attachment:trac_11342_fix_pari_initialization.patch]
* [attachment:trac_11342_fix_pari_ring.patch]
* [attachment:trac_11342_crypto_fixes.patch]
* [attachment:trac11342_test_parent_on_getattr.patch]
--
Comment(by SimonKing):
I have added a new patch. Since ''any'' element has a parent, I do
actually not test whether `self._parent is None`. Instead, I do `cdef
Parent P = self._parent or self.parent()`.
That works, because `self.parent()` is supposed to return something (even
in cases of improper elements as we have met them in Pari). Moreover, it
is fast, since the slow call `self.parent()` will not be used if
`self._parent` is not None.
Since the parent needs to be retrieved anyway, my new patch essentially
preserves the performance. So, the additional test is essentially for
free.
I suppose that my patch would suffice to fix the problem. However,
Volker's crypto patch improves the code quality and thus should be
included as well.
For me, the tests pass when adding Volker's and my patch. But, after all,
I did not suffer from the segfault. So, please see if it's fixed!
Apply trac11342-attribute_error_message.rebased.patch,
trac_11342_fix_pari_initialization.patch, trac_11342_fix_pari_ring.patch,
trac_11342_crypto_fixes.patch, trac11342_test_parent_on_getattr.patch
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11342#comment:52>
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.
