Skip to content

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

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

Merged
merged 7 commits into from
Apr 28, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
37e67cc
gh-91603: Speed up isinstance/issubclass on union types
uriyyo Apr 17, 2022
bd26884
Unify logic for tuple and types.UnionType checks
uriyyo Apr 17, 2022
55af5a6
Fix test_isinstance_with_or_union
uriyyo Apr 17, 2022
cbd44cc
Update Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issu…
uriyyo Apr 17, 2022
cc1cb40
Update Lib/test/test_types.py
uriyyo Apr 19, 2022
1514cdf
Update Objects/unionobject.c
uriyyo Apr 25, 2022
20c44f8
Add braces
JelleZijlstra Apr 25, 2022
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
3 changes: 2 additions & 1 deletion Doc/library/functions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,8 @@ are always available. They are listed here in alphabetical order.
tuples) or a :ref:`types-union` of multiple types, return ``True`` if
*object* is an instance of any of the types.
If *classinfo* is not a type or tuple of types and such tuples,
a :exc:`TypeError` exception is raised.
a :exc:`TypeError` exception is raised. :exc:`TypeError` may not be
raised for an invalid type if an earlier check succeeds.

.. versionchanged:: 3.10
*classinfo* can be a :ref:`types-union`.
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_unionobject.h
View file
Open in desktop
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(PyObject *self);

#ifdef __cplusplus
}
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_isinstance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def test_isinstance_with_or_union(self):
with self.assertRaises(TypeError):
isinstance(2, list[int] | int)
with self.assertRaises(TypeError):
isinstance(2, int | str | list[int] | float)
isinstance(2, float | str | list[int] | int)



Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,9 +951,9 @@ def __eq__(self, other):
with self.assertRaises(ZeroDivisionError):
list[int] | list[bt]

union_ga = (int | list[str], int | collections.abc.Callable[..., str],
int | d)
# Raise error when isinstance(type, type | genericalias)
union_ga = (list[str] | int, collections.abc.Callable[..., str] | int,
d | int)
# Raise error when isinstance(type, genericalias | type)
for type_ in union_ga:
with self.subTest(f"check isinstance/issubclass is invalid for {type_}"):
with self.assertRaises(TypeError):
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Speed up :func:`isinstance` and :func:`issubclass` checks for :class:`types.UnionType`.
Patch by Yurii Karabas.
8 changes: 8 additions & 0 deletions Objects/abstract.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2625,6 +2625,10 @@ object_recursive_isinstance(PyThreadState *tstate, PyObject *inst, PyObject *cls
return object_isinstance(inst, cls);
}

if (_PyUnion_Check(cls)) {
cls = _Py_union_args(cls);
}

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

if (_PyUnion_Check(cls)) {
cls = _Py_union_args(cls);
}

if (PyTuple_Check(cls)) {

if (_Py_EnterRecursiveCall(tstate, " in __subclasscheck__")) {
Expand Down
81 changes: 7 additions & 74 deletions Objects/unionobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,73 +48,6 @@ union_hash(PyObject *self)
return hash;
}

static int
is_generic_alias_in_args(PyObject *args)
{
Py_ssize_t nargs = PyTuple_GET_SIZE(args);
for (Py_ssize_t iarg = 0; iarg < nargs; iarg++) {
PyObject *arg = PyTuple_GET_ITEM(args, iarg);
if (_PyGenericAlias_Check(arg)) {
return 0;
}
}
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 +281,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 +367,13 @@ union_getattro(PyObject *self, PyObject *name)
return PyObject_GenericGetAttr(self, name);
}

PyObject *
_Py_union_args(PyObject *self)
{
assert(_PyUnion_Check(self));
return ((unionobject *) self)->args;
}

PyTypeObject _PyUnion_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
.tp_name = "types.UnionType",
Expand All @@ -455,7 +389,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