Skip to content
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.

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

gh-91603: Speed up isinstance/issubclass on union types #91631

Merged
merged 7 commits into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 Include/internal/pycore_unionobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ extern PyObject *_Py_union_type_or(PyObject *, PyObject *);
#define _PyGenericAlias_Check(op) PyObject_TypeCheck(op, &Py_GenericAliasType)
extern PyObject *_Py_subs_parameters(PyObject *, PyObject *, PyObject *, PyObject *);
extern PyObject *_Py_make_parameters(PyObject *);
extern PyObject *_Py_union_args_for_check(PyObject *, bool);

#ifdef __cplusplus
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Speed-up ``isinstance`` and ``issubclass`` checks for ``types.UnionType``.
Patch provided by Yurii Karabas.
uriyyo marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 12 additions & 0 deletions Objects/abstract.c
Original file line number Diff line number Diff line change
Expand Up @@ -2625,6 +2625,12 @@ object_recursive_isinstance(PyThreadState *tstate, PyObject *inst, PyObject *cls
return object_isinstance(inst, cls);
}

if (_PyUnion_Check(cls)) {
cls = _Py_union_args_for_check(cls, true);
if (!cls)
return -1;
}

if (PyTuple_Check(cls)) {
/* Not a general sequence -- that opens up the road to
recursion and stack overflow. */
Expand Down Expand Up @@ -2714,6 +2720,12 @@ object_issubclass(PyThreadState *tstate, PyObject *derived, PyObject *cls)
return recursive_issubclass(derived, cls);
}

if (_PyUnion_Check(cls)) {
cls = _Py_union_args_for_check(cls, false);
if (!cls)
return -1;
}

if (PyTuple_Check(cls)) {

if (_Py_EnterRecursiveCall(tstate, " in __subclasscheck__")) {
Expand Down
74 changes: 13 additions & 61 deletions Objects/unionobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,60 +61,6 @@ is_generic_alias_in_args(PyObject *args)
return 1;
}

static PyObject *
union_instancecheck(PyObject *self, PyObject *instance)
{
unionobject *alias = (unionobject *) self;
Py_ssize_t nargs = PyTuple_GET_SIZE(alias->args);
if (!is_generic_alias_in_args(alias->args)) {
PyErr_SetString(PyExc_TypeError,
"isinstance() argument 2 cannot contain a parameterized generic");
return NULL;
}
for (Py_ssize_t iarg = 0; iarg < nargs; iarg++) {
PyObject *arg = PyTuple_GET_ITEM(alias->args, iarg);
if (PyType_Check(arg)) {
int res = PyObject_IsInstance(instance, arg);
if (res < 0) {
return NULL;
}
if (res) {
Py_RETURN_TRUE;
}
}
}
Py_RETURN_FALSE;
}

static PyObject *
union_subclasscheck(PyObject *self, PyObject *instance)
{
if (!PyType_Check(instance)) {
PyErr_SetString(PyExc_TypeError, "issubclass() arg 1 must be a class");
return NULL;
}
unionobject *alias = (unionobject *)self;
if (!is_generic_alias_in_args(alias->args)) {
PyErr_SetString(PyExc_TypeError,
"issubclass() argument 2 cannot contain a parameterized generic");
return NULL;
}
Py_ssize_t nargs = PyTuple_GET_SIZE(alias->args);
for (Py_ssize_t iarg = 0; iarg < nargs; iarg++) {
PyObject *arg = PyTuple_GET_ITEM(alias->args, iarg);
if (PyType_Check(arg)) {
int res = PyObject_IsSubclass(instance, arg);
if (res < 0) {
return NULL;
}
if (res) {
Py_RETURN_TRUE;
}
}
}
Py_RETURN_FALSE;
}

static PyObject *
union_richcompare(PyObject *a, PyObject *b, int op)
{
Expand Down Expand Up @@ -348,12 +294,6 @@ static PyMemberDef union_members[] = {
{0}
};

static PyMethodDef union_methods[] = {
{"__instancecheck__", union_instancecheck, METH_O},
Copy link
Member

Choose a reason for hiding this comment

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

I think we may still want to keep those methods too in case people call them directly. Let me think about it some more.

Copy link
Member

Choose a reason for hiding this comment

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

On second thought, I don't think we need to keep these. The language reference (https://docs.python.org/3/library/stdtypes.html#types-union) explicitly says that isinstance() works with union objects, so we don't need __instancecheck__ to be present to indicate that they work with isinstance().

So no change needed here.

{"__subclasscheck__", union_subclasscheck, METH_O},
{0}};


static PyObject *
union_getitem(PyObject *self, PyObject *item)
{
Expand Down Expand Up @@ -440,6 +380,19 @@ union_getattro(PyObject *self, PyObject *name)
return PyObject_GenericGetAttr(self, name);
}

PyObject *
_Py_union_args_for_check(PyObject *self, bool is_instance_check)
{
uriyyo marked this conversation as resolved.
Show resolved Hide resolved
PyObject *args = ((unionobject *) self)->args;
if (!is_generic_alias_in_args(args)) {
PyErr_Format(PyExc_TypeError,
"%s() argument 2 cannot contain a parameterized generic",
is_instance_check ? "isinstance" : "issubclass");
return NULL;
}
return args;
}

PyTypeObject _PyUnion_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
.tp_name = "types.UnionType",
Expand All @@ -455,7 +408,6 @@ PyTypeObject _PyUnion_Type = {
.tp_hash = union_hash,
.tp_getattro = union_getattro,
.tp_members = union_members,
.tp_methods = union_methods,
.tp_richcompare = union_richcompare,
.tp_as_mapping = &union_as_mapping,
.tp_as_number = &union_as_number,
Expand Down