Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@colorfulappl

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

Thanks for the PR and benchmarks @colorfulappl, I am current blocked a bit but plan to review this soon!

Thanks for the PR and benchmarks @colorfulappl, I am current blocked a bit but plan to review this soon!

Thank you @isidentical !
With @sweeneyde 's comment I have found that my patch is actually buggy.
I am writing a new patch but blocked by some affairs last a few days.
You can review after I push a new commit and ignore this version. Sorry :)

Sure, then just ping me whenever the new patch ready and I'll try to take a look at it then!

Sorry for such a long delay.

When generate parsing code for function with varargs, this PR:

1. Introduce a new function _PyArg_UnpackKeywordsWithVarargKwonly.
   It's a copy of _PyArg_UnpackKeywordsWithVararg but copying of positional args and packing of varargs are deleted.
   _PyArg_UnpackKeywordsWithVarargKwonly returns an array pointer of parsed keyword args only.
2. Positional args are not copied to argsbuf. argsbuf only saves parsed keyword args.
3. Pass varargsize and vararg array pointer to XXXX_impl, this avoid pack/unpack of varargs.

I'd really appreciate it if you could take a look and give me some advice @isidentical .

Benchmark Result

Benchmark:
https://bugs.python.org/file50533

Environment:
OS: macOS 12.3
CPU: Apple M1 Pro
Compiler: Apple clang version 13.1.6
Build args: --enable-optimizations --with-lto

+-----------------------------------------------------+---------+-----------------------+
| Benchmark                                           | base    | opt                   |
+=====================================================+=========+=======================+
| print(a)                                            | 133 ns  | 126 ns: 1.06x faster  |
+-----------------------------------------------------+---------+-----------------------+
| print(a, b, c)                                      | 337 ns  | 326 ns: 1.04x faster  |
+-----------------------------------------------------+---------+-----------------------+
| print(a, b, c, *v)                                  | 679 ns  | 661 ns: 1.03x faster  |
+-----------------------------------------------------+---------+-----------------------+
| print(a, sep='', file=stdout)                       | 134 ns  | 125 ns: 1.07x faster  |
+-----------------------------------------------------+---------+-----------------------+
| print(a, sep='', flush=True, file=stdout)           | 571 ns  | 563 ns: 1.01x faster  |
+-----------------------------------------------------+---------+-----------------------+
| print(*v, sep='', flush=True, file=stdout)          | 833 ns  | 819 ns: 1.02x faster  |
+-----------------------------------------------------+---------+-----------------------+
| print(a, b, c, *v, sep='', flush=True, file=stdout) | 1.18 us | 1.16 us: 1.02x faster |
+-----------------------------------------------------+---------+-----------------------+
| Geometric mean                                      | (ref)   | 1.03x faster          |
+-----------------------------------------------------+---------+-----------------------+

I found there are some bugs when argument clinic processes varargs and made a fix in #32092.
Maybe this patch should wait until these bugs are fixed?

we prefer keeping all of the PR history since it makes reviewing easier, so please do not force-push.

Thanks for the advice; I will pay attention to it.

I added some test cases for the class method __new__ (without actually creating the objects, only packing and returning the arguments). And I found that when the __new__ or __init__ function accepts a vararg only, the argument clinic generated code does not check keyword arguments.

e.g.

# Signature of this function:
#
# @classmethod
# _testclinic.TestClassVarargOnly.__new__(cls, *args)

test_class_new_vararg_only(kw=1)

# Here we expect an error like "TypeError: TestClassVarargOnly.__new__() got an unexpected keyword argument 'kw'",
# but nothing raised.

We may need to fix it in another issue.

Not sure, if this is a good idea, but could you consider using nargs instead of varargsize for *_impl() functions? This will reduce needs for changing the body of converted function (take math_gcd as an example to convert from scratch).

When "Augument Clinic generated code" are parsing arguments, the args are packed to a tuple before passing to callee. This may be unnecessary.

This patch intends to reduce the overhead on passing vararg from Argument Clinic generated code to user-implemented *.impl() function.

Before this patch, vararg arguments are packed to a tuple, *_impl() function fetches these arguments by access the tuple items. In such a case, the number of vararg is implied in the tuple object.
After this patch, a pointer which points to vararg array is passed to *_impl() function directly, so the varargssize is necessary to tell the user how many vararg arguments is received.

but could you consider using nargs instead of varargsize for *_impl() functions?

@skirpichev Do you mean pass both nargs and max_pos to *_impl() function and let the user to calculate the number of vararg arguments? This may be confusing for users, and violate the purpose of Argument Clinic (Argument Clinic How-To - Abstract):

Argument Clinic is a preprocessor for CPython C files. Its purpose is to automate all the boilerplate involved with writing argument parsing code for “builtins”.

Indeed, my suggestion only valid for only-vararg case (as gcd, lcm and so on). Not sure if this case worth a special treatment...

@colorfulappl, could you fix merge conflicts?

Thanks for your work, but this has merge conflicts. Also, issue seems to be fixed. I'm closing this.