Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve :opcode:`MATCH_CLASS` performance. Patch by Marc Mueller
Comment thread
cdce8p marked this conversation as resolved.
Outdated
62 changes: 32 additions & 30 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
cdce8p marked this conversation as resolved.
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);
Expand All @@ -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;
Comment thread
cdce8p marked this conversation as resolved.
Outdated
if (!nattrs) {
PyObject *attrs = PyTuple_New(0);
return attrs;
Comment thread
cdce8p marked this conversation as resolved.
Outdated
}
// So far so good:
PyObject *seen = PySet_New(NULL);
if (seen == NULL) {
return NULL;
PyObject *seen = NULL;
if (nattrs > 1) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
if (nattrs > 1) {
if (nargs > 1 || (nargs == 1 && nkwargs > 0)) {

As mentioned in #138912, I'd also like to change this. If there are only keyword arguments, it's easy to see if two are the same name. Furthermore linters are able to detect these based on the AST alone.

It would also be great to say if we could suggest users to prefer keyword arguments. Not only are they more explicit, they are also faster. (No lookup of __match_args__ and, with the change, no expensive duplicate name detection.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added this in f585253 with if (nargs > 0 && nattrs > 1). I can revert it if necessary.

Copy link
Copy Markdown
Contributor Author

@cdce8p cdce8p Dec 21, 2025

Choose a reason for hiding this comment

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

Only mentioned it on the issue a while ago but the keyword attributes are validated already during the compile stage at which point a SyntaxError is raised. Just added an explicit test case for it in 9375e2b.

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:
Expand Down Expand Up @@ -788,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);
Comment thread
cdce8p marked this conversation as resolved.
Outdated
}
else {
for (Py_ssize_t i = 0; i < nargs; i++) {
Expand All @@ -806,36 +817,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);
Comment thread
cdce8p marked this conversation as resolved.
}
}
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);
Comment thread
cdce8p marked this conversation as resolved.
}
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;
}
Expand Down
Loading