homepage

Changes 27dc9a1c061e and 01b06ca45f64 converted the str (Unicode) methods to Argument Clinic, cool! But str methods taking more than one argument use positional-only arguments.
Currently, Argument Clinic doesn't use METH_FASTCALL for these methods, but METH_VARARGS.
There are two options:
* Allow passing arguments as keywoards: str.replace(old='a', new='b')
* Enhance Argument Clinic to use also METH_FASTCALL for functions using positional-only functions
The goal is to speedup method calls. Example with str.replace():
$ ./python-patch -m perf timeit '"a".replace("x", "y")' --duplicate=100 --compare-to ./python-ref
python-ref: ..................... 132 ns +- 1 ns
python-patch: ..................... 102 ns +- 2 ns
Median +- std dev: [python-ref] 132 ns +- 1 ns -> [python-patch] 102 ns +- 2 ns: 1.30x faster (-23%)

Background: see also the old issue #17170 closed as rejected: "string method lookup is too slow".

The issue #29263 "Implement LOAD_METHOD/CALL_METHOD for C functions" should also optimize C methods even more.

> * Allow passing arguments as keywoards: str.replace(old='a', new='b')
I think Guido explicitly stated that he doesn't like the idea to always allow keyword arguments for all methods. I.e. `str.find('aaa')` just reads better than `str.find(needle='aaa')`. Essentially, the idea is that for most of the builtins that accept one or two arguments, positional-only parameters are better.
> * Enhance Argument Clinic to use also METH_FASTCALL for functions using positional-only functions
So this is the option to go with.

What is python-patch?

> What is python-patch?
It's Python patched with attached unicode_allow_kw.patch: allow to pass parameters as keywords for Unicode methods.

Parsing positional arguments for METH_KEYWORDS and METH_FASTCALL can be faster by 
http://bugs.python.org/issue29029 

This patch makes AC produces more FASTCALL instead of VARARGS.
When looking AC output, one downside is it produces many consts like:
+ static const char * const _keywords[] = {"", "", "", "", "", NULL};

ac_more_fastcalls.patch: Hum, I guess that your change on _PyStack_UnpackDict() is related to a bug in the function prototype. The function is unable to report failure if args is NULL. It changed the API in the change 38ab8572fde2.

New changeset d07fd6e6d449 by Victor Stinner in branch 'default':
Rename _PyArg_ParseStack to _PyArg_ParseStackAndKeywords
https://hg.python.org/cpython/rev/d07fd6e6d449
New changeset 01c57ef1b651 by Victor Stinner in branch 'default':
Add _PyArg_ParseStack() helper function
https://hg.python.org/cpython/rev/01c57ef1b651
New changeset 8bfec37ea86a by Victor Stinner in branch 'default':
Add _PyArg_NoStackKeywords() helper function
https://hg.python.org/cpython/rev/8bfec37ea86a
New changeset 38ab8572fde2 by Victor Stinner in branch 'default':
_PyStack_UnpackDict() now returns -1 on error
https://hg.python.org/cpython/rev/38ab8572fde2
New changeset de90f3d06bc9 by Victor Stinner in branch 'default':
Argument Clinic: Use METH_FASTCALL for positionals
https://hg.python.org/cpython/rev/de90f3d06bc9
New changeset dea8922751a1 by Victor Stinner in branch 'default':
Run Argument Clinic: METH_VARARGS=>METH_FASTCALL
https://hg.python.org/cpython/rev/dea8922751a1 

Naoki> This patch makes AC produces more FASTCALL instead of VARARGS.
Oh, funny, I wrote the same patch :-) (almost)
> When looking AC output, one downside is it produces many consts like:
> + static const char * const _keywords[] = {"", "", "", "", "", NULL};
Yeah, I got the same result. I tried to hack a _PyArg_ParseStack() function which doesn't take keyword argments (no kwnames), but I lost my mind in parser_init().
So I decided to simply rebase my old patches from my random CPython fastcall forks to add a simple _PyArg_ParseStack(). My implementation doesn't use the super-fast _PyArg_Parser object, but at least it allows to use METH_FASTCALL. Compared to what we had previously (METH_VARARGS), it is an enhancement :-)
I prefer to move step by step, since each commit is already big enough. It's also easier for me to maintain my forks if I push more changes upstream.
So there is still room for improvement in _PyArg_ParseStack(), but I suggest to defer further optimizations to a new issue.

New changeset 3bf78c286daf by Victor Stinner in branch 'default':
Add _PyArg_UnpackStack() function helper
https://hg.python.org/cpython/rev/3bf78c286daf
New changeset 905e902bd47e by Victor Stinner in branch 'default':
Argument Clinic: Use METH_FASTCALL for boring positionals
https://hg.python.org/cpython/rev/905e902bd47e
New changeset 52acda52b353 by Victor Stinner in branch 'default':
Run Argument Clinic: METH_VARARGS=>METH_FASTCALL
https://hg.python.org/cpython/rev/52acda52b353 

