Improve startuptime by postponing the creation of argspecs of cached functions/methods

[simon-king-jena]

Currently, if a cached method or function is created, an instance of sage.misc.function_mangling.ArgumentFixer is created as well. But this is only needed when the cached function/method is actually called.

With this patch, the ArgumentFixer is either created when it is needed to normalise arguments (i.e., when the function is called), or when a CachedMethodCaller is bound to an instance. The latter is because different CachedMethodCallers bound to different instances share the ArgumentFixer with the CachedMethod that is bound to the class of the instances. Hence, it is reasonable to avoid creating the same ArgumentFixer repeatedly.

Without the patch, more than 500 ArgumentFixers are created during startup of Sage. This is expensive, because one needs to determine the argspec of the cached functions/methods (this may even involve reading source code!!). With the patch, only 6 ArgumentFixers are created.

CC: @nbruin @vbraun @robertwb

Component: performance

Keywords: cached method, argspec, introspection, startup

Author: Simon King

Reviewer: Travis Scrimshaw

Merged: sage-5.12.beta3

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

[simon-king-jena]

comment:1

Attachment: trac15038-postpone_argspec_of_cached_functions.patch.gz

It seems to me that I observe a decrease of startup time of 4 or 5 percent with my patch, but I'd like to know whether the patchbots confirm this.

[simon-king-jena]

comment:2

Startup time is not everything, I think we should also test that there is no big regression in calling a cached method.

With the patch:

sage: @cached_function
....: def f(a,b=1):
....:     return 1
....: 
sage: class C:
....:     @cached_method
....:     def a(self):
....:         return 1
....:     @cached_method
....:     def b(self, a,b=1,*args,**kwds):
....:         return 2
....:     
sage: c = C()
sage: %timeit f(1)
1000000 loops, best of 3: 916 ns per loop
sage: %timeit f(1,2)
1000000 loops, best of 3: 984 ns per loop
sage: %timeit f(a=1,b=2)
100000 loops, best of 3: 2.56 us per loop
sage: %timeit c.b(1,x=4)
100000 loops, best of 3: 2.8 us per loop
sage: %timeit c.b(1,4)
1000000 loops, best of 3: 886 ns per loop

Without the patch:

sage: c = C()
sage: %timeit f(1)
1000000 loops, best of 3: 922 ns per loop
sage: %timeit f(1,2)
1000000 loops, best of 3: 975 ns per loop
sage: %timeit f(a=1,b=2)
100000 loops, best of 3: 2.42 us per loop
sage: %timeit c.b(1,x=4)
100000 loops, best of 3: 2.86 us per loop
sage: %timeit c.b(1,4)
1000000 loops, best of 3: 991 ns per loop

I think this is fine. So, if the startup time would really improve by a couple percent, it would be a good progress.

[simon-king-jena]

comment:3

Unfortunately, the patchbot does not confirm what I saw when I've run sage -startuptime a couple of times with and without the patch. Anyway, I think the approach of this patch is still valid.

[tscrim]

comment:4

Hey Simon,

Here are my timings with patch (same setup):

sage: %timeit f(1)
100000 loops, best of 3: 1.37 us per loop
sage: %timeit f(1,2)
100000 loops, best of 3: 1.5 us per loop
sage: %timeit f(a=1,b=2)
100000 loops, best of 3: 3.73 us per loop
sage: %timeit c.b(1,x=4)
100000 loops, best of 3: 4 us per loop
sage: %timeit c.b(1,4)
100000 loops, best of 3: 1.67 us per loop

and without:

sage: %timeit f(1)
100000 loops, best of 3: 1.44 us per loop
sage: %timeit f(1,2)
100000 loops, best of 3: 1.55 us per loop
sage: %timeit f(a=1,b=2)
100000 loops, best of 3: 3.79 us per loop
sage: %timeit c.b(1,x=4)
100000 loops, best of 3: 4.02 us per loop
sage: %timeit c.b(1,4)
100000 loops, best of 3: 1.64 us per loop

So I don't see any (statically) significant speed regression, nor do I expect a couple of simple C-level if statements to slow anything down. I also get a modest startup-time improvement of about 5%, so I'm happy with this patch.

Best,

Travis

[tscrim]

Reviewer: Travis Scrimshaw

[jdemeyer]

Merged: sage-5.12.beta3