Export to GitHub

unladen-swallow - issue #88

Expand direct calling to more C functions


Posted on Nov 5, 2009 by Helpful Elephant

r890 added support for direct calls to fixed-arity functions with arity greater than METH_O. There are two things to do based on this patch:

  1. Tag as many functions as possible with METH_FIXED. r890 only added a few, and there are lots more C functions that can benefit from this.

  2. Add support for variadic C functions. This would allow direct calls to methods like dict.get() and unicode.replace().

Comment #1

Posted on Nov 5, 2009 by Helpful Elephant

This work was inspired by http://bugs.python.org/issue1107887, which includes an implementation of the varargs support above.

Comment #2

Posted on Nov 8, 2009 by Happy Elephant

I uploaded a patch for this issue at http://codereview.appspot.com/151055

Comment #3

Posted on Nov 9, 2009 by Helpful Bear

I'm taking the liberty of reviewing this patch (as I was working on one myself), and it looks good so far (as far as I can tell), but I just thought I'd mention that there are many misindentations all over.

It seems to be mostly because of using spaces instead of tabs (as PEP7 recommends for pre-existing, tab-indented code), but there are other inconsistencies as well (for example, on methodobject.c, line 42 should be indented to align with the column after the opening parenthesis, on the line above).

Comment #4

Posted on Nov 9, 2009 by Happy Elephant

Thanks for your comments. I already wondered about certain strange entries in the patch file. I'll fix the indentation asap.

I'm sorry to hear that you worked on a patch, too. Feel free to review in rietveld (I hope this works without an explicit invite). If you have any larger addition just email me, and we'll come up with a way to integrate them.

Comment #5

Posted on Nov 14, 2009 by Helpful Elephant

Direct calls for variadic calls were implemented in r894 by applying ebo's patch:

django: Min: 0.874269 -> 0.850007: 2.85% faster Avg: 0.876131 -> 0.854261: 2.56% faster Significant (t=28.631433, a=0.95) Stddev: 0.00526 -> 0.00554: 5.09% larger

slowpickle: Min: 0.596070 -> 0.546111: 9.15% faster Avg: 0.612642 -> 0.563118: 8.79% faster Significant (t=4.020278, a=0.95) Stddev: 0.08579 -> 0.08840: 2.95% larger

There are probably more functions that can be tagged with METH_ARG_RANGE (aka, the new METH_FIXED).

Comment #6

Posted on Dec 9, 2009 by Helpful Elephant

r904 tagged some more methods with METH_ARG_RANGE.

Highlight:

django: Min: 0.868683 -> 0.847636: 2.48% faster Avg: 0.874128 -> 0.854678: 2.28% faster Significant (t=23.333001, a=0.95) Stddev: 0.00617 -> 0.00561: 9.95% smaller

Other benchmarks were performance-neutral.

Comment #7

Posted on Jan 6, 2010 by Helpful Elephant

(No comment was entered for this change.)

Comment #8

Posted on Jan 11, 2010 by Helpful Elephant

Per issue105, direct calling should be expanded to str.startswith().

Comment #9

Posted on Feb 21, 2010 by Helpful Elephant

How to find good targets for METH_ARG_RANGE: Run benchmarks directly with -- profile, e.g.,

PYTHONPATH=lib/django python2.6 performance/bm_django.py -n 100

That will give cProfile output for the Django benchmark. Look at which C functions are being called a lot. Do those functions take a fixed or variable number of positional arguments? If so, add METH_ARG_RANGE and benchmark with perf.py.

Lather, rinse, repeat.

Comment #10

Posted on Feb 21, 2010 by Swift Camel

One good target would be cStringIO for the slow(un)?pickle benchmarks, I seem to remember those are still METH_VARARGS.

Comment #11

Posted on Feb 21, 2010 by Happy Monkey

A number of methods on file are candidates as well. We need a wrapper for argument checking/coercion though.

Comment #12

Posted on Feb 25, 2010 by Grumpy Ox

Issue 143 is tracking what happens if you break out the code from getargs.c into functions so as to get consistent coercion of objects.

Status: Accepted

Labels:
Type-Enhancement Priority-Medium Performance StarterProject PyCon