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 6 commits
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
3 changes: 2 additions & 1 deletion Doc/library/functions.rst
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
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
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
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)
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
for type_ in union_ga:
with self.subTest(f"check isinstance/issubclass is invalid for {type_}"):
with self.assertRaises(TypeError):
Expand Down
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.
6 changes: 6 additions & 0 deletions Objects/abstract.c
Original file line number Diff line number Diff line change
Expand Up @@ -2625,6 +2625,9 @@ object_recursive_isinstance(PyThreadState *tstate, PyObject *inst, PyObject *cls
return object_isinstance(inst, cls);
}

if (_PyUnion_Check(cls))
cls = _Py_union_args(cls);
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved

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

if (_PyUnion_Check(cls))
cls = _Py_union_args(cls);
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved

if (PyTuple_Check(cls)) {

if (_Py_EnterRecursiveCall(tstate, " in __subclasscheck__")) {
Expand Down
81 changes: 7 additions & 74 deletions Objects/unionobject.c
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)
{
uriyyo marked this conversation as resolved.
Show resolved Hide resolved
assert(_PyUnion_Check(self));
return ((unionobject *) self)->args;
uriyyo marked this conversation as resolved.
Show resolved Hide resolved
}

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