Speed-up "O" calls

[61337411-43fc-4a9c-b8d5-4060aede66d0]

BPO	427190
Nosy	@loewis
Files	meth_o.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = 'https://github.com/loewis' closed_at = created_at = labels = ['interpreter-core'] title = 'Speed-up "O" calls' updated_at = user = 'https://github.com/loewis' ``` bugs.python.org fields: ```python activity = actor = 'loewis' assignee = 'loewis' closed = True closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'loewis' dependencies = [] files = ['3335'] hgrepos = [] issue_num = 427190 keywords = ['patch'] message_count = 10.0 messages = ['36650', '36651', '36652', '36653', '36654', '36655', '36656', '36657', '36658', '36659'] nosy_count = 2.0 nosy_names = ['loewis', 'jhylton'] pr_nums = [] priority = 'high' resolution = 'accepted' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue427190' versions = [] ```

[61337411-43fc-4a9c-b8d5-4060aede66d0]

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.

[03bde425-37ce-4291-88bd-d6cecc46a30e]

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?

[61337411-43fc-4a9c-b8d5-4060aede66d0]

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.

[03bde425-37ce-4291-88bd-d6cecc46a30e]

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?

[61337411-43fc-4a9c-b8d5-4060aede66d0]

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.

[61337411-43fc-4a9c-b8d5-4060aede66d0]

Logged In: YES user_id=21627

Uploaded new version which invokes string_join correctly from _PyString_Join.

[03bde425-37ce-4291-88bd-d6cecc46a30e]

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.

[03bde425-37ce-4291-88bd-d6cecc46a30e]

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.

[03bde425-37ce-4291-88bd-d6cecc46a30e]

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.

[61337411-43fc-4a9c-b8d5-4060aede66d0]

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.