From fdcb49c36b2ed8347d8d9f2dcd7052cc90207beb Mon Sep 17 00:00:00 2001 From: Itamar Ostricher Date: Thu, 4 May 2023 07:50:26 -0700 Subject: [PATCH] gh-104066: Improve performance of hasattr for module objects (#104063) --- Include/internal/pycore_moduleobject.h | 3 + ...-05-01-14-48-29.gh-issue-104066.pzoUZQ.rst | 2 + Objects/moduleobject.c | 91 +++++++++++++------ Objects/object.c | 11 +++ 4 files changed, 78 insertions(+), 29 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-05-01-14-48-29.gh-issue-104066.pzoUZQ.rst diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index 76361b8dff1..15a1bcb6ae5 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -36,6 +36,9 @@ static inline PyObject* _PyModule_GetDict(PyObject *mod) { return dict; } +PyObject* _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress); +PyObject* _Py_module_getattro(PyModuleObject *m, PyObject *name); + #ifdef __cplusplus } #endif diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-01-14-48-29.gh-issue-104066.pzoUZQ.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-01-14-48-29.gh-issue-104066.pzoUZQ.rst new file mode 100644 index 00000000000..97e0c01689c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-01-14-48-29.gh-issue-104066.pzoUZQ.rst @@ -0,0 +1,2 @@ +Improve the performance of :func:`hasattr` for module objects with a missing +attribute. diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index a0be19a3ca8..ffcd90ed122 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -702,7 +702,11 @@ int _PyModuleSpec_IsInitializing(PyObject *spec) { if (spec != NULL) { - PyObject *value = PyObject_GetAttr(spec, &_Py_ID(_initializing)); + PyObject *value; + int ok = _PyObject_LookupAttr(spec, &_Py_ID(_initializing), &value); + if (ok == 0) { + return 0; + } if (value != NULL) { int initializing = PyObject_IsTrue(value); Py_DECREF(value); @@ -738,19 +742,37 @@ _PyModuleSpec_IsUninitializedSubmodule(PyObject *spec, PyObject *name) return is_uninitialized; } -static PyObject* -module_getattro(PyModuleObject *m, PyObject *name) +PyObject* +_Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress) { + // When suppress=1, this function suppresses AttributeError. PyObject *attr, *mod_name, *getattr; - attr = PyObject_GenericGetAttr((PyObject *)m, name); - if (attr || !PyErr_ExceptionMatches(PyExc_AttributeError)) { + attr = _PyObject_GenericGetAttrWithDict((PyObject *)m, name, NULL, suppress); + if (attr) { return attr; } - PyErr_Clear(); + if (suppress == 1) { + if (PyErr_Occurred()) { + // pass up non-AttributeError exception + return NULL; + } + } + else { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + // pass up non-AttributeError exception + return NULL; + } + PyErr_Clear(); + } assert(m->md_dict != NULL); getattr = PyDict_GetItemWithError(m->md_dict, &_Py_ID(__getattr__)); if (getattr) { - return PyObject_CallOneArg(getattr, name); + PyObject *result = PyObject_CallOneArg(getattr, name); + if (result == NULL && suppress == 1 && PyErr_ExceptionMatches(PyExc_AttributeError)) { + // suppress AttributeError + PyErr_Clear(); + } + return result; } if (PyErr_Occurred()) { return NULL; @@ -763,37 +785,48 @@ module_getattro(PyModuleObject *m, PyObject *name) Py_DECREF(mod_name); return NULL; } - Py_XINCREF(spec); - if (_PyModuleSpec_IsInitializing(spec)) { - PyErr_Format(PyExc_AttributeError, - "partially initialized " - "module '%U' has no attribute '%U' " - "(most likely due to a circular import)", - mod_name, name); + if (suppress != 1) { + Py_XINCREF(spec); + if (_PyModuleSpec_IsInitializing(spec)) { + PyErr_Format(PyExc_AttributeError, + "partially initialized " + "module '%U' has no attribute '%U' " + "(most likely due to a circular import)", + mod_name, name); + } + else if (_PyModuleSpec_IsUninitializedSubmodule(spec, name)) { + PyErr_Format(PyExc_AttributeError, + "cannot access submodule '%U' of module '%U' " + "(most likely due to a circular import)", + name, mod_name); + } + else { + PyErr_Format(PyExc_AttributeError, + "module '%U' has no attribute '%U'", + mod_name, name); + } + Py_XDECREF(spec); } - else if (_PyModuleSpec_IsUninitializedSubmodule(spec, name)) { - PyErr_Format(PyExc_AttributeError, - "cannot access submodule '%U' of module '%U' " - "(most likely due to a circular import)", - name, mod_name); - } - else { - PyErr_Format(PyExc_AttributeError, - "module '%U' has no attribute '%U'", - mod_name, name); - } - Py_XDECREF(spec); Py_DECREF(mod_name); return NULL; } else if (PyErr_Occurred()) { return NULL; } - PyErr_Format(PyExc_AttributeError, - "module has no attribute '%U'", name); + if (suppress != 1) { + PyErr_Format(PyExc_AttributeError, + "module has no attribute '%U'", name); + } return NULL; } + +PyObject* +_Py_module_getattro(PyModuleObject *m, PyObject *name) +{ + return _Py_module_getattro_impl(m, name, 0); +} + static int module_traverse(PyModuleObject *m, visitproc visit, void *arg) { @@ -951,7 +984,7 @@ PyTypeObject PyModule_Type = { 0, /* tp_hash */ 0, /* tp_call */ 0, /* tp_str */ - (getattrofunc)module_getattro, /* tp_getattro */ + (getattrofunc)_Py_module_getattro, /* tp_getattro */ PyObject_GenericSetAttr, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | diff --git a/Objects/object.c b/Objects/object.c index c6ef5928164..41c52e21045 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1085,6 +1085,17 @@ _PyObject_LookupAttr(PyObject *v, PyObject *name, PyObject **result) return 0; } } + else if (tp->tp_getattro == (getattrofunc)_Py_module_getattro) { + // optimization: suppress attribute error from module getattro method + *result = _Py_module_getattro_impl((PyModuleObject*)v, name, 1); + if (*result != NULL) { + return 1; + } + if (PyErr_Occurred()) { + return -1; + } + return 0; + } else if (tp->tp_getattro != NULL) { *result = (*tp->tp_getattro)(v, name); }