Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-90370: Avoid temporary varargs tuple creation in argument passing #30312

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
+1,134 −212
Open

gh-90370: Avoid temporary varargs tuple creation in argument passing #30312

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b68176d
Avoid temporary `varargs` tuple creation in argument passing
colorfulappl Dec 31, 2021
43b9410
Optimize argument passing of builtin print() with modified Argument C…
colorfulappl Dec 31, 2021
9b37a77
Revert modification of PyAPI_FUNC _PyArg_UnpackKeywordsWithVararg to …
colorfulappl Jan 4, 2022
46e4472
📜🤖 Added by blurb_it.
blurb-it[bot] Jan 4, 2022
e06f343
Revert _PyArg_UnpackKeywordsWithVararg and add _PyArg_UnpackKeywordsW…
colorfulappl Jan 4, 2022
d35410b
Fix a bug which allows more than one varargs
colorfulappl Feb 22, 2022
0a9fe91
Do not copy posargs and vararg during argument parsing
colorfulappl Mar 22, 2022
9188052
Rename _PyArg_UnpackKeywordsWithVarargFast, it returns kwargs only now
colorfulappl Mar 23, 2022
d245ec0
Check type of varargs when generating argument parser
colorfulappl Mar 23, 2022
8d16685
Fix varargssize calculation in class init and add test cases
colorfulappl Mar 23, 2022
f4213ea
Merge branch 'main' into opt_ac
colorfulappl Mar 23, 2022
156f8d6
Rerun make clinic
colorfulappl Mar 23, 2022
c32aed4
Edit documentation
colorfulappl Mar 23, 2022
7ef5623
Merge branch 'main' into opt_ac
colorfulappl Feb 3, 2023
3ac2821
Fix errors introduced by merging and rerun make clinic
colorfulappl Feb 3, 2023
f63a704
Update news
colorfulappl Feb 3, 2023
8f61f5e
Fix varargssize in new_or_init
colorfulappl Feb 6, 2023
1eff215
Simplify the code a bit
colorfulappl Feb 6, 2023
5baf0f5
Optimize generated varargssize assignment
colorfulappl Feb 6, 2023
30c0606
Fix argument passing in new_or_init
colorfulappl Feb 7, 2023
d9e7408
Add tests for class method `__new__`
colorfulappl Feb 7, 2023
0f04a95
Merge branch 'main' into opt_ac
colorfulappl Feb 7, 2023
85a8ac5
Correct test function name
colorfulappl Feb 8, 2023
a9d1424
Fix vararg parsing when its name is not `args`
colorfulappl Feb 8, 2023
d6ff01a
Merge branch 'main' into opt_ac
colorfulappl Feb 8, 2023
597152e
Add testcases for undeclared keyword arguments
colorfulappl Feb 8, 2023
3119fa6
Merge branch 'main' into opt_ac
erlend-aasland Apr 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Include/modsupport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ PyAPI_FUNC(PyObject * const *) _PyArg_UnpackKeywordsWithVararg(
int minpos, int maxpos, int minkw,
int vararg, PyObject **buf);

PyAPI_FUNC(PyObject * const *) _PyArg_UnpackKeywordsWithVarargFast(
PyObject *const *args, Py_ssize_t nargs,
PyObject *kwargs, PyObject *kwnames,
struct _PyArg_Parser *parser,
int minpos, int maxpos, int minkw,
int vararg, Py_ssize_t varargssize,
PyObject **buf);

