From 7c81286074fd3e425f29be4e5d65f5669c141f56 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Mon, 15 Sep 2025 13:08:30 +0200 Subject: [PATCH 1/8] gh-138912: Improve MATCH_CLASS opcode performance --- ...-09-15-13-28-48.gh-issue-138912.61EYbn.rst | 1 + Python/ceval.c | 61 ++++++++++--------- 2 files changed, 32 insertions(+), 30 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst new file mode 100644 index 00000000000000..e0008c3bbfe3e1 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst @@ -0,0 +1 @@ +Improve :opcode:`MATCH_CLASS` performance. Patch by Marc Mueller diff --git a/Python/ceval.c b/Python/ceval.c index 578c5d2a8b1420..76e9db0754c302 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -709,15 +709,17 @@ match_class_attr(PyThreadState *tstate, PyObject *subject, PyObject *type, PyObject *name, PyObject *seen) { assert(PyUnicode_CheckExact(name)); - assert(PySet_CheckExact(seen)); - if (PySet_Contains(seen, name) || PySet_Add(seen, name)) { - if (!_PyErr_Occurred(tstate)) { - // Seen it before! - _PyErr_Format(tstate, PyExc_TypeError, - "%s() got multiple sub-patterns for attribute %R", - ((PyTypeObject*)type)->tp_name, name); + if (seen != NULL) { + assert(PySet_CheckExact(seen)); + if (PySet_Contains(seen, name) || PySet_Add(seen, name)) { + if (!_PyErr_Occurred(tstate)) { + // Seen it before! + _PyErr_Format(tstate, PyExc_TypeError, + "%s() got multiple sub-patterns for attribute %R", + ((PyTypeObject*)type)->tp_name, name); + } + return NULL; } - return NULL; } PyObject *attr; (void)PyObject_GetOptionalAttr(subject, name, &attr); @@ -740,14 +742,24 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (PyObject_IsInstance(subject, type) <= 0) { return NULL; } + // Short circuit if there aren't any arguments: + Py_ssize_t nkwargs = PyTuple_GET_SIZE(kwargs); + Py_ssize_clean_t nattrs = nargs + nkwargs; + if (!nattrs) { + PyObject *attrs = PyTuple_New(0); + return attrs; + } // So far so good: - PyObject *seen = PySet_New(NULL); - if (seen == NULL) { - return NULL; + PyObject *seen = NULL; + if (nattrs > 1) { + seen = PySet_New(NULL); + if (seen == NULL) { + return NULL; + } } - PyObject *attrs = PyList_New(0); + PyObject *attrs = PyTuple_New(nattrs); if (attrs == NULL) { - Py_DECREF(seen); + Py_XDECREF(seen); return NULL; } // NOTE: From this point on, goto fail on failure: @@ -788,9 +800,7 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } if (match_self) { // Easy. Copy the subject itself, and move on to kwargs. - if (PyList_Append(attrs, subject) < 0) { - goto fail; - } + PyTuple_SET_ITEM(attrs, 0, subject); } else { for (Py_ssize_t i = 0; i < nargs; i++) { @@ -806,36 +816,27 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (attr == NULL) { goto fail; } - if (PyList_Append(attrs, attr) < 0) { - Py_DECREF(attr); - goto fail; - } - Py_DECREF(attr); + PyTuple_SET_ITEM(attrs, i, attr); } } Py_CLEAR(match_args); } // Finally, the keyword subpatterns: - for (Py_ssize_t i = 0; i < PyTuple_GET_SIZE(kwargs); i++) { + for (Py_ssize_t i = 0; i < nkwargs; i++) { PyObject *name = PyTuple_GET_ITEM(kwargs, i); PyObject *attr = match_class_attr(tstate, subject, type, name, seen); if (attr == NULL) { goto fail; } - if (PyList_Append(attrs, attr) < 0) { - Py_DECREF(attr); - goto fail; - } - Py_DECREF(attr); + PyTuple_SET_ITEM(attrs, nargs + i, attr); } - Py_SETREF(attrs, PyList_AsTuple(attrs)); - Py_DECREF(seen); + Py_XDECREF(seen); return attrs; fail: // We really don't care whether an error was raised or not... that's our // caller's problem. All we know is that the match failed. Py_XDECREF(match_args); - Py_DECREF(seen); + Py_XDECREF(seen); Py_DECREF(attrs); return NULL; } From 597adb0d484a5a504c17c0b158acefa8205cec83 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Mon, 15 Sep 2025 14:02:18 +0200 Subject: [PATCH 2/8] Revert using a tuple directly --- Python/ceval.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 76e9db0754c302..d97f3628feb16d 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -757,7 +757,7 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, return NULL; } } - PyObject *attrs = PyTuple_New(nattrs); + PyObject *attrs = PyList_New(0); if (attrs == NULL) { Py_XDECREF(seen); return NULL; @@ -800,7 +800,9 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } if (match_self) { // Easy. Copy the subject itself, and move on to kwargs. - PyTuple_SET_ITEM(attrs, 0, subject); + if (PyList_Append(attrs, subject) < 0) { + goto fail; + } } else { for (Py_ssize_t i = 0; i < nargs; i++) { @@ -816,7 +818,11 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (attr == NULL) { goto fail; } - PyTuple_SET_ITEM(attrs, i, attr); + if (PyList_Append(attrs, attr) < 0) { + Py_DECREF(attr); + goto fail; + } + Py_DECREF(attr); } } Py_CLEAR(match_args); @@ -828,8 +834,13 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (attr == NULL) { goto fail; } - PyTuple_SET_ITEM(attrs, nargs + i, attr); + if (PyList_Append(attrs, attr) < 0) { + Py_DECREF(attr); + goto fail; + } + Py_DECREF(attr); } + Py_SETREF(attrs, PyList_AsTuple(attrs)); Py_XDECREF(seen); return attrs; fail: From a2fa0a87b1b656c5afc0a01611fa419b88cbaa46 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Mon, 15 Sep 2025 14:17:40 +0200 Subject: [PATCH 3/8] Use tuple for attrs directly --- Python/ceval.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index d97f3628feb16d..ace432c2c78949 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -757,7 +757,7 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, return NULL; } } - PyObject *attrs = PyList_New(0); + PyObject *attrs = PyTuple_New(nattrs); if (attrs == NULL) { Py_XDECREF(seen); return NULL; @@ -800,9 +800,8 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } if (match_self) { // Easy. Copy the subject itself, and move on to kwargs. - if (PyList_Append(attrs, subject) < 0) { - goto fail; - } + Py_INCREF(subject); + PyTuple_SET_ITEM(attrs, 0, subject); } else { for (Py_ssize_t i = 0; i < nargs; i++) { @@ -818,11 +817,7 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (attr == NULL) { goto fail; } - if (PyList_Append(attrs, attr) < 0) { - Py_DECREF(attr); - goto fail; - } - Py_DECREF(attr); + PyTuple_SET_ITEM(attrs, i, attr); } } Py_CLEAR(match_args); @@ -834,13 +829,8 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (attr == NULL) { goto fail; } - if (PyList_Append(attrs, attr) < 0) { - Py_DECREF(attr); - goto fail; - } - Py_DECREF(attr); + PyTuple_SET_ITEM(attrs, nargs + i, attr); } - Py_SETREF(attrs, PyList_AsTuple(attrs)); Py_XDECREF(seen); return attrs; fail: From f5852537a019b225d3586ef1806effe8e03913c6 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Mon, 15 Sep 2025 23:56:42 +0200 Subject: [PATCH 4/8] Only check for duplicates if there is at least one positional pattern --- Python/ceval.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index ace432c2c78949..4a320a32f445bf 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -751,7 +751,7 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } // So far so good: PyObject *seen = NULL; - if (nattrs > 1) { + if (nargs > 0 && nattrs > 1) { seen = PySet_New(NULL); if (seen == NULL) { return NULL; From 5281c71fa2655bd95fc4a730041262695538aa63 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Mon, 15 Sep 2025 23:57:26 +0200 Subject: [PATCH 5/8] Add additional comments --- Python/ceval.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Python/ceval.c b/Python/ceval.c index 4a320a32f445bf..bcecd75abaa100 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -709,6 +709,7 @@ match_class_attr(PyThreadState *tstate, PyObject *subject, PyObject *type, PyObject *name, PyObject *seen) { assert(PyUnicode_CheckExact(name)); + // Only check for duplicates if seen is not NULL. if (seen != NULL) { assert(PySet_CheckExact(seen)); if (PySet_Contains(seen, name) || PySet_Add(seen, name)) { @@ -751,6 +752,8 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } // So far so good: PyObject *seen = NULL; + // Only check for duplicates if there is at least one positional attribute + // and two or more attributes in total. if (nargs > 0 && nattrs > 1) { seen = PySet_New(NULL); if (seen == NULL) { From b4686477ceb31972a6a53077162726a14760a09b Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Tue, 16 Sep 2025 12:57:23 +0200 Subject: [PATCH 6/8] Code review --- .../2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst | 2 +- Python/ceval.c | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst index e0008c3bbfe3e1..f5d312a289fe21 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst @@ -1 +1 @@ -Improve :opcode:`MATCH_CLASS` performance. Patch by Marc Mueller +Improve :opcode:`MATCH_CLASS` performance by up to 52% in certain cases. Patch by Marc Mueller. diff --git a/Python/ceval.c b/Python/ceval.c index bcecd75abaa100..eb618b5e4f8654 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -745,10 +745,9 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } // Short circuit if there aren't any arguments: Py_ssize_t nkwargs = PyTuple_GET_SIZE(kwargs); - Py_ssize_clean_t nattrs = nargs + nkwargs; + Py_ssize_t nattrs = nargs + nkwargs; if (!nattrs) { - PyObject *attrs = PyTuple_New(0); - return attrs; + return PyTuple_New(0); } // So far so good: PyObject *seen = NULL; @@ -803,7 +802,8 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } if (match_self) { // Easy. Copy the subject itself, and move on to kwargs. - Py_INCREF(subject); + Py_NewRef(subject); + assert(PyTuple_GET_ITEM(attrs, 0) == NULL); PyTuple_SET_ITEM(attrs, 0, subject); } else { @@ -820,6 +820,7 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (attr == NULL) { goto fail; } + assert(PyTuple_GET_ITEM(attrs, i) == NULL); PyTuple_SET_ITEM(attrs, i, attr); } } @@ -832,6 +833,7 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, if (attr == NULL) { goto fail; } + assert(PyTuple_GET_ITEM(attrs, nargs + i) == NULL); PyTuple_SET_ITEM(attrs, nargs + i, attr); } Py_XDECREF(seen); From ef84a10cad1018ffc443bc04fa8ebebed2f4fd53 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Tue, 28 Oct 2025 14:32:56 +0100 Subject: [PATCH 7/8] Code review --- Python/ceval.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index eb618b5e4f8654..a9b35e65b66a8b 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -802,9 +802,8 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, } if (match_self) { // Easy. Copy the subject itself, and move on to kwargs. - Py_NewRef(subject); assert(PyTuple_GET_ITEM(attrs, 0) == NULL); - PyTuple_SET_ITEM(attrs, 0, subject); + PyTuple_SET_ITEM(attrs, 0, Py_NewRef(subject)); } else { for (Py_ssize_t i = 0; i < nargs; i++) { From 9375e2b27e27ce6ce5ca28128b0403eb894d3553 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Sun, 21 Dec 2025 23:28:26 +0100 Subject: [PATCH 8/8] Add test case for duplicate keyword attributes --- Lib/test/test_syntax.py | 6 ++++++ Python/ceval.c | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py index 93f0b98de71d81..f245db287be769 100644 --- a/Lib/test/test_syntax.py +++ b/Lib/test/test_syntax.py @@ -2345,6 +2345,12 @@ Traceback (most recent call last): SyntaxError: positional patterns follow keyword patterns + >>> match ...: + ... case Foo(y=1, x=2, y=3): + ... ... + Traceback (most recent call last): + SyntaxError: attribute name repeated in class pattern: y + >>> match ...: ... case C(a=b, c, d=e, f, g=h, i, j=k, ...): ... ... diff --git a/Python/ceval.c b/Python/ceval.c index 51f9fcde6d062b..2623eff5a3392c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -887,7 +887,8 @@ _PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, // So far so good: PyObject *seen = NULL; // Only check for duplicates if there is at least one positional attribute - // and two or more attributes in total. + // and two or more attributes in total. Duplicate keyword attributes are + // detected during the compile stage and raise a SyntaxError. if (nargs > 0 && nattrs > 1) { seen = PySet_New(NULL); if (seen == NULL) {