#12808: Optimize ClassCallMetaClass using Cython
--------------------------------------------------+-------------------------
Reporter: hivert | Owner: jason
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-5.0
Component: misc | Resolution:
Keywords: classcall UniqueRepresentation | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Florent Hivert | Merged in:
Dependencies: | Stopgaps:
--------------------------------------------------+-------------------------
Comment (by SimonKing):
Hi Florent,
Replying to [comment:37 hivert]:
> > Also, when `nested_pickle` calls `modify_for_nested_pickle`, it always
looks up `sys.modules[...]` - thus, why not store `sys.modules` in a `cdef
dict` variable?!
>
> Does this give a large speed-up ? I would guess that the dict search is
the
> bottleneck, but without serious profiling one cannot be sure.
Let's test:
{{{
sage: cython("""
....: import sys
....: cdef D = sys.modules
....: d = sys.modules
....: def test1(name):
....: cdef int i
....: for i from 0<=i<1000000:
....: a = D[name]
....: def test2(name):
....: cdef int i
....: for i from 0<=i<1000000:
....: a = d[name]
....: def test3(name):
....: cdef int i
....: for i from 0<=i<1000000:
....: a = sys.modules[name]
....: """)
sage: name = "sage"
sage: %time test1(name)
CPU times: user 0.02 s, sys: 0.00 s, total: 0.02 s
Wall time: 0.02 s
sage: %time test2(name)
CPU times: user 0.08 s, sys: 0.00 s, total: 0.08 s
Wall time: 0.08 s
sage: %time test3(name)
CPU times: user 0.11 s, sys: 0.00 s, total: 0.11 s
Wall time: 0.12 s
""")
}}}
Conclusion: Assigning `sys.modules` to a Python variable brings a speed-up
of 1/3, and assigning it to a Cython variable brings a speed-up of 5/6.
So, the speed-up obtained from Cython is indeed huge.
> Sure ! ClasscallType was for me a step before doing more in depth change
in
> the Metaclass infrastructure. I was quite scared to do those in depth
change
> in Sage/Python and feared a lot of hard to debug segfaults.
Fear thee not!
> I planned to keep the rest of the metaclass infrastructure
> for another patch but if you think you cdefing NestedClassMetaclass is
easily
> done, we can do both at once an thus the need of ClasscallType vanishes.
Yes, it is easily done - *if* one knows what to cimport. How did you find
out?
"Easy" means that one can really just change def into cdef. And in
addition to that, I tried to call some frequently used routine directly,
in order to avoid a calling overhead, and also I cpdefined that routine.
> > Variant 1: I produce a patch that does the changes in nested_class,
and includes most of the changes from your patch (but not
`ClasscallType`). Then I post it here, and we cross-review.
> >
> > Variant 2: I review your patch from here, and do my changes (which
would revert your changes in the bases of `ClasscallMetaclass`) on a
different ticket.
> >
> > What do you prefer?
>
> As you wish ! I most probably wont be working on that ticket anymore,
unless
> for addressing some reviewer remark or to help you if you need. The two
> variants are equal to me and you can consider yourself the owner of that
> ticker if you wish. We just need to have some clean review.
My preference is to do it on one ticket (the common topic being "cdefine
`ClasscallMetaclass`", which of course also involves cdefining
`NestedClassMetaclass`).
Concerning clean review: I would provide a patch that is to be applied
before your patch (dealing with nested classes) and another one to be
applied after your patch (refactoring the bases of classcall metaclass).
Hence, we can easily cross-review, which is considered to be "clean".
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12808#comment:38>
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.