#define _PyArg_UnpackKeywords(args, nargs, kwargs, kwnames, parser, minpos, maxpos, minkw, buf) \
(((minkw) == 0 && (kwargs) == NULL && (kwnames) == NULL && \
(minpos) <= (nargs) && (nargs) <= (maxpos) && args != NULL) ? (args) : \
Expand Down
67 changes: 37 additions & 30 deletions Lib/test/clinic.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3324,14 +3324,15 @@ PyDoc_STRVAR(test_vararg_and_posonly__doc__,
{"test_vararg_and_posonly", (PyCFunction)(void(*)(void))test_vararg_and_posonly, METH_FASTCALL, test_vararg_and_posonly__doc__},

static PyObject *
test_vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args);
test_vararg_and_posonly_impl(PyObject *module, PyObject *a,
Py_ssize_t varargssize, PyObject *const *args);

static PyObject *
test_vararg_and_posonly(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
{
PyObject *return_value = NULL;
PyObject *a;
PyObject *__clinic_args = NULL;
PyObject *const *__clinic_args = NULL;

if (!_PyArg_CheckPositional("test_vararg_and_posonly", nargs, 1, PY_SSIZE_T_MAX)) {
goto exit;
Expand All @@ -3341,16 +3342,16 @@ test_vararg_and_posonly(PyObject *module, PyObject *const *args, Py_ssize_t narg
for (Py_ssize_t i = 0; i < nargs - 1; ++i) {
PyTuple_SET_ITEM(__clinic_args, i, args[1 + i]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems suspicious -- isn't __clinic_args a pointer-to-PyObject-pointer, not a pointer-to-tuple?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My fault. The tuple doesn't need to be created.

}
return_value = test_vararg_and_posonly_impl(module, a, __clinic_args);
return_value = test_vararg_and_posonly_impl(module, a, varargssize, __clinic_args);

exit:
Py_XDECREF(__clinic_args);
return return_value;
}

static PyObject *
test_vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args)
/*[clinic end generated code: output=ada613d2d87c9341 input=08dc2bf7afbf1613]*/
test_vararg_and_posonly_impl(PyObject *module, PyObject *a,
Py_ssize_t varargssize, PyObject *const *args)
/*[clinic end generated code: output=6a6c69c6ff7deb9b input=08dc2bf7afbf1613]*/

/*[clinic input]
test_vararg
Expand All @@ -3370,7 +3371,8 @@ PyDoc_STRVAR(test_vararg__doc__,
{"test_vararg", (PyCFunction)(void(*)(void))test_vararg, METH_FASTCALL|METH_KEYWORDS, test_vararg__doc__},

static PyObject *
test_vararg_impl(PyObject *module, PyObject *a, PyObject *args);
test_vararg_impl(PyObject *module, PyObject *a, Py_ssize_t varargssize,
PyObject *const *args);

static PyObject *
test_vararg(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
Expand All @@ -3379,26 +3381,27 @@ test_vararg(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject
static const char * const _keywords[] = {"a", NULL};
static _PyArg_Parser _parser = {NULL, _keywords, "test_vararg", 0};
PyObject *argsbuf[2];
Py_ssize_t varargssize = Py_MAX(nargs - 1, 0);
Py_ssize_t noptargs = 0 + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 1;
PyObject *a;
PyObject *__clinic_args = NULL;
PyObject *const *__clinic_args = NULL;

args = _PyArg_UnpackKeywordsWithVararg(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, 1, argsbuf);
args = _PyArg_UnpackKeywordsWithVarargFast(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, 1, varargssize, argsbuf);
if (!args) {
goto exit;
}
a = args[0];
__clinic_args = args[1];
return_value = test_vararg_impl(module, a, __clinic_args);
__clinic_args = (PyObject *const *)args[1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to avoid such unsafe casts. Maybe passing a PyObject *** into _PyArg_UnpackKeywordsWithVarargFast instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This copy and cast are unnecessary.
Maybe __clinic_args = args + 1; is enough, note the args refers to the 2nd argument of function test_vararg, assuming it's not assigned in line 3389.

return_value = test_vararg_impl(module, a, varargssize, __clinic_args);

exit:
Py_XDECREF(__clinic_args);
return return_value;
}

static PyObject *
test_vararg_impl(PyObject *module, PyObject *a, PyObject *args)
/*[clinic end generated code: output=f721025731c3bfe8 input=81d33815ad1bae6e]*/
test_vararg_impl(PyObject *module, PyObject *a, Py_ssize_t varargssize,
PyObject *const *args)
/*[clinic end generated code: output=b9cb9484bd7303db input=81d33815ad1bae6e]*/

/*[clinic input]
test_vararg_with_default
Expand All @@ -3419,7 +3422,8 @@ PyDoc_STRVAR(test_vararg_with_default__doc__,
{"test_vararg_with_default", (PyCFunction)(void(*)(void))test_vararg_with_default, METH_FASTCALL|METH_KEYWORDS, test_vararg_with_default__doc__},

static PyObject *
test_vararg_with_default_impl(PyObject *module, PyObject *a, PyObject *args,
test_vararg_with_default_impl(PyObject *module, PyObject *a,
Py_ssize_t varargssize, PyObject *const *args,
int b);

static PyObject *
Expand All @@ -3429,17 +3433,18 @@ test_vararg_with_default(PyObject *module, PyObject *const *args, Py_ssize_t nar
static const char * const _keywords[] = {"a", "b", NULL};
static _PyArg_Parser _parser = {NULL, _keywords, "test_vararg_with_default", 0};
PyObject *argsbuf[3];
Py_ssize_t varargssize = Py_MAX(nargs - 1, 0);
Py_ssize_t noptargs = 0 + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 1;
PyObject *a;
PyObject *__clinic_args = NULL;
PyObject *const *__clinic_args = NULL;
int b = 0;

args = _PyArg_UnpackKeywordsWithVararg(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, 1, argsbuf);
args = _PyArg_UnpackKeywordsWithVarargFast(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, 1, varargssize, argsbuf);
if (!args) {
goto exit;
}
a = args[0];
__clinic_args = args[1];
__clinic_args = (PyObject *const *)args[1];
if (!noptargs) {
goto skip_optional_kwonly;
}
Expand All @@ -3448,17 +3453,17 @@ test_vararg_with_default(PyObject *module, PyObject *const *args, Py_ssize_t nar
goto exit;
}
skip_optional_kwonly:
return_value = test_vararg_with_default_impl(module, a, __clinic_args, b);
return_value = test_vararg_with_default_impl(module, a, varargssize, __clinic_args, b);

exit:
Py_XDECREF(__clinic_args);
return return_value;
}

static PyObject *
test_vararg_with_default_impl(PyObject *module, PyObject *a, PyObject *args,
test_vararg_with_default_impl(PyObject *module, PyObject *a,
Py_ssize_t varargssize, PyObject *const *args,
int b)
/*[clinic end generated code: output=63b34d3241c52fda input=6e110b54acd9b22d]*/
/*[clinic end generated code: output=d38066fd9fa9336f input=6e110b54acd9b22d]*/

/*[clinic input]
test_vararg_with_only_defaults
Expand All @@ -3479,7 +3484,8 @@ PyDoc_STRVAR(test_vararg_with_only_defaults__doc__,
{"test_vararg_with_only_defaults", (PyCFunction)(void(*)(void))test_vararg_with_only_defaults, METH_FASTCALL|METH_KEYWORDS, test_vararg_with_only_defaults__doc__},

static PyObject *
test_vararg_with_only_defaults_impl(PyObject *module, PyObject *args, int b,
test_vararg_with_only_defaults_impl(PyObject *module, Py_ssize_t varargssize,
PyObject *const *args, int b,
PyObject *c);

static PyObject *
Expand All @@ -3489,16 +3495,17 @@ test_vararg_with_only_defaults(PyObject *module, PyObject *const *args, Py_ssize
static const char * const _keywords[] = {"b", "c", NULL};
static _PyArg_Parser _parser = {NULL, _keywords, "test_vararg_with_only_defaults", 0};
PyObject *argsbuf[3];
Py_ssize_t varargssize = Py_MAX(nargs - 0, 0);
Py_ssize_t noptargs = 0 + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 0;
PyObject *__clinic_args = NULL;
PyObject *const *__clinic_args = NULL;
int b = 0;
PyObject *c = " ";

args = _PyArg_UnpackKeywordsWithVararg(args, nargs, NULL, kwnames, &_parser, 0, 0, 0, 0, argsbuf);
args = _PyArg_UnpackKeywordsWithVarargFast(args, nargs, NULL, kwnames, &_parser, 0, 0, 0, 0, varargssize, argsbuf);
if (!args) {
goto exit;
}
__clinic_args = args[0];
__clinic_args = (PyObject *const *)args[0];
if (!noptargs) {
goto skip_optional_kwonly;
}
Expand All @@ -3513,14 +3520,14 @@ test_vararg_with_only_defaults(PyObject *module, PyObject *const *args, Py_ssize
}
c = args[2];
skip_optional_kwonly:
return_value = test_vararg_with_only_defaults_impl(module, __clinic_args, b, c);
return_value = test_vararg_with_only_defaults_impl(module, varargssize, __clinic_args, b, c);

exit:
Py_XDECREF(__clinic_args);
return return_value;
}

static PyObject *
test_vararg_with_only_defaults_impl(PyObject *module, PyObject *args, int b,
test_vararg_with_only_defaults_impl(PyObject *module, Py_ssize_t varargssize,
PyObject *const *args, int b,
PyObject *c)
/*[clinic end generated code: output=dc29ce6ebc2ec10c input=fa56a709a035666e]*/
/*[clinic end generated code: output=833c72a533c80a0e input=fa56a709a035666e]*/
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2022-01-04-08-09-32.bpo-46212._Fvxj6.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid creating temporary varargs tuple in argument passing with "Argument Clinic generated code".
11 changes: 6 additions & 5 deletions Python/bltinmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1963,9 +1963,10 @@ Prints the values to a stream, or to sys.stdout by default.
[clinic start generated code]*/

static PyObject *
builtin_print_impl(PyObject *module, PyObject *args, PyObject *sep,
PyObject *end, PyObject *file, int flush)
/*[clinic end generated code: output=3cfc0940f5bc237b input=c143c575d24fe665]*/
builtin_print_impl(PyObject *module, Py_ssize_t varargssize,
PyObject *const *args, PyObject *sep, PyObject *end,
PyObject *file, int flush)
/*[clinic end generated code: output=50b729b4a8731c69 input=c143c575d24fe665]*/
{
int i, err;

Expand Down Expand Up @@ -2001,7 +2002,7 @@ builtin_print_impl(PyObject *module, PyObject *args, PyObject *sep,
return NULL;
}

for (i = 0; i < PyTuple_GET_SIZE(args); i++) {
for (i = 0; i < varargssize; i++) {
if (i > 0) {
if (sep == NULL) {
err = PyFile_WriteString(" ", file);
Expand All @@ -2013,7 +2014,7 @@ builtin_print_impl(PyObject *module, PyObject *args, PyObject *sep,
return NULL;
}
}
err = PyFile_WriteObject(PyTuple_GET_ITEM(args, i), file, Py_PRINT_RAW);
err = PyFile_WriteObject(args[i], file, Py_PRINT_RAW);
if (err) {
return NULL;
}
Expand Down
17 changes: 9 additions & 8 deletions Python/clinic/bltinmodule.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading