Skip to content

gh-138912: Improve MATCH_CLASS opcode performance #138915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

gh-138912: Improve MATCH_CLASS opcode performance #138915

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7c81286
gh-138912: Improve MATCH_CLASS opcode performance
cdce8p Sep 15, 2025
597adb0
Revert using a tuple directly
cdce8p Sep 15, 2025
a2fa0a8
Use tuple for attrs directly
cdce8p Sep 15, 2025
f585253
Only check for duplicates if there is at least one positional pattern
cdce8p Sep 15, 2025
5281c71
Add additional comments
cdce8p Sep 15, 2025
b468647
Code review
cdce8p Sep 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-09-15-13-28-48.gh-issue-138912.61EYbn.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve :opcode:`MATCH_CLASS` performance. Patch by Marc Mueller
62 changes: 32 additions & 30 deletions Python/ceval.c
View file
Open in desktop
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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For what do you need this change? Maybe I'm missing, but other changes don't require this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ough, I see. At least this needs a comment, IMHO.

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;
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) {
Copy link
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
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.

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);
}
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);
}
}
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;
}
Expand Down
Loading