New changeset d07fd6e6d449 by Victor Stinner in branch 'default':
Rename _PyArg_ParseStack to _PyArg_ParseStackAndKeywords
https://hg.python.org/cpython/rev/d07fd6e6d449
New changeset 01c57ef1b651 by Victor Stinner in branch 'default':
Add _PyArg_ParseStack() helper function
https://hg.python.org/cpython/rev/01c57ef1b651
New changeset 8bfec37ea86a by Victor Stinner in branch 'default':
Add _PyArg_NoStackKeywords() helper function
https://hg.python.org/cpython/rev/8bfec37ea86a
New changeset 38ab8572fde2 by Victor Stinner in branch 'default':
_PyStack_UnpackDict() now returns -1 on error
https://hg.python.org/cpython/rev/38ab8572fde2
New changeset de90f3d06bc9 by Victor Stinner in branch 'default':
Argument Clinic: Use METH_FASTCALL for positionals
https://hg.python.org/cpython/rev/de90f3d06bc9
New changeset dea8922751a1 by Victor Stinner in branch 'default':
Run Argument Clinic: METH_VARARGS=>METH_FASTCALL
https://hg.python.org/cpython/rev/dea8922751a1 

Oh, I have wrote almost the same patch before going to sleep yesteday! ;) But the building crashed (likely due to a bug in _PyStack_UnpackDict()) and it was too late to resolve this.
I would prefer to rename "l" to "nargs" in PyArg_UnpackStack_impl and don't use upper case and namespace prefix in static functions.

Recently, I saw complains that I push changes too fast without reviews. This issue is a good example. So let me elaborate myself a little bit on these changes.
I'm not really proud of the "Rename _PyArg_ParseStack to _PyArg_ParseStackAndKeywords" change. It should be called _PyArg_ParseStackAndKeywords() from the beginning, but when I designed FASTCALL, my plan was to only check arguments (positional and keyword arguments) in the callee, and remove checks in the caller. I expected that all functions will get positional + keyword arguments.
I didn't know that not accepting keywords but only positional arguments like in str.replace(), was a deliberate language design choice.
So yes, for functions accepting only positional arguments, _PyArg_ParseStack() + _PyArg_NoStackKeywords() makes sense.
IMHO the implementation of the new _PyArg_ParseStack() and _PyArg_NoStackKeywords() functions was simple enough to avoid a review, they reuse existing code with minimum changes.
In general for FASTCALL, I only add private functions. I consider that it's ok to push code quickly, since the private property allows us to change these functions anytime.
Again, I consider that the "Argument Clinic: Use METH_FASTCALL for positionals" change is simple enough to avoid a review, I don't think that they are many ways to use FASTCALL for functions using positional-only parameters. I mean, I only see a single way to implement it. Again, if someone sees a new way to handle such parameters, we can change it anytime, Argument Clinic and FASTCALL are stil private.
More generally, FASTCALL requires a lot of tiny changes everywhere.
GitHub will allow to get reviews on long patch series. It will be easier to handle large changes.

New changeset 758674087b12 by Victor Stinner in branch 'default':
Issue #29286: Rename private PyArg_UnpackStack_impl() to unpack_stack()
https://hg.python.org/cpython/rev/758674087b12 

Serhiy Storchaka: "Oh, I have wrote almost the same patch before going to sleep yesteday! ;) But the building crashed (likely due to a bug in _PyStack_UnpackDict()) and it was too late to resolve this."
Oh, sorry that you wrote almost the same code. Well, at least it means that we agree on the design :-)
Serhiy: "I would prefer to rename "l" to "nargs" in PyArg_UnpackStack_impl and don't use upper case and namespace prefix in static functions."
Done.

str type still has a few methods using METH_VARARGS (slower than METH_FASTCALL):
* count()
* find()
* index()
* rfind()
* rindex()
* startswith()
* endswith()
* format()
Maybe I will propose a change later to convert these methods to Argument Clinic, but I consider that the initial issue "Currently, Argument Clinic doesn't use METH_FASTCALL for these methods, but METH_VARARGS" is now fixed, so I close the issue.
Thanks Naoki and Serhiy.

New changeset 5910fd7231d34363798d2815be2f66909e638d1c by Victor Stinner in branch 'master':
Issue #29286: Rename private PyArg_UnpackStack_impl() to unpack_stack()
https://github.com/python/cpython/commit/5910fd7231d34363798d2815be2f66909e638d1c

New changeset f0ff849adc6b4a01f9d1f08d9ad0f1511ff84541 by Victor Stinner in branch '3.6':
bpo-30524: Fix _PyStack_UnpackDict() (#1886)
https://github.com/python/cpython/commit/f0ff849adc6b4a01f9d1f08d9ad0f1511ff84541