Support cached_methods on extension types

[simon-king-jena]

In old versions of Cython, one would have seen an error like "arbitrary decorators are not supported" when attempting to use @cached_method on the methods of a cdef class. That has now been improved. However, cached methods would still not work, because an attribute __module__ is requested that does not exist.

The aim is to work around that limitation, so that cached methods and functions genuinely work in Cython.

Depends on #12215

CC: @hivert @robertwb

Component: misc

Author: Simon King

Reviewer: Travis Scrimshaw

Merged: sage-5.9.beta4

Issue created by migration from https://trac.sagemath.org/ticket/12951

[simon-king-jena]

comment:1

As it turns out, it is still impossible to use arbitrary decorators on c(p)def functions or methods. However, the attached patch makes it possible to use the decorator on def-methods of extension classes. That used to fail, because of the attribute error mentioned in the ticket description. It is of course still required that the instances of the extension class either allow to override an attribute of the class by an attribute of the instance, or that the instance has a public attribute __cached_methods.

For the latter case, we now have (from the doc tests):

    sage: cython_code = [
    ... "from sage.misc.cachefunc import cached_method",
    ... "cdef class MyClass:",
    ... "    cdef public dict __cached_methods",
    ... "    @cached_method",
    ... "    def f(self, a,b):",
    ... "        return a*b"]
    sage: cython(os.linesep.join(cython_code))
    sage: P = MyClass()
    sage: P.f(2,3)
    6
    sage: P.f(2,3) is P.f(2,3)
    True

Note that by #11115, sage.structure.parent.Parent has the __cached_methods attribute. Hence, any cdef subclass of Parent will do!

Providing attribute access is a bit more tricky, since it is needed that an attribute inherited by the instance from its class can be overridden on the instance. That is why providing a __getattr__ would not be enough in the following example:

    sage: cython_code = [
    ... "from sage.misc.cachefunc import cached_method",
    ... "cdef class MyOtherClass:",
    ... "    cdef dict D",
    ... "    def __init__(self):",
    ... "        self.D = {}",
    ... "    def __setattr__(self, n,v):",
    ... "        self.D[n] = v",
    ... "    def __getattribute__(self, n):",
    ... "        try:",
    ... "            return self.D[n]",
    ... "        except KeyError:",
    ... "            pass",
    ... "        return getattr(type(self),n).__get__(self)",
    ... "    @cached_method",
    ... "    def f(self, a,b):",
    ... "        return a+b"]
    sage: cython(os.linesep.join(cython_code))
    sage: Q = MyOtherClass()
    sage: Q.f(2,3)
    5
    sage: Q.f(2,3) is Q.f(2,3)
    True

Supporting attribute access apparently is more complicated than the other approach, and probably not recommended. However, on cached methods, it is somehow faster than the easier method:

    sage: timeit("a = P.f(2,3)")   # random
    625 loops, best of 3: 1.3 µs per loop
    sage: timeit("a = Q.f(2,3)")   # random
    625 loops, best of 3: 931 ns per loop

Needs review!

[simon-king-jena]

Author: Simon King

[simon-king-jena]

comment:2

Any reviewer?

[simon-king-jena]

comment:3

There is a conflict with #12215. Both tickets need review, but I think that #12215 is more important than this ticket. Hence, I introduced a dependency, and changed my patch accordingly.

[simon-king-jena]

Dependencies: #12215

[simon-king-jena]

comment:4

Note to myself: Test that this patch still applies.

[simon-king-jena]

Patch relative to #12215

[simon-king-jena]

comment:5

Attachment: trac12951_cached_extension.patch.gz

I had to rebase the patch. Now it should work again. Still needing review!

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:7

Looks good to me. Thanks Simon.

[jdemeyer]

Merged: sage-5.9.beta4