From 0a865c9309c6548fe6380500f02240d1995d3b16 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 4 Sep 2017 13:27:09 +0200 Subject: [PATCH 01/22] Speed up type creation, which is highly dominated by slow dict lookups. --- Objects/typeobject.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index d1a12a7efac2c86..8069ebaae9ddb04 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2945,6 +2945,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) { Py_ssize_t i, n; PyObject *mro, *res, *base, *dict; + Py_hash_t hash; unsigned int h; if (MCACHE_CACHEABLE_NAME(name) && @@ -2983,6 +2984,21 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) } } + if (!PyUnicode_CheckExact(name) || + (hash = ((PyASCIIObject *) name)->hash) == -1) + { + hash = PyObject_Hash(name); + if (hash == -1) { + /* Same comment as above applies: this should not ignore the + error. But it's highly unlikely that we even get here since + 'name' is bound to be a PyUnicode object and almost certainly + not a subtype. + */ + PyErr_Clear(); + return NULL; + } + } + res = NULL; /* keep a strong reference to mro because type->tp_mro can be replaced during PyDict_GetItem(dict, name) */ @@ -2994,7 +3010,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) assert(PyType_Check(base)); dict = ((PyTypeObject *)base)->tp_dict; assert(dict && PyDict_Check(dict)); - res = PyDict_GetItem(dict, name); + res = _PyDict_GetItem_KnownHash(dict, name, hash); if (res != NULL) break; } From c4779e3f7c06d1e53386f18e6bad3b045741369d Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 4 Sep 2017 15:14:19 +0200 Subject: [PATCH 02/22] Update comment after changing the function call that it refers to. --- Objects/typeobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 8069ebaae9ddb04..af5cba121f45525 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3000,8 +3000,8 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) } res = NULL; - /* keep a strong reference to mro because type->tp_mro can be replaced - during PyDict_GetItem(dict, name) */ + /* Keep a strong reference to mro because type->tp_mro can be replaced + during dict lookup, e.g. when comparing to non-string keys. */ Py_INCREF(mro); assert(PyTuple_Check(mro)); n = PyTuple_GET_SIZE(mro); From ff2ea631ebc3b64f8785cfdbaa922cd3546bd1b1 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 4 Sep 2017 15:33:33 +0200 Subject: [PATCH 03/22] Ignore any errors (however unlikely) that may happen during the mro chained name lookups in _PyType_Lookup(). --- Objects/typeobject.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index af5cba121f45525..99eefbf3ab10240 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3013,6 +3013,9 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) res = _PyDict_GetItem_KnownHash(dict, name, hash); if (res != NULL) break; + /* Ignore any errors during lookup - unlikely to happen, + but not impossible. */ + PyErr_Clear(); } Py_DECREF(mro); From 76b1986e093e9b236106df3abb7dfcd6f60e43ad Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Tue, 5 Sep 2017 19:07:31 +0200 Subject: [PATCH 04/22] Extract non-caching code from _PyType_Lookup() to use it directly from update_one_slot(). --- Objects/typeobject.c | 95 ++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 99eefbf3ab10240..84ed24adfb5d557 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2938,44 +2938,21 @@ PyType_GetSlot(PyTypeObject *type, int slot) return *(void**)(((char*)type) + slotoffsets[slot]); } -/* Internal API to look for a name through the MRO. - This returns a borrowed reference, and doesn't set an exception! */ -PyObject * -_PyType_Lookup(PyTypeObject *type, PyObject *name) +/* Internal API to look for a name through the MRO, bypassing the method cache. + This returns a borrowed reference, and might set an exception! */ +static PyObject * +_PyType_LookupUncached(PyTypeObject *type, PyObject *name, int *error) { Py_ssize_t i, n; PyObject *mro, *res, *base, *dict; Py_hash_t hash; - unsigned int h; - - if (MCACHE_CACHEABLE_NAME(name) && - PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) { - /* fast path */ - h = MCACHE_HASH_METHOD(type, name); - if (method_cache[h].version == type->tp_version_tag && - method_cache[h].name == name) { -#if MCACHE_STATS - method_cache_hits++; -#endif - return method_cache[h].value; - } - } - + *error = 1; /* Look in tp_dict of types in MRO */ mro = type->tp_mro; if (mro == NULL) { if ((type->tp_flags & Py_TPFLAGS_READYING) == 0 && - PyType_Ready(type) < 0) { - /* It's not ideal to clear the error condition, - but this function is documented as not setting - an exception, and I don't want to change that. - When PyType_Ready() can't proceed, it won't - set the "ready" flag, so future attempts to ready - the same type will call it again -- hopefully - in a context that propagates the exception out. - */ - PyErr_Clear(); + PyType_Ready(type) < 0) { return NULL; } mro = type->tp_mro; @@ -2989,12 +2966,6 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) { hash = PyObject_Hash(name); if (hash == -1) { - /* Same comment as above applies: this should not ignore the - error. But it's highly unlikely that we even get here since - 'name' is bound to be a PyUnicode object and almost certainly - not a subtype. - */ - PyErr_Clear(); return NULL; } } @@ -3013,11 +2984,50 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) res = _PyDict_GetItem_KnownHash(dict, name, hash); if (res != NULL) break; - /* Ignore any errors during lookup - unlikely to happen, - but not impossible. */ - PyErr_Clear(); + if (PyErr_Occurred()) + return NULL; } Py_DECREF(mro); + *error = 0; + return res; +} + +/* Internal API to look for a name through the MRO. + This returns a borrowed reference, and doesn't set an exception! */ +PyObject * +_PyType_Lookup(PyTypeObject *type, PyObject *name) +{ + PyObject *res; + int error; + unsigned int h; + + if (MCACHE_CACHEABLE_NAME(name) && + PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) { + /* fast path */ + h = MCACHE_HASH_METHOD(type, name); + if (method_cache[h].version == type->tp_version_tag && + method_cache[h].name == name) { +#if MCACHE_STATS + method_cache_hits++; +#endif + return method_cache[h].value; + } + } + + res = _PyType_LookupUncached(type, name, &error); + /* Only put NULL results into cache if there was no error. */ + if (error) { + /* It's not ideal to clear the error condition, + but this function is documented as not setting + an exception, and I don't want to change that. + E.g., when PyType_Ready() can't proceed, it won't + set the "ready" flag, so future attempts to ready + the same type will call it again -- hopefully + in a context that propagates the exception out. + */ + PyErr_Clear(); + return NULL; + } if (MCACHE_CACHEABLE_NAME(name) && assign_version_tag(type)) { h = MCACHE_HASH_METHOD(type, name); @@ -6968,6 +6978,7 @@ update_one_slot(PyTypeObject *type, slotdef *p) void *generic = NULL, *specific = NULL; int use_generic = 0; int offset = p->offset; + int error; void **ptr = slotptr(type, offset); if (ptr == NULL) { @@ -6977,8 +6988,14 @@ update_one_slot(PyTypeObject *type, slotdef *p) return p; } do { - descr = _PyType_Lookup(type, p->name_strobj); + descr = _PyType_LookupUncached(type, p->name_strobj, &error); if (descr == NULL) { + if (error) { + /* It is unlikely by not impossible that there has been an exception + during lookup. Since this function originally expected no errors, + we ignore them here in order to keep up the interface. */ + PyErr_Clear(); + } if (ptr == (void**)&type->tp_iternext) { specific = (void *)_PyObject_NextNotImplemented; } From 6f9848520ca858ae72d14ee99d0d86796b664981 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Tue, 5 Sep 2017 19:39:49 +0200 Subject: [PATCH 05/22] Avoid some useless overhead for the no-basetype case. If "object" ever ceases to be a valid base type, there'll probably be larger code sections to change than this one. --- Objects/typeobject.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 84ed24adfb5d557..7563523e18b64b8 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2375,18 +2375,20 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds) /* Adjust for empty tuple bases */ nbases = PyTuple_GET_SIZE(bases); if (nbases == 0) { - bases = PyTuple_Pack(1, &PyBaseObject_Type); + base = &PyBaseObject_Type; + bases = PyTuple_Pack(1, base); if (bases == NULL) goto error; nbases = 1; } - else + else { Py_INCREF(bases); - /* Calculate best base, and check that all bases are type objects */ - base = best_base(bases); - if (base == NULL) { - goto error; + /* Calculate best base, and check that all bases are type objects */ + base = best_base(bases); + if (base == NULL) { + goto error; + } } dict = PyDict_Copy(orig_dict); From d9b726e6ddaae31b01b41c33572e4f6f3fa1a63b Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Wed, 6 Sep 2017 07:19:09 +0200 Subject: [PATCH 06/22] Add comment. --- Objects/typeobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7563523e18b64b8..045b595e30a0aaf 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6990,6 +6990,7 @@ update_one_slot(PyTypeObject *type, slotdef *p) return p; } do { + /* Use faster uncached lookup as we won't get any cache hits during type setup. */ descr = _PyType_LookupUncached(type, p->name_strobj, &error); if (descr == NULL) { if (error) { From be15255ff763ce1018a7318590dc37a4c1e04a00 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Wed, 6 Sep 2017 10:18:04 +0200 Subject: [PATCH 07/22] Avoid uselessly searching empty bases for a metaclass. This is quite common in Python 3 now. Also make the step from "return NULL" error handling to "goto error" reference cleanup explicit. --- Objects/typeobject.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 045b595e30a0aaf..a5f0eee945a1a01 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2360,37 +2360,39 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds) &bases, &PyDict_Type, &orig_dict)) return NULL; - /* Determine the proper metatype to deal with this: */ - winner = _PyType_CalculateMetaclass(metatype, bases); - if (winner == NULL) { - return NULL; - } - - if (winner != metatype) { - if (winner->tp_new != type_new) /* Pass it to the winner */ - return winner->tp_new(winner, args, kwds); - metatype = winner; - } - /* Adjust for empty tuple bases */ nbases = PyTuple_GET_SIZE(bases); if (nbases == 0) { base = &PyBaseObject_Type; bases = PyTuple_Pack(1, base); if (bases == NULL) - goto error; + return NULL; nbases = 1; } else { - Py_INCREF(bases); + /* Search the bases for the proper metatype to deal with this: */ + winner = _PyType_CalculateMetaclass(metatype, bases); + if (winner == NULL) { + return NULL; + } + + if (winner != metatype) { + if (winner->tp_new != type_new) /* Pass it to the winner */ + return winner->tp_new(winner, args, kwds); + metatype = winner; + } /* Calculate best base, and check that all bases are type objects */ base = best_base(bases); if (base == NULL) { - goto error; + return NULL; } + + Py_INCREF(bases); } + /* Use "goto error" from this point on as we now own the reference to "bases". */ + dict = PyDict_Copy(orig_dict); if (dict == NULL) goto error; From 0736226d67b43c70013a397be7157a7317b42057 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Sun, 10 Sep 2017 21:57:20 +0200 Subject: [PATCH 08/22] Avoid unsafe handling of borrowed "mro" reference during hash() call. Distinguish between "error" and "error with exception" cases in _PyType_LookupUncached(). Fix a reference leak of "mro" on lookup errors. Resolves issues found by Serhiy Storchaka. --- Objects/typeobject.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a5f0eee945a1a01..0ef076828b53a77 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2943,33 +2943,37 @@ PyType_GetSlot(PyTypeObject *type, int slot) } /* Internal API to look for a name through the MRO, bypassing the method cache. - This returns a borrowed reference, and might set an exception! */ + This returns a borrowed reference, and might set an exception. + 'error' is set to: -1: error with exception; 1: error without exception; 0: ok */ static PyObject * _PyType_LookupUncached(PyTypeObject *type, PyObject *name, int *error) { Py_ssize_t i, n; PyObject *mro, *res, *base, *dict; Py_hash_t hash; - *error = 1; + + if (!PyUnicode_CheckExact(name) || + (hash = ((PyASCIIObject *) name)->hash) == -1) + { + hash = PyObject_Hash(name); + if (hash == -1) { + *error = -1; + return NULL; + } + } + /* Look in tp_dict of types in MRO */ mro = type->tp_mro; if (mro == NULL) { if ((type->tp_flags & Py_TPFLAGS_READYING) == 0 && PyType_Ready(type) < 0) { + *error = -1; return NULL; } mro = type->tp_mro; if (mro == NULL) { - return NULL; - } - } - - if (!PyUnicode_CheckExact(name) || - (hash = ((PyASCIIObject *) name)->hash) == -1) - { - hash = PyObject_Hash(name); - if (hash == -1) { + *error = 1; return NULL; } } @@ -2988,11 +2992,14 @@ _PyType_LookupUncached(PyTypeObject *type, PyObject *name, int *error) res = _PyDict_GetItem_KnownHash(dict, name, hash); if (res != NULL) break; - if (PyErr_Occurred()) - return NULL; + if (PyErr_Occurred()) { + *error = -1; + goto done; + } } - Py_DECREF(mro); *error = 0; +done: + Py_DECREF(mro); return res; } @@ -3029,7 +3036,8 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) the same type will call it again -- hopefully in a context that propagates the exception out. */ - PyErr_Clear(); + if (error == -1) + PyErr_Clear(); return NULL; } @@ -6995,7 +7003,7 @@ update_one_slot(PyTypeObject *type, slotdef *p) /* Use faster uncached lookup as we won't get any cache hits during type setup. */ descr = _PyType_LookupUncached(type, p->name_strobj, &error); if (descr == NULL) { - if (error) { + if (error == -1) { /* It is unlikely by not impossible that there has been an exception during lookup. Since this function originally expected no errors, we ignore them here in order to keep up the interface. */ From 8bc783f07e60c39afa464029b8d58443a0140137 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Sun, 10 Sep 2017 22:44:09 +0200 Subject: [PATCH 09/22] Clean up code and formatting a little. --- Objects/typeobject.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 0ef076828b53a77..4335bd90660ac71 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2966,12 +2966,13 @@ _PyType_LookupUncached(PyTypeObject *type, PyObject *name, int *error) mro = type->tp_mro; if (mro == NULL) { - if ((type->tp_flags & Py_TPFLAGS_READYING) == 0 && - PyType_Ready(type) < 0) { - *error = -1; - return NULL; + if ((type->tp_flags & Py_TPFLAGS_READYING) == 0) { + if (PyType_Ready(type) < 0) { + *error = -1; + return NULL; + } + mro = type->tp_mro; } - mro = type->tp_mro; if (mro == NULL) { *error = 1; return NULL; From 66648dd568cfefdf1f003bce8fc2aced4ec6972b Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Sun, 10 Sep 2017 22:44:57 +0200 Subject: [PATCH 10/22] Add braces for code style reasons. --- Objects/typeobject.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 4335bd90660ac71..8e64d6082bcb609 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3037,8 +3037,9 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) the same type will call it again -- hopefully in a context that propagates the exception out. */ - if (error == -1) + if (error == -1) { PyErr_Clear(); + } return NULL; } From 85fb3aedd4a878cbd18145c92b32a03c7fdcbfeb Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Sun, 10 Sep 2017 22:53:22 +0200 Subject: [PATCH 11/22] Give internal helper function a local name that does not resemble (existing) API names. Suggested by Serhiy Storchaka. --- Objects/typeobject.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 8e64d6082bcb609..17713a0750d7d4f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2946,7 +2946,7 @@ PyType_GetSlot(PyTypeObject *type, int slot) This returns a borrowed reference, and might set an exception. 'error' is set to: -1: error with exception; 1: error without exception; 0: ok */ static PyObject * -_PyType_LookupUncached(PyTypeObject *type, PyObject *name, int *error) +find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) { Py_ssize_t i, n; PyObject *mro, *res, *base, *dict; @@ -3026,7 +3026,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) } } - res = _PyType_LookupUncached(type, name, &error); + res = find_name_in_mro(type, name, &error); /* Only put NULL results into cache if there was no error. */ if (error) { /* It's not ideal to clear the error condition, @@ -7003,7 +7003,7 @@ update_one_slot(PyTypeObject *type, slotdef *p) } do { /* Use faster uncached lookup as we won't get any cache hits during type setup. */ - descr = _PyType_LookupUncached(type, p->name_strobj, &error); + descr = find_name_in_mro(type, p->name_strobj, &error); if (descr == NULL) { if (error == -1) { /* It is unlikely by not impossible that there has been an exception From 09e716a2e8eda777400da779c5a4f1b5e90375f6 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 11 Sep 2017 17:51:46 +0200 Subject: [PATCH 12/22] Allow non-dict types for the class dict when looking up names and call PyObject_GetItem() for them. --- Objects/typeobject.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 17713a0750d7d4f..87c48785a3acc7f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2989,8 +2989,12 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) base = PyTuple_GET_ITEM(mro, i); assert(PyType_Check(base)); dict = ((PyTypeObject *)base)->tp_dict; - assert(dict && PyDict_Check(dict)); - res = _PyDict_GetItem_KnownHash(dict, name, hash); + assert(dict); + if (PyDict_CheckExact(dict)) { + res = _PyDict_GetItem_KnownHash(dict, name, hash); + } else { + res = PyObject_GetItem(dict, name); + } if (res != NULL) break; if (PyErr_Occurred()) { From 1d52082bb544de02901f922242942253497cba58 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 11 Sep 2017 18:12:27 +0200 Subject: [PATCH 13/22] Lazily calculate name hash in find_name_in_mro() to avoid potential re-calculation if the mapping is not exactly a dict. --- Objects/typeobject.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 87c48785a3acc7f..374b2b2baf75e83 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2950,17 +2950,7 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) { Py_ssize_t i, n; PyObject *mro, *res, *base, *dict; - Py_hash_t hash; - - if (!PyUnicode_CheckExact(name) || - (hash = ((PyASCIIObject *) name)->hash) == -1) - { - hash = PyObject_Hash(name); - if (hash == -1) { - *error = -1; - return NULL; - } - } + Py_hash_t hash = -1; /* Look in tp_dict of types in MRO */ mro = type->tp_mro; @@ -2990,9 +2980,22 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) assert(PyType_Check(base)); dict = ((PyTypeObject *)base)->tp_dict; assert(dict); + /* Optimise for the extremely common case: dict for lookup, unicode name. */ if (PyDict_CheckExact(dict)) { + if (hash == -1) { + if (!PyUnicode_CheckExact(name) || + (hash = ((PyASCIIObject *) name)->hash) == -1) + { + hash = PyObject_Hash(name); + if (hash == -1) { + *error = -1; + goto done; + } + } + } res = _PyDict_GetItem_KnownHash(dict, name, hash); } else { + /* Every other combination is much less safe, so be conservative. */ res = PyObject_GetItem(dict, name); } if (res != NULL) From f09bfd3fe764ae5cfb18c2a8b334964c76566c91 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 11 Sep 2017 20:44:57 +0200 Subject: [PATCH 14/22] Revert "Lazily calculate name hash in find_name_in_mro() to avoid potential re-calculation if the mapping is not exactly a dict." This reverts commit 1d52082bb544de02901f922242942253497cba58. --- Objects/typeobject.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 374b2b2baf75e83..87c48785a3acc7f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2950,7 +2950,17 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) { Py_ssize_t i, n; PyObject *mro, *res, *base, *dict; - Py_hash_t hash = -1; + Py_hash_t hash; + + if (!PyUnicode_CheckExact(name) || + (hash = ((PyASCIIObject *) name)->hash) == -1) + { + hash = PyObject_Hash(name); + if (hash == -1) { + *error = -1; + return NULL; + } + } /* Look in tp_dict of types in MRO */ mro = type->tp_mro; @@ -2980,22 +2990,9 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) assert(PyType_Check(base)); dict = ((PyTypeObject *)base)->tp_dict; assert(dict); - /* Optimise for the extremely common case: dict for lookup, unicode name. */ if (PyDict_CheckExact(dict)) { - if (hash == -1) { - if (!PyUnicode_CheckExact(name) || - (hash = ((PyASCIIObject *) name)->hash) == -1) - { - hash = PyObject_Hash(name); - if (hash == -1) { - *error = -1; - goto done; - } - } - } res = _PyDict_GetItem_KnownHash(dict, name, hash); } else { - /* Every other combination is much less safe, so be conservative. */ res = PyObject_GetItem(dict, name); } if (res != NULL) From 824d7cb25cd25368e9d04c5be5f75c8c790d8447 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Mon, 11 Sep 2017 20:45:05 +0200 Subject: [PATCH 15/22] Revert "Allow non-dict types for the class dict when looking up names and call PyObject_GetItem() for them." This reverts commit 09e716a2e8eda777400da779c5a4f1b5e90375f6. --- Objects/typeobject.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 87c48785a3acc7f..17713a0750d7d4f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2989,12 +2989,8 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) base = PyTuple_GET_ITEM(mro, i); assert(PyType_Check(base)); dict = ((PyTypeObject *)base)->tp_dict; - assert(dict); - if (PyDict_CheckExact(dict)) { - res = _PyDict_GetItem_KnownHash(dict, name, hash); - } else { - res = PyObject_GetItem(dict, name); - } + assert(dict && PyDict_Check(dict)); + res = _PyDict_GetItem_KnownHash(dict, name, hash); if (res != NULL) break; if (PyErr_Occurred()) { From 51841915d6f5f87c7f3559c7c83d0b2ce8ffebff Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Wed, 13 Sep 2017 12:42:14 +0200 Subject: [PATCH 16/22] add news entry for faster class creation --- .../Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst new file mode 100644 index 000000000000000..cd2f2b6cfb51b2f --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst @@ -0,0 +1,2 @@ +Speed up class creation by reducing overhead in the necessary special method +lookups. Patch by Stefan Behnel. From 4efde8ea72195e987a7946985b22725b1d8f4e3a Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Wed, 13 Sep 2017 15:04:43 +0200 Subject: [PATCH 17/22] Change nice interface of "find_name_in_mro()" to evil interface eating lookup exceptions, just like "_PyType_Lookup()" did previously. --- Objects/typeobject.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 17713a0750d7d4f..83128173fce6046 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2993,10 +2993,9 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) res = _PyDict_GetItem_KnownHash(dict, name, hash); if (res != NULL) break; - if (PyErr_Occurred()) { - *error = -1; - goto done; - } + /* _PyType_Lookup() ignored and cleared lookup errors and we keep this + bad behaviour, instead of returning NULL and setting error = -1. */ + PyErr_Clear(); } *error = 0; done: From f5bce2a5494ee5856677d6345a55254e6cff2607 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Wed, 13 Sep 2017 16:33:00 +0200 Subject: [PATCH 18/22] Mention amount of speedup in News entry. --- .../2017-09-13-12-04-23.bpo-31336.gi2ahY.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst index cd2f2b6cfb51b2f..e62b065af17b5af 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-13-12-04-23.bpo-31336.gi2ahY.rst @@ -1,2 +1,2 @@ -Speed up class creation by reducing overhead in the necessary special method -lookups. Patch by Stefan Behnel. +Speed up class creation by 10-20% by reducing the overhead in the +necessary special method lookups. Patch by Stefan Behnel. From 02bfef0f8dbe7b51e6f3c231c0d3d7a2554687ca Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Thu, 14 Sep 2017 10:12:06 +0200 Subject: [PATCH 19/22] Revert "Change nice interface of "find_name_in_mro()" to evil interface eating lookup exceptions, just like "_PyType_Lookup()" did previously." This reverts commit 4efde8ea72195e987a7946985b22725b1d8f4e3a. --- Objects/typeobject.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 83128173fce6046..17713a0750d7d4f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2993,9 +2993,10 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) res = _PyDict_GetItem_KnownHash(dict, name, hash); if (res != NULL) break; - /* _PyType_Lookup() ignored and cleared lookup errors and we keep this - bad behaviour, instead of returning NULL and setting error = -1. */ - PyErr_Clear(); + if (PyErr_Occurred()) { + *error = -1; + goto done; + } } *error = 0; done: From 24978587ccaebb90ac858c6285b6d793a47ed969 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Thu, 14 Sep 2017 10:16:52 +0200 Subject: [PATCH 20/22] Guard against external live exceptions when calling find_name_in_mro() from functions that swallow live exceptions. --- Objects/typeobject.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 17713a0750d7d4f..16cd9b8c08a5f33 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3026,6 +3026,9 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) } } + /* We may end up clearing live exceptions below, so make sure it's ours. */ + assert(!PyErr_Occurred()); + res = find_name_in_mro(type, name, &error); /* Only put NULL results into cache if there was no error. */ if (error) { @@ -7001,6 +7004,8 @@ update_one_slot(PyTypeObject *type, slotdef *p) } while (p->offset == offset); return p; } + /* We may end up clearing live exceptions below, so make sure it's ours. */ + assert(!PyErr_Occurred()); do { /* Use faster uncached lookup as we won't get any cache hits during type setup. */ descr = find_name_in_mro(type, p->name_strobj, &error); From 6c610287510e25479b18eca0fcca5b6e6178e628 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Thu, 14 Sep 2017 17:43:32 +0200 Subject: [PATCH 21/22] Propagate exception when _PyObject_LookupSpecial() fails (as done everywhere else). --- Objects/bytesobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 6a4eb67808ab836..a8d7e7b662e78de 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -567,6 +567,8 @@ format_obj(PyObject *v, const char **pbuf, Py_ssize_t *plen) *pbuf = PyBytes_AS_STRING(result); *plen = PyBytes_GET_SIZE(result); return result; + } else if (PyErr_Occurred()) { + return NULL; } /* does it support buffer protocol? */ if (PyObject_CheckBuffer(v)) { From 1b5950faeef5cf0a733d004f078b0b0e0e1c6790 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Fri, 15 Sep 2017 10:23:32 +0200 Subject: [PATCH 22/22] Allow _PyType_Lookup() and _PyType_LookupId() to raise exceptions and handle them in the code that calls these functions. The one non-exception error case during lookup ("no MRO") is not handled since it is unlikely and not user visible. Note that this changes the signature of both functions, but they are clearly not public functions. Currently leads to crashes due to NULL returns without exceptions set. Needs investigation. --- Include/object.h | 4 +- Modules/_collectionsmodule.c | 21 +++++-- Modules/_lsprof.c | 3 +- Objects/classobject.c | 10 ++- Objects/object.c | 15 ++++- Objects/typeobject.c | 118 +++++++++++++++++++++-------------- 6 files changed, 112 insertions(+), 59 deletions(-) diff --git a/Include/object.h b/Include/object.h index 9bb780e28bcca14..84efaba2f6bec5a 100644 --- a/Include/object.h +++ b/Include/object.h @@ -501,8 +501,8 @@ PyAPI_FUNC(PyObject *) PyType_GenericAlloc(PyTypeObject *, Py_ssize_t); PyAPI_FUNC(PyObject *) PyType_GenericNew(PyTypeObject *, PyObject *, PyObject *); #ifndef Py_LIMITED_API -PyAPI_FUNC(PyObject *) _PyType_Lookup(PyTypeObject *, PyObject *); -PyAPI_FUNC(PyObject *) _PyType_LookupId(PyTypeObject *, _Py_Identifier *); +PyAPI_FUNC(PyObject *) _PyType_Lookup(PyTypeObject *, PyObject *, int *); +PyAPI_FUNC(PyObject *) _PyType_LookupId(PyTypeObject *, _Py_Identifier *, int *); PyAPI_FUNC(PyObject *) _PyObject_LookupSpecial(PyObject *, _Py_Identifier *); PyAPI_FUNC(PyTypeObject *) _PyType_CalculateMetaclass(PyTypeObject *, PyObject *); #endif diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 8766d86dd3ef371..3d6fa0aba63dfbb 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2257,6 +2257,7 @@ _count_elements(PyObject *self, PyObject *args) PyObject *dict_get; PyObject *mapping_setitem; PyObject *dict_setitem; + int error; if (!PyArg_UnpackTuple(args, "_count_elements", 2, 2, &mapping, &iterable)) return NULL; @@ -2268,10 +2269,22 @@ _count_elements(PyObject *self, PyObject *args) /* Only take the fast path when get() and __setitem__() * have not been overridden. */ - mapping_get = _PyType_LookupId(Py_TYPE(mapping), &PyId_get); - dict_get = _PyType_LookupId(&PyDict_Type, &PyId_get); - mapping_setitem = _PyType_LookupId(Py_TYPE(mapping), &PyId___setitem__); - dict_setitem = _PyType_LookupId(&PyDict_Type, &PyId___setitem__); + mapping_get = _PyType_LookupId(Py_TYPE(mapping), &PyId_get, &error); + if (error == -1) { + goto done; + } + dict_get = _PyType_LookupId(&PyDict_Type, &PyId_get, &error); + if (error == -1) { + goto done; + } + mapping_setitem = _PyType_LookupId(Py_TYPE(mapping), &PyId___setitem__, &error); + if (error == -1) { + goto done; + } + dict_setitem = _PyType_LookupId(&PyDict_Type, &PyId___setitem__, &error); + if (error == -1) { + goto done; + } if (mapping_get != NULL && mapping_get == dict_get && mapping_setitem != NULL && mapping_setitem == dict_setitem) { diff --git a/Modules/_lsprof.c b/Modules/_lsprof.c index 602747098bb66bc..15ee215a54c934f 100644 --- a/Modules/_lsprof.c +++ b/Modules/_lsprof.c @@ -199,9 +199,10 @@ normalizeUserObj(PyObject *obj) PyObject *self = fn->m_self; PyObject *name = PyUnicode_FromString(fn->m_ml->ml_name); PyObject *modname = fn->m_module; + int ignore; if (name != NULL) { - PyObject *mo = _PyType_Lookup(Py_TYPE(self), name); + PyObject *mo = _PyType_Lookup(Py_TYPE(self), name, &ignore); Py_XINCREF(mo); Py_DECREF(name); if (mo != NULL) { diff --git a/Objects/classobject.c b/Objects/classobject.c index 063c24a7171e8fd..0fd66cc4fae37b1 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -138,13 +138,14 @@ method_getattro(PyObject *obj, PyObject *name) PyMethodObject *im = (PyMethodObject *)obj; PyTypeObject *tp = obj->ob_type; PyObject *descr = NULL; + int error; { if (tp->tp_dict == NULL) { if (PyType_Ready(tp) < 0) return NULL; } - descr = _PyType_Lookup(tp, name); + descr = _PyType_Lookup(tp, name, &error); } if (descr != NULL) { @@ -155,6 +156,8 @@ method_getattro(PyObject *obj, PyObject *name) Py_INCREF(descr); return descr; } + } else if (error == -1) { + return NULL; } return PyObject_GetAttr(im->im_func, name); @@ -462,12 +465,13 @@ instancemethod_getattro(PyObject *self, PyObject *name) { PyTypeObject *tp = self->ob_type; PyObject *descr = NULL; + int error; if (tp->tp_dict == NULL) { if (PyType_Ready(tp) < 0) return NULL; } - descr = _PyType_Lookup(tp, name); + descr = _PyType_Lookup(tp, name, &error); if (descr != NULL) { descrgetfunc f = TP_DESCR_GET(descr->ob_type); @@ -477,6 +481,8 @@ instancemethod_getattro(PyObject *self, PyObject *name) Py_INCREF(descr); return descr; } + } else if (error == -1) { + return NULL; } return PyObject_GetAttr(PyInstanceMethod_GET_FUNCTION(self), name); diff --git a/Objects/object.c b/Objects/object.c index ed8a62a163aa652..7b8620be3e5ebe9 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1043,6 +1043,7 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) PyObject **dictptr, *dict; PyObject *attr; int meth_found = 0; + int error; assert(*method == NULL); @@ -1055,7 +1056,7 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) if (tp->tp_dict == NULL && PyType_Ready(tp) < 0) return 0; - descr = _PyType_Lookup(tp, name); + descr = _PyType_Lookup(tp, name, &error); if (descr != NULL) { Py_INCREF(descr); if (PyFunction_Check(descr) || @@ -1069,6 +1070,8 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) return 0; } } + } else if (error == -1) { + return 0; } dictptr = _PyObject_GetDictPtr(obj); @@ -1122,6 +1125,7 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, PyObject *dict) descrgetfunc f; Py_ssize_t dictoffset; PyObject **dictptr; + int error; if (!PyUnicode_Check(name)){ PyErr_Format(PyExc_TypeError, @@ -1136,7 +1140,7 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, PyObject *dict) goto done; } - descr = _PyType_Lookup(tp, name); + descr = _PyType_Lookup(tp, name, &error); f = NULL; if (descr != NULL) { @@ -1146,6 +1150,8 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, PyObject *dict) res = f(descr, obj, (PyObject *)obj->ob_type); goto done; } + } else if (error == -1) { + goto done; } if (dict == NULL) { @@ -1216,6 +1222,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, descrsetfunc f; PyObject **dictptr; int res = -1; + int error; if (!PyUnicode_Check(name)){ PyErr_Format(PyExc_TypeError, @@ -1229,7 +1236,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, Py_INCREF(name); - descr = _PyType_Lookup(tp, name); + descr = _PyType_Lookup(tp, name, &error); if (descr != NULL) { Py_INCREF(descr); @@ -1238,6 +1245,8 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, res = f(descr, obj, value); goto done; } + } else if (error == -1) { + goto done; } if (dict == NULL) { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e770962d2fd651f..a43b7623db0f8a3 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -580,7 +580,7 @@ static int add_subclass(PyTypeObject*, PyTypeObject*); static int add_all_subclasses(PyTypeObject *type, PyObject *bases); static void remove_subclass(PyTypeObject *, PyTypeObject *); static void remove_all_subclasses(PyTypeObject *type, PyObject *bases); -static void update_all_slots(PyTypeObject *); +static int update_all_slots(PyTypeObject *); typedef int (*update_callback)(PyTypeObject *, void *); static int update_subclasses(PyTypeObject *type, PyObject *name, @@ -736,7 +736,10 @@ type_set_bases(PyTypeObject *type, PyObject *new_bases, void *context) add to all new_bases */ remove_all_subclasses(type, old_bases); res = add_all_subclasses(type, new_bases); - update_all_slots(type); + /* FIXME: unclear if we must update the slots even on errors or not */ + if (res == 0) { + res = update_all_slots(type); + } } Py_DECREF(old_bases); @@ -1405,7 +1408,7 @@ PyType_IsSubtype(PyTypeObject *a, PyTypeObject *b) Variants: - - _PyObject_LookupSpecial() returns NULL without raising an exception + - _PyObject_LookupSpecial() returns NULL with or without raising an exception when the _PyType_Lookup() call fails; - lookup_maybe_method() and lookup_method() are internal routines similar @@ -1418,8 +1421,9 @@ PyObject * _PyObject_LookupSpecial(PyObject *self, _Py_Identifier *attrid) { PyObject *res; + int ignore; - res = _PyType_LookupId(Py_TYPE(self), attrid); + res = _PyType_LookupId(Py_TYPE(self), attrid, &ignore); if (res != NULL) { descrgetfunc f; if ((f = Py_TYPE(res)->tp_descr_get) == NULL) @@ -1433,7 +1437,8 @@ _PyObject_LookupSpecial(PyObject *self, _Py_Identifier *attrid) static PyObject * lookup_maybe_method(PyObject *self, _Py_Identifier *attrid, int *unbound) { - PyObject *res = _PyType_LookupId(Py_TYPE(self), attrid); + int ignore; + PyObject *res = _PyType_LookupId(Py_TYPE(self), attrid, &ignore); if (res == NULL) { return NULL; } @@ -2074,7 +2079,7 @@ solid_base(PyTypeObject *type) static void object_dealloc(PyObject *); static int object_init(PyObject *, PyObject *, PyObject *); static int update_slot(PyTypeObject *, PyObject *); -static void fixup_slot_dispatchers(PyTypeObject *); +static int fixup_slot_dispatchers(PyTypeObject *); static int set_names(PyTypeObject *); static int init_subclass(PyTypeObject *, PyObject *); @@ -2099,8 +2104,9 @@ static PyObject * get_dict_descriptor(PyTypeObject *type) { PyObject *descr; + int ignore; - descr = _PyType_LookupId(type, &PyId___dict__); + descr = _PyType_LookupId(type, &PyId___dict__, &ignore); if (descr == NULL || !PyDescr_IsData(descr)) return NULL; @@ -2751,7 +2757,9 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds) goto error; /* Put the proper slots in place */ - fixup_slot_dispatchers(type); + if (fixup_slot_dispatchers(type) < 0) { + goto error; + } if (type->tp_dictoffset) { et->ht_cached_keys = _PyDict_NewKeysForClass(); @@ -3006,12 +3014,12 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) } /* Internal API to look for a name through the MRO. - This returns a borrowed reference, and doesn't set an exception! */ + This returns a borrowed reference, and might set an exception. + 'error' is set to: -1: error with exception; 1: error without exception; 0: ok */ PyObject * -_PyType_Lookup(PyTypeObject *type, PyObject *name) +_PyType_Lookup(PyTypeObject *type, PyObject *name, int *error) { PyObject *res; - int error; unsigned int h; if (MCACHE_CACHEABLE_NAME(name) && @@ -3023,6 +3031,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) #if MCACHE_STATS method_cache_hits++; #endif + *error = 0; return method_cache[h].value; } } @@ -3030,20 +3039,10 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) /* We may end up clearing live exceptions below, so make sure it's ours. */ assert(!PyErr_Occurred()); - res = find_name_in_mro(type, name, &error); + res = find_name_in_mro(type, name, error); /* Only put NULL results into cache if there was no error. */ - if (error) { - /* It's not ideal to clear the error condition, - but this function is documented as not setting - an exception, and I don't want to change that. - E.g., when PyType_Ready() can't proceed, it won't - set the "ready" flag, so future attempts to ready - the same type will call it again -- hopefully - in a context that propagates the exception out. - */ - if (error == -1) { - PyErr_Clear(); - } + if (*error) { + assert(*error == 1 || PyErr_Occurred()); return NULL; } @@ -3061,17 +3060,20 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) #endif Py_SETREF(method_cache[h].name, name); } + *error = 0; return res; } PyObject * -_PyType_LookupId(PyTypeObject *type, struct _Py_Identifier *name) +_PyType_LookupId(PyTypeObject *type, struct _Py_Identifier *name, int *error) { PyObject *oname; oname = _PyUnicode_FromId(name); /* borrowed */ - if (oname == NULL) + if (oname == NULL) { + *error = -1; return NULL; - return _PyType_Lookup(type, oname); + } + return _PyType_Lookup(type, oname, error); } /* This is similar to PyObject_GenericGetAttr(), @@ -3082,6 +3084,7 @@ type_getattro(PyTypeObject *type, PyObject *name) PyTypeObject *metatype = Py_TYPE(type); PyObject *meta_attribute, *attribute; descrgetfunc meta_get; + int error; if (!PyUnicode_Check(name)) { PyErr_Format(PyExc_TypeError, @@ -3100,7 +3103,7 @@ type_getattro(PyTypeObject *type, PyObject *name) meta_get = NULL; /* Look for the attribute in the metatype */ - meta_attribute = _PyType_Lookup(metatype, name); + meta_attribute = _PyType_Lookup(metatype, name, &error); if (meta_attribute != NULL) { meta_get = Py_TYPE(meta_attribute)->tp_descr_get; @@ -3114,11 +3117,13 @@ type_getattro(PyTypeObject *type, PyObject *name) (PyObject *)metatype); } Py_INCREF(meta_attribute); + } else if (error == -1) { + return NULL; } /* No data descriptor found on metatype. Look in tp_dict of this * type and its bases */ - attribute = _PyType_Lookup(type, name); + attribute = _PyType_Lookup(type, name, &error); if (attribute != NULL) { /* Implement descriptor functionality, if any */ descrgetfunc local_get = Py_TYPE(attribute)->tp_descr_get; @@ -3134,6 +3139,9 @@ type_getattro(PyTypeObject *type, PyObject *name) Py_INCREF(attribute); return attribute; + } else if (error == -1) { + Py_XDECREF(meta_attribute); + return NULL; } /* No attribute found in local __dict__ (or bases): use the @@ -6353,14 +6361,18 @@ slot_tp_getattr_hook(PyObject *self, PyObject *name) PyTypeObject *tp = Py_TYPE(self); PyObject *getattr, *getattribute, *res; _Py_IDENTIFIER(__getattr__); + int error; /* speed hack: we could use lookup_maybe, but that would resolve the method fully for each attribute lookup for classes with __getattr__, even when the attribute is present. So we use _PyType_Lookup and create the method only when needed, with call_attribute. */ - getattr = _PyType_LookupId(tp, &PyId___getattr__); + getattr = _PyType_LookupId(tp, &PyId___getattr__, &error); if (getattr == NULL) { + if (error == -1) { + return NULL; + } /* No __getattr__ hook: use a simpler dispatcher */ tp->tp_getattro = slot_tp_getattro; return slot_tp_getattro(self, name); @@ -6371,13 +6383,17 @@ slot_tp_getattr_hook(PyObject *self, PyObject *name) __getattr__, even when self has the default __getattribute__ method. So we use _PyType_Lookup and create the method only when needed, with call_attribute. */ - getattribute = _PyType_LookupId(tp, &PyId___getattribute__); + getattribute = _PyType_LookupId(tp, &PyId___getattribute__, &error); + if (getattribute == NULL && error == -1) { + Py_DECREF(getattr); + return NULL; + } if (getattribute == NULL || - (Py_TYPE(getattribute) == &PyWrapperDescr_Type && - ((PyWrapperDescrObject *)getattribute)->d_wrapped == - (void *)PyObject_GenericGetAttr)) + (Py_TYPE(getattribute) == &PyWrapperDescr_Type && + ((PyWrapperDescrObject *)getattribute)->d_wrapped == + (void *)PyObject_GenericGetAttr)) { res = PyObject_GenericGetAttr(self, name); - else { + } else { Py_INCREF(getattribute); res = call_attribute(self, getattribute, name); Py_DECREF(getattribute); @@ -6486,9 +6502,13 @@ slot_tp_descr_get(PyObject *self, PyObject *obj, PyObject *type) PyTypeObject *tp = Py_TYPE(self); PyObject *get; _Py_IDENTIFIER(__get__); + int error; - get = _PyType_LookupId(tp, &PyId___get__); + get = _PyType_LookupId(tp, &PyId___get__, &error); if (get == NULL) { + if (error == -1) { + return NULL; + } /* Avoid further slowdowns */ if (tp->tp_descr_get == slot_tp_descr_get) tp->tp_descr_get = NULL; @@ -7012,10 +7032,7 @@ update_one_slot(PyTypeObject *type, slotdef *p) descr = find_name_in_mro(type, p->name_strobj, &error); if (descr == NULL) { if (error == -1) { - /* It is unlikely by not impossible that there has been an exception - during lookup. Since this function originally expected no errors, - we ignore them here in order to keep up the interface. */ - PyErr_Clear(); + return NULL; } if (ptr == (void**)&type->tp_iternext) { specific = (void *)_PyObject_NextNotImplemented; @@ -7086,8 +7103,11 @@ update_slots_callback(PyTypeObject *type, void *data) { slotdef **pp = (slotdef **)data; - for (; *pp; pp++) - update_one_slot(type, *pp); + for (; *pp; pp++) { + if (update_one_slot(type, *pp) == NULL) { + return -1; + } + } return 0; } @@ -7163,26 +7183,30 @@ update_slot(PyTypeObject *type, PyObject *name) /* Store the proper functions in the slot dispatches at class (type) definition time, based upon which operations the class overrides in its dict. */ -static void +static int fixup_slot_dispatchers(PyTypeObject *type) { slotdef *p; init_slotdefs(); - for (p = slotdefs; p->name; ) + for (p = slotdefs; p && p->name; ) { p = update_one_slot(type, p); + } + return (p == NULL) ? -1 : 0; } -static void +static int update_all_slots(PyTypeObject* type) { slotdef *p; init_slotdefs(); for (p = slotdefs; p->name; p++) { - /* update_slot returns int but can't actually fail */ - update_slot(type, p->name_strobj); + if (update_slot(type, p->name_strobj) < 0) { + return -1; + } } + return 0; } /* Call __set_name__ on all descriptors in a newly generated type */