From 37e67cc89910110a7087c5fe66a93af9bc336893 Mon Sep 17 00:00:00 2001 From: Yurii Karabas <1998uriyyo@gmail.com> Date: Sun, 17 Apr 2022 11:17:25 +0300 Subject: [PATCH 1/7] gh-91603: Speed up isinstance/issubclass on union types --- Include/internal/pycore_unionobject.h | 1 + ...2-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst | 2 + Objects/abstract.c | 12 +++ Objects/unionobject.c | 74 ++++--------------- 4 files changed, 28 insertions(+), 61 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst diff --git a/Include/internal/pycore_unionobject.h b/Include/internal/pycore_unionobject.h index 9962f57610387d..6f4f1be04eac0c 100644 --- a/Include/internal/pycore_unionobject.h +++ b/Include/internal/pycore_unionobject.h @@ -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 } diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst b/Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst new file mode 100644 index 00000000000000..042482ad06c37c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst @@ -0,0 +1,2 @@ +Speed-up ``isinstance`` and ``issubclass`` checks for ``types.UnionType``. +Patch provided by Yurii Karabas. diff --git a/Objects/abstract.c b/Objects/abstract.c index 79f5a5f760f8e2..14d71fb4e0bbb9 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -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. */ @@ -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__")) { diff --git a/Objects/unionobject.c b/Objects/unionobject.c index 72a0a3f2cf8d22..98daac03c95554 100644 --- a/Objects/unionobject.c +++ b/Objects/unionobject.c @@ -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) { @@ -348,12 +294,6 @@ static PyMemberDef union_members[] = { {0} }; -static PyMethodDef union_methods[] = { - {"__instancecheck__", union_instancecheck, METH_O}, - {"__subclasscheck__", union_subclasscheck, METH_O}, - {0}}; - - static PyObject * union_getitem(PyObject *self, PyObject *item) { @@ -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) +{ + 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", @@ -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, From bd2688445825cd91795ee95d986106325f77d694 Mon Sep 17 00:00:00 2001 From: Yurii Karabas <1998uriyyo@gmail.com> Date: Sun, 17 Apr 2022 18:07:57 +0300 Subject: [PATCH 2/7] Unify logic for tuple and types.UnionType checks --- Doc/library/functions.rst | 3 ++- Include/internal/pycore_unionobject.h | 2 +- Lib/test/test_types.py | 4 ++-- Objects/abstract.c | 14 ++++---------- Objects/unionobject.c | 24 ++---------------------- 5 files changed, 11 insertions(+), 36 deletions(-) diff --git a/Doc/library/functions.rst b/Doc/library/functions.rst index eaa4d482ce3fc9..4ab33945bbbb25 100644 --- a/Doc/library/functions.rst +++ b/Doc/library/functions.rst @@ -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`. diff --git a/Include/internal/pycore_unionobject.h b/Include/internal/pycore_unionobject.h index 6f4f1be04eac0c..a9ed5651a410e8 100644 --- a/Include/internal/pycore_unionobject.h +++ b/Include/internal/pycore_unionobject.h @@ -15,7 +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); +extern PyObject *_Py_union_args(PyObject *self); #ifdef __cplusplus } diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 42fd4f56235fab..3cc24af4980643 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -951,8 +951,8 @@ def __eq__(self, other): with self.assertRaises(ZeroDivisionError): list[int] | list[bt] - union_ga = (int | list[str], int | collections.abc.Callable[..., str], - int | d) + union_ga = (list[str] | int, collections.abc.Callable[..., str] | int, + d | int) # Raise error when isinstance(type, type | genericalias) for type_ in union_ga: with self.subTest(f"check isinstance/issubclass is invalid for {type_}"): diff --git a/Objects/abstract.c b/Objects/abstract.c index 14d71fb4e0bbb9..a219ee1d747f66 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -2625,11 +2625,8 @@ 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 (_PyUnion_Check(cls)) + cls = _Py_union_args(cls); if (PyTuple_Check(cls)) { /* Not a general sequence -- that opens up the road to @@ -2720,11 +2717,8 @@ 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 (_PyUnion_Check(cls)) + cls = _Py_union_args(cls); if (PyTuple_Check(cls)) { diff --git a/Objects/unionobject.c b/Objects/unionobject.c index 98daac03c95554..a1f6cb86c6cbc0 100644 --- a/Objects/unionobject.c +++ b/Objects/unionobject.c @@ -48,19 +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_richcompare(PyObject *a, PyObject *b, int op) { @@ -381,16 +368,9 @@ union_getattro(PyObject *self, PyObject *name) } PyObject * -_Py_union_args_for_check(PyObject *self, bool is_instance_check) +_Py_union_args(PyObject *self) { - 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; + return ((unionobject *) self)->args; } PyTypeObject _PyUnion_Type = { From 55af5a6adfdce1ec7f636dc5cff8567201db859b Mon Sep 17 00:00:00 2001 From: Yurii Karabas <1998uriyyo@gmail.com> Date: Sun, 17 Apr 2022 18:31:37 +0300 Subject: [PATCH 3/7] Fix test_isinstance_with_or_union --- Lib/test/test_isinstance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py index 9d37cff9903385..a0974640bc1146 100644 --- a/Lib/test/test_isinstance.py +++ b/Lib/test/test_isinstance.py @@ -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) From cbd44cc14e96d33868c38a676e0ce08395f03614 Mon Sep 17 00:00:00 2001 From: Yurii Karabas <1998uriyyo@gmail.com> Date: Sun, 17 Apr 2022 23:40:37 +0300 Subject: [PATCH 4/7] Update Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst Co-authored-by: Jelle Zijlstra --- .../2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst b/Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst index 042482ad06c37c..957bd5ea09b58a 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst @@ -1,2 +1,2 @@ -Speed-up ``isinstance`` and ``issubclass`` checks for ``types.UnionType``. -Patch provided by Yurii Karabas. +Speed up :func:`isinstance` and :func:`issubclass` checks for :class:`types.UnionType`. +Patch by Yurii Karabas. From cc1cb4095947aad0afe78c8e8abb3c9ae6543c32 Mon Sep 17 00:00:00 2001 From: Yurii Karabas <1998uriyyo@gmail.com> Date: Tue, 19 Apr 2022 10:27:34 +0300 Subject: [PATCH 5/7] Update Lib/test/test_types.py Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com> --- Lib/test/test_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 3cc24af4980643..cde9dadc5e97fa 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -953,7 +953,7 @@ def __eq__(self, other): union_ga = (list[str] | int, collections.abc.Callable[..., str] | int, d | int) - # Raise error when isinstance(type, type | genericalias) + # 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): From 1514cdfeb6efff6f205a47bdd3f0ab94a066387a Mon Sep 17 00:00:00 2001 From: Yurii Karabas <1998uriyyo@gmail.com> Date: Mon, 25 Apr 2022 10:02:24 +0300 Subject: [PATCH 6/7] Update Objects/unionobject.c Co-authored-by: Jelle Zijlstra --- Objects/unionobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/unionobject.c b/Objects/unionobject.c index a1f6cb86c6cbc0..88eb6e36f08875 100644 --- a/Objects/unionobject.c +++ b/Objects/unionobject.c @@ -370,6 +370,7 @@ union_getattro(PyObject *self, PyObject *name) PyObject * _Py_union_args(PyObject *self) { + assert(_PyUnion_Check(self)); return ((unionobject *) self)->args; } From 20c44f81686683abd08b582fecdaf0f596c79397 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 25 Apr 2022 06:25:18 -0700 Subject: [PATCH 7/7] Add braces --- Objects/abstract.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Objects/abstract.c b/Objects/abstract.c index a219ee1d747f66..cfb0edcab0e5d3 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -2625,8 +2625,9 @@ object_recursive_isinstance(PyThreadState *tstate, PyObject *inst, PyObject *cls return object_isinstance(inst, cls); } - if (_PyUnion_Check(cls)) + if (_PyUnion_Check(cls)) { cls = _Py_union_args(cls); + } if (PyTuple_Check(cls)) { /* Not a general sequence -- that opens up the road to @@ -2717,8 +2718,9 @@ object_issubclass(PyThreadState *tstate, PyObject *derived, PyObject *cls) return recursive_issubclass(derived, cls); } - if (_PyUnion_Check(cls)) + if (_PyUnion_Check(cls)) { cls = _Py_union_args(cls); + } if (PyTuple_Check(cls)) {