[Patches] [ python-Patches-427190 ] Speed-up "O" calls

noreply@sourceforge.net noreply@sourceforge.net
Thu, 16 Aug 2001 06:21:17 -0700


Patches item #427190, was opened at 2001-05-24 22:30
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=427190&group_id=5470

Category: core (C code)
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 7
Submitted By: Martin v. Löwis (loewis)
Assigned to: Martin v. Löwis (loewis)
>Summary: Speed-up "O" calls

Initial Comment:
This patch improves the performance of a few functions
which have an "O" signature (ord, len, and
list_append). On selected test cases, this patch gives
a speed-up of 40%. If accepted, the approach can be
extended to more signatures. E.g. "l" is already
provided in the patch, but currently not used.

----------------------------------------------------------------------

>Comment By: Martin v. Löwis (loewis)
Date: 2001-08-16 06:21

Message:
Logged In: YES 
user_id=21627

Committed as api.tex  1.140, NEWS 1.206, complexobject.c
2.38, descrobject.c 2.3, dictobject.c 2.109, fileobject.c
2.118, iterobject.c 1.7, listobject.c 2.99, methodobject.c
2.37, rangeobject.c 2.28, stringobject.c 2.123, typeobject.c
2.36, unicodeobject.c 2.108, bltinmodule.c 2.226, ceval.c
2.267, and sysmodule.c 2.91.

There were only slight changes to the patch: a few
additional METH_O usages, and get/setdlopenflags was
restored. None of the modules uses the new calling
convention, yet.

----------------------------------------------------------------------

Comment By: Jeremy Hylton (jhylton)
Date: 2001-08-13 15:47

Message:
Logged In: YES 
user_id=31392

The attached patch applies cleanly against current CVS and
implements the fast_cfunction() support for METH_O and
METH_NOARGS.

Does this patch still look okay to you, Martin?  If so, I
say we check it in.


----------------------------------------------------------------------

Comment By: Jeremy Hylton (jhylton)
Date: 2001-08-12 14:46

Message:
Logged In: YES 
user_id=31392

Must also update fast_cfunction() to handle METH_NOARGS and
METH_ARGS, as these can be done on the fast path.


----------------------------------------------------------------------

Comment By: Jeremy Hylton (jhylton)
Date: 2001-08-12 14:11

Message:
Logged In: YES 
user_id=31392

I've updated the patch to compile against the current source
tree.  I also revised the switch statement that dispatches
on function flags (METH_O, METH_VARARGS, ...) to avoid the
goto.  The big change for descr branch compatibility was to
define
the dispatch for C functions in methodobject.c as
PyCFunction_Call() so that it can be used in ceval.c and
methodobject.c.

I'd approve right now, but it looks like there is a lot of
unused code for calling functions in ceval.c, and I'd like
to clean that up first.

We also need to see if there are no opportunities to use
METHO_O and METH_NOARGS, which I didn't do.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2001-06-17 18:41

Message:
Logged In: YES 
user_id=21627

Uploaded new version which invokes string_join correctly 
from _PyString_Join.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2001-06-02 03:12

Message:
Logged In: YES 
user_id=21627

New version uploaded. This uses functions with only the 
self argument for METH_NOARGS, and introduces 
PyNoArgsFunction for them.

It also adds a section for api.tex documenting the METH_ 
flags, and an entry in NEWS mentioning the new METH_ flags.


----------------------------------------------------------------------

Comment By: Jeremy Hylton (jhylton)
Date: 2001-06-01 08:14

Message:
Logged In: YES 
user_id=31392

Just took a quick look -- looks good.  

One question: Why does METH_NOARGS call the method with two
arguments where the second is always NULL?  Wouldn't it be
clearer to have these functions take one argument?


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2001-06-01 07:34

Message:
Logged In: YES 
user_id=21627

I rewrote the patch to only support METH_NOARGS and METH_O,
and to not use bit masks for them.

I also changed calling conventions for all Object operations
and bltin and sys functions. In the course of these changes,
two functions got a changed meaning:
- file.writelines accepts only exactly one argument
- iter.next does not accept any arguments anymore

As you can see in the patch,there is still a lot of places
that continue to use OLDARGS (plus all the Modules functions
that have not been changed in this patch), so OLDARGS will
be needed for quite some time.

----------------------------------------------------------------------

Comment By: Jeremy Hylton (jhylton)
Date: 2001-05-29 13:59

Message:
Logged In: YES 
user_id=31392

I like METH_O, but I'm not sure about METH_L.  I'd rather
see the call handling in ceval be type-neutral.  It's easy
enough for the callee to cast from an object to an int (or
any other type).  There should be no effect on performance
and it reduces the amount of code in the core.

I think the implementation could be simplified a lot if it
defined METH_O -- or perhaps METH_NOARGS,  METH_ONEARG, and
maybe even METH_TWOARGS (but Tim has a pretty good argument
against that one).  I don't think there's any define METH_O
via METH_SPECIAL and reserve all of 0xFFF0 for flags on
METH_SPECIAL.  Instead, I'd just use the next N bits to
implement the next N flags.

The SPECIALSIZE and extra stack used in the implementation
seem like unneeded generality, too.  If the implementation
is only going to support 0 and 1 (and possibly 2) argument,
there's no need for anything more general.

Finally, I suggest appropriating fast_cfunction() for this
purpose, rather than calling the new function
do_call_special(), where "special" isn't a very specific
meaning.  If METH_NOARGS and METH_ONEARG are implemented,
there is basically no reason to use METH_OLDARGS.  So we can
get rid of it in the code base and stop attempting to
optimize it.

Do you want to have a go at a smaller patch that just did
METH_ONEARG and METH_NOARGS?


----------------------------------------------------------------------

You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=427190&group_id=5470