From a9413ab30be521e9426234b26cd228317d718f6b Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Wed, 29 Dec 2021 14:37:16 +0800 Subject: [PATCH 1/4] Builtin functions min() and max() now use METH_FASTCALL --- Python/bltinmodule.c | 73 +++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 6763f9969707d2..fa2dd125d2acfb 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -1705,35 +1705,27 @@ builtin_locals_impl(PyObject *module) static PyObject * -min_max(PyObject *args, PyObject *kwds, int op) +min_max(PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames, int op) { - PyObject *v, *it, *item, *val, *maxitem, *maxval, *keyfunc=NULL; - PyObject *emptytuple, *defaultval = NULL; - static char *kwlist[] = {"key", "default", NULL}; - const char *name = op == Py_LT ? "min" : "max"; - const int positional = PyTuple_Size(args) > 1; - int ret; + PyObject *it, *item, *val, *maxitem, *maxval, *keyfunc=NULL; + PyObject *defaultval = NULL; + static const char * const keywords[] = {"key", "default", NULL}; + static _PyArg_Parser _parser_min = {"|$OO:min", keywords, 0}; + static _PyArg_Parser _parser_max = {"|$OO:max", keywords, 0}; + const char *name = (op == Py_LT) ? "min" : "max"; + _PyArg_Parser *_parser = (op == Py_LT) ? &_parser_min : &_parser_max; - if (positional) { - v = args; - } - else if (!PyArg_UnpackTuple(args, name, 1, 1, &v)) { - if (PyExceptionClass_Check(PyExc_TypeError)) { - PyErr_Format(PyExc_TypeError, "%s expected at least 1 argument, got 0", name); - } + if (nargs == 0) { + PyErr_Format(PyExc_TypeError, "%s expected at least 1 argument, got 0", name); return NULL; } - emptytuple = PyTuple_New(0); - if (emptytuple == NULL) - return NULL; - ret = PyArg_ParseTupleAndKeywords(emptytuple, kwds, - (op == Py_LT) ? "|$OO:min" : "|$OO:max", - kwlist, &keyfunc, &defaultval); - Py_DECREF(emptytuple); - if (!ret) + if (kwnames != NULL && !_PyArg_ParseStackAndKeywords(args + nargs, 0, kwnames, _parser, + &keyfunc, &defaultval)) { return NULL; + } + const int positional = nargs > 1; // False iff nargs == 1 if (positional && defaultval != NULL) { PyErr_Format(PyExc_TypeError, "Cannot specify a default for %s() with multiple " @@ -1741,9 +1733,11 @@ min_max(PyObject *args, PyObject *kwds, int op) return NULL; } - it = PyObject_GetIter(v); - if (it == NULL) { - return NULL; + if (!positional) { + it = PyObject_GetIter(args[0]); + if (it == NULL) { + return NULL; + } } if (keyfunc == Py_None) { @@ -1752,7 +1746,14 @@ min_max(PyObject *args, PyObject *kwds, int op) maxitem = NULL; /* the result */ maxval = NULL; /* the value associated with the result */ - while (( item = PyIter_Next(it) )) { + int i = 0; + while (positional ? + ((i < nargs) && (item = args[i++])) + : !!(item = PyIter_Next(it))) { + if (positional) { + Py_INCREF(item); + } + /* get the value from the key function */ if (keyfunc != NULL) { val = PyObject_CallOneArg(keyfunc, item); @@ -1801,7 +1802,9 @@ min_max(PyObject *args, PyObject *kwds, int op) } else Py_DECREF(maxval); - Py_DECREF(it); + if (!positional) { + Py_DECREF(it); + } return maxitem; Fail_it_item_and_val: @@ -1811,15 +1814,17 @@ min_max(PyObject *args, PyObject *kwds, int op) Fail_it: Py_XDECREF(maxval); Py_XDECREF(maxitem); - Py_DECREF(it); + if (!positional) { + Py_DECREF(it); + } return NULL; } /* AC: cannot convert yet, waiting for *args support */ static PyObject * -builtin_min(PyObject *self, PyObject *args, PyObject *kwds) +builtin_min(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { - return min_max(args, kwds, Py_LT); + return min_max(args, nargs, kwnames, Py_LT); } PyDoc_STRVAR(min_doc, @@ -1834,9 +1839,9 @@ With two or more arguments, return the smallest argument."); /* AC: cannot convert yet, waiting for *args support */ static PyObject * -builtin_max(PyObject *self, PyObject *args, PyObject *kwds) +builtin_max(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { - return min_max(args, kwds, Py_GT); + return min_max(args, nargs, kwnames, Py_GT); } PyDoc_STRVAR(max_doc, @@ -2954,8 +2959,8 @@ static PyMethodDef builtin_methods[] = { BUILTIN_AITER_METHODDEF BUILTIN_LEN_METHODDEF BUILTIN_LOCALS_METHODDEF - {"max", (PyCFunction)(void(*)(void))builtin_max, METH_VARARGS | METH_KEYWORDS, max_doc}, - {"min", (PyCFunction)(void(*)(void))builtin_min, METH_VARARGS | METH_KEYWORDS, min_doc}, + {"max", (PyCFunction)(void(*)(void))builtin_max, METH_FASTCALL | METH_KEYWORDS, max_doc}, + {"min", (PyCFunction)(void(*)(void))builtin_min, METH_FASTCALL | METH_KEYWORDS, min_doc}, {"next", (PyCFunction)(void(*)(void))builtin_next, METH_FASTCALL, next_doc}, BUILTIN_ANEXT_METHODDEF BUILTIN_OCT_METHODDEF From 9852e27fc7a8e6fae952217300d80b7075dce558 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 11 Dec 2023 19:45:23 +0200 Subject: [PATCH 2/4] Fix compiler warnings. --- Python/bltinmodule.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 40ccf8eb734a84..f609210971efb0 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -1768,7 +1768,7 @@ builtin_locals_impl(PyObject *module) static PyObject * min_max(PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames, int op) { - PyObject *it, *item, *val, *maxitem, *maxval, *keyfunc=NULL; + PyObject *it = NULL, *item, *val, *maxitem, *maxval, *keyfunc=NULL; PyObject *defaultval = NULL; static const char * const keywords[] = {"key", "default", NULL}; static _PyArg_Parser _parser_min = {"|$OO:min", keywords, 0}; @@ -1807,13 +1807,23 @@ min_max(PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames, int op) maxitem = NULL; /* the result */ maxval = NULL; /* the value associated with the result */ - int i = 0; - while (positional ? - ((i < nargs) && (item = args[i++])) - : !!(item = PyIter_Next(it))) { - if (positional) { + while (1) { + if (it == NULL) { + if (nargs-- <= 0) { + break; + } + item = *args++; Py_INCREF(item); } + else { + item = PyIter_Next(it); + if (item == NULL) { + if (PyErr_Occurred()) { + goto Fail_it; + } + break; + } + } /* get the value from the key function */ if (keyfunc != NULL) { @@ -1848,8 +1858,6 @@ min_max(PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames, int op) } } } - if (PyErr_Occurred()) - goto Fail_it; if (maxval == NULL) { assert(maxitem == NULL); if (defaultval != NULL) { @@ -1861,9 +1869,7 @@ min_max(PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames, int op) } else Py_DECREF(maxval); - if (!positional) { - Py_DECREF(it); - } + Py_XDECREF(it); return maxitem; Fail_it_item_and_val: @@ -1873,9 +1879,7 @@ min_max(PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames, int op) Fail_it: Py_XDECREF(maxval); Py_XDECREF(maxitem); - if (!positional) { - Py_DECREF(it); - } + Py_XDECREF(it); return NULL; } From 610c3d576d6af84cfa576adbdc9d956a964ab892 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 11 Dec 2023 19:52:07 +0200 Subject: [PATCH 3/4] Use _PyCFunction_CAST. --- Python/bltinmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index f609210971efb0..e54d5cbacdc96f 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -3063,8 +3063,8 @@ static PyMethodDef builtin_methods[] = { BUILTIN_AITER_METHODDEF BUILTIN_LEN_METHODDEF BUILTIN_LOCALS_METHODDEF - {"max", (PyCFunction)(void(*)(void))builtin_max, METH_FASTCALL | METH_KEYWORDS, max_doc}, - {"min", (PyCFunction)(void(*)(void))builtin_min, METH_FASTCALL | METH_KEYWORDS, min_doc}, + {"max", _PyCFunction_CAST(builtin_max), METH_FASTCALL | METH_KEYWORDS, max_doc}, + {"min", _PyCFunction_CAST(builtin_min), METH_FASTCALL | METH_KEYWORDS, min_doc}, {"next", _PyCFunction_CAST(builtin_next), METH_FASTCALL, next_doc}, BUILTIN_ANEXT_METHODDEF BUILTIN_OCT_METHODDEF From 3a0413e9a041e239c880a86fd931d150df335bb9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 11 Dec 2023 19:53:40 +0200 Subject: [PATCH 4/4] Add a NEWS entry. --- .../2023-12-11-19-53-32.gh-issue-90350.-FQy3E.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-12-11-19-53-32.gh-issue-90350.-FQy3E.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-12-11-19-53-32.gh-issue-90350.-FQy3E.rst b/Misc/NEWS.d/next/Core and Builtins/2023-12-11-19-53-32.gh-issue-90350.-FQy3E.rst new file mode 100644 index 00000000000000..6b7881bbd19f59 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-12-11-19-53-32.gh-issue-90350.-FQy3E.rst @@ -0,0 +1 @@ +Optimize builtin functions :func:`min` and :func:`max`.