Issue #29029: Speed up processing positional arguments in

PyArg_ParseTupleAndKeywords(), _PyArg_ParseTupleAndKeywordsFast() and like.

1 files changed, 89 insertions, 101 deletions

diff --git a/Python/getargs.c b/Python/getargs.c
index 47b00af..e3be6d9 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 (i < nargs) {
+                current_arg = PyTuple_GET_ITEM(args, i);
             }
-            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 (nkwargs && i >= pos) {
+                current_arg = PyDict_GetItemString(kwargs, kwlist[i]);
+                if (current_arg)
+                    --nkwargs;
+            }
+            else {
+                current_arg = NULL;
             }
-            else if (i < nargs)
-                current_arg = PyTuple_GET_ITEM(args, i);
 
             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,31 +2087,17 @@ 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 (i < nargs) {
+            current_arg = args[i];
         }
-        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 (nkwargs && i >= pos) {
+            keyword = PyTuple_GET_ITEM(kwtuple, i - pos);
+            current_arg = find_keyword(kwargs, kwnames, kwstack, keyword);
+            if (current_arg)
+                --nkwargs;
         }
-        else if (i < nargs) {
-            current_arg = args[i];
+        else {
+            current_arg = NULL;
         }
 
         if (current_arg) {
@@ -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;
-
-            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);
-                }
+        /* 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++;
+            }
 
-                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);
             }
         }
     }