From 1741441649fca5a4e642817ca475d5edcf43f37e Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 17 Jan 2017 10:07:25 +0200 Subject: [PATCH] Issue #29029: Speed up processing positional arguments in PyArg_ParseTupleAndKeywords(), _PyArg_ParseTupleAndKeywordsFast() and like. --- Python/getargs.c | 194 ++++++++++++++++++++++------------------------- 1 file changed, 91 insertions(+), 103 deletions(-) diff --git a/Python/getargs.c b/Python/getargs.c index 47b00af4427..e3be6d92802 100644 --- a/Python/getargs.c +++ b/Python/getargs.c @@ -1598,7 +1598,7 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, { char msgbuf[512]; int levels[32]; - const char *fname, *msg, *custom_msg, *keyword; + const char *fname, *msg, *custom_msg; int min = INT_MAX; int max = INT_MAX; int i, pos, len; @@ -1666,7 +1666,6 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, /* convert tuple args and keyword args in same loop, using kwlist to drive process */ for (i = 0; i < len; i++) { - keyword = kwlist[i]; if (*format == '|') { if (min != INT_MAX) { PyErr_SetString(PyExc_SystemError, @@ -1720,26 +1719,17 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, return cleanreturn(0, &freelist); } if (!skip) { - current_arg = NULL; - if (nkwargs && i >= pos) { - current_arg = PyDict_GetItemString(kwargs, keyword); - if (!current_arg && PyErr_Occurred()) { - return cleanreturn(0, &freelist); - } - } - if (current_arg) { - --nkwargs; - if (i < nargs) { - /* arg present in tuple and in dict */ - PyErr_Format(PyExc_TypeError, - "Argument given by name ('%s') " - "and position (%d)", - keyword, i+1); - return cleanreturn(0, &freelist); - } - } - else if (i < nargs) + if (i < nargs) { current_arg = PyTuple_GET_ITEM(args, i); + } + else if (nkwargs && i >= pos) { + current_arg = PyDict_GetItemString(kwargs, kwlist[i]); + if (current_arg) + --nkwargs; + } + else { + current_arg = NULL; + } if (current_arg) { msg = convertitem(current_arg, &format, p_va, flags, @@ -1763,8 +1753,8 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, } else { PyErr_Format(PyExc_TypeError, "Required argument " - "'%s' (pos %d) not found", - keyword, i+1); + "'%s' (pos %d) not found", + kwlist[i], i+1); return cleanreturn(0, &freelist); } } @@ -1803,19 +1793,32 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, return cleanreturn(0, &freelist); } - /* make sure there are no extraneous keyword arguments */ if (nkwargs > 0) { - PyObject *key, *value; - Py_ssize_t pos = 0; - while (PyDict_Next(kwargs, &pos, &key, &value)) { + PyObject *key; + Py_ssize_t j; + /* make sure there are no arguments given by name and position */ + for (i = pos; i < nargs; i++) { + current_arg = PyDict_GetItemString(kwargs, kwlist[i]); + if (current_arg) { + /* arg present in tuple and in dict */ + PyErr_Format(PyExc_TypeError, + "Argument given by name ('%s') " + "and position (%d)", + kwlist[i], i+1); + return cleanreturn(0, &freelist); + } + } + /* make sure there are no extraneous keyword arguments */ + j = 0; + while (PyDict_Next(kwargs, &j, &key, NULL)) { int match = 0; if (!PyUnicode_Check(key)) { PyErr_SetString(PyExc_TypeError, "keywords must be strings"); return cleanreturn(0, &freelist); } - for (i = 0; i < len; i++) { - if (*kwlist[i] && _PyUnicode_EqualToASCIIString(key, kwlist[i])) { + for (i = pos; i < len; i++) { + if (_PyUnicode_EqualToASCIIString(key, kwlist[i])) { match = 1; break; } @@ -1963,10 +1966,13 @@ parser_clear(struct _PyArg_Parser *parser) } static PyObject* -find_keyword(PyObject *kwnames, PyObject **kwstack, PyObject *key) +find_keyword(PyObject *kwargs, PyObject *kwnames, PyObject **kwstack, PyObject *key) { Py_ssize_t i, nkwargs; + if (kwargs != NULL) { + return PyDict_GetItem(kwargs, key); + } nkwargs = PyTuple_GET_SIZE(kwnames); for (i=0; i < nkwargs; i++) { PyObject *kwname = PyTuple_GET_ITEM(kwnames, i); @@ -1978,7 +1984,7 @@ find_keyword(PyObject *kwnames, PyObject **kwstack, PyObject *key) } if (!PyUnicode_Check(kwname)) { /* ignore non-string keyword keys: - an error will be raised above */ + an error will be raised below */ continue; } if (_PyUnicode_EQ(kwname, key)) { @@ -2012,8 +2018,7 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, freelist.entries_malloced = 0; assert(kwargs == NULL || PyDict_Check(kwargs)); - assert((kwargs != NULL || kwnames != NULL) - || (kwargs == NULL && kwnames == NULL)); + assert(kwargs == NULL || kwnames == NULL); assert(p_va != NULL); if (parser == NULL) { @@ -2074,7 +2079,6 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, format = parser->format; /* convert tuple args and keyword args in same loop, using kwtuple to drive process */ for (i = 0; i < len; i++) { - keyword = (i >= pos) ? PyTuple_GET_ITEM(kwtuple, i - pos) : NULL; if (*format == '|') { format++; } @@ -2083,32 +2087,18 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, } assert(!IS_END_OF_FORMAT(*format)); - current_arg = NULL; - if (nkwargs && i >= pos) { - if (kwargs != NULL) { - current_arg = PyDict_GetItem(kwargs, keyword); - if (!current_arg && PyErr_Occurred()) { - return cleanreturn(0, &freelist); - } - } - else { - current_arg = find_keyword(kwnames, kwstack, keyword); - } - } - if (current_arg) { - --nkwargs; - if (i < nargs) { - /* arg present in tuple and in dict */ - PyErr_Format(PyExc_TypeError, - "Argument given by name ('%U') " - "and position (%d)", - keyword, i+1); - return cleanreturn(0, &freelist); - } - } - else if (i < nargs) { + if (i < nargs) { current_arg = args[i]; } + else if (nkwargs && i >= pos) { + keyword = PyTuple_GET_ITEM(kwtuple, i - pos); + current_arg = find_keyword(kwargs, kwnames, kwstack, keyword); + if (current_arg) + --nkwargs; + } + else { + current_arg = NULL; + } if (current_arg) { msg = convertitem(current_arg, &format, p_va, flags, @@ -2123,13 +2113,15 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, if (i < parser->min) { /* Less arguments than required */ if (i < pos) { + Py_ssize_t min = Py_MIN(pos, parser->min); PyErr_Format(PyExc_TypeError, "Function takes %s %d positional arguments" " (%d given)", - (Py_MIN(pos, parser->min) < parser->max) ? "at least" : "exactly", - Py_MIN(pos, parser->min), nargs); + min < parser->max ? "at least" : "exactly", + min, nargs); } else { + keyword = PyTuple_GET_ITEM(kwtuple, i - pos); PyErr_Format(PyExc_TypeError, "Required argument " "'%U' (pos %d) not found", keyword, i+1); @@ -2152,54 +2144,50 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, assert(IS_END_OF_FORMAT(*format) || (*format == '|') || (*format == '$')); - /* make sure there are no extraneous keyword arguments */ if (nkwargs > 0) { - if (kwargs != NULL) { - PyObject *key, *value; - Py_ssize_t pos = 0; - while (PyDict_Next(kwargs, &pos, &key, &value)) { - int match; - if (!PyUnicode_Check(key)) { - PyErr_SetString(PyExc_TypeError, - "keywords must be strings"); - return cleanreturn(0, &freelist); - } - match = PySequence_Contains(kwtuple, key); - if (match <= 0) { - if (!match) { - PyErr_Format(PyExc_TypeError, - "'%U' is an invalid keyword " - "argument for this function", - key); - } - return cleanreturn(0, &freelist); - } + Py_ssize_t j; + /* make sure there are no arguments given by name and position */ + for (i = pos; i < nargs; i++) { + keyword = PyTuple_GET_ITEM(kwtuple, i - pos); + current_arg = find_keyword(kwargs, kwnames, kwstack, keyword); + if (current_arg) { + /* arg present in tuple and in dict */ + PyErr_Format(PyExc_TypeError, + "Argument given by name ('%U') " + "and position (%d)", + keyword, i+1); + return cleanreturn(0, &freelist); } } - else { - Py_ssize_t j, nkwargs; + /* make sure there are no extraneous keyword arguments */ + j = 0; + while (1) { + int match; + if (kwargs != NULL) { + if (!PyDict_Next(kwargs, &j, &keyword, NULL)) + break; + } + else { + if (j >= PyTuple_GET_SIZE(kwnames)) + break; + keyword = PyTuple_GET_ITEM(kwnames, j); + j++; + } - nkwargs = PyTuple_GET_SIZE(kwnames); - for (j=0; j < nkwargs; j++) { - PyObject *key = PyTuple_GET_ITEM(kwnames, j); - int match; - - if (!PyUnicode_Check(key)) { - PyErr_SetString(PyExc_TypeError, - "keywords must be strings"); - return cleanreturn(0, &freelist); - } - - match = PySequence_Contains(kwtuple, key); - if (match <= 0) { - if (!match) { - PyErr_Format(PyExc_TypeError, - "'%U' is an invalid keyword " - "argument for this function", - key); - } - return cleanreturn(0, &freelist); + if (!PyUnicode_Check(keyword)) { + PyErr_SetString(PyExc_TypeError, + "keywords must be strings"); + return cleanreturn(0, &freelist); + } + match = PySequence_Contains(kwtuple, keyword); + if (match <= 0) { + if (!match) { + PyErr_Format(PyExc_TypeError, + "'%U' is an invalid keyword " + "argument for this function", + keyword); } + return cleanreturn(0, &freelist); } } }