From 1a4b2e8bb464a50d5817c3511db39081444e42a2 Mon Sep 17 00:00:00 2001 From: wangxiang Date: Sat, 25 Feb 2023 23:45:57 +0800 Subject: [PATCH 1/3] Fix wangxiang-hz/getattr_optimization #102213 when __getattr__ is defined, python with try to find an attribute using _PyObject_GenericGetAttrWithDict find nothing is reasonable so we don't need an exception, it will hurt performance. --- Include/object.h | 1 + Objects/object.c | 6 ++++++ Objects/typeobject.c | 8 +++++--- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Include/object.h b/Include/object.h index 3774f126730005..4623e99f2b9019 100644 --- a/Include/object.h +++ b/Include/object.h @@ -304,6 +304,7 @@ PyAPI_FUNC(int) PyObject_SetAttr(PyObject *, PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_HasAttr(PyObject *, PyObject *); PyAPI_FUNC(PyObject *) PyObject_SelfIter(PyObject *); PyAPI_FUNC(PyObject *) PyObject_GenericGetAttr(PyObject *, PyObject *); +PyAPI_FUNC(PyObject *) PyObject_GenericTryGetAttr(PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_GenericSetAttr(PyObject *, PyObject *, PyObject *); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03030000 PyAPI_FUNC(int) PyObject_GenericSetDict(PyObject *, PyObject *, void *); diff --git a/Objects/object.c b/Objects/object.c index 446c7b1f5f0302..5d0068df6a7684 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1364,6 +1364,12 @@ PyObject_GenericGetAttr(PyObject *obj, PyObject *name) return _PyObject_GenericGetAttrWithDict(obj, name, NULL, 0); } +PyObject * +PyObject_GenericTryGetAttr(PyObject *obj, PyObject *name) +{ + return _PyObject_GenericGetAttrWithDict(obj, name, NULL, 1); +} + int _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, PyObject *value, PyObject *dict) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b3f1429debc58b..7870ea437f8026 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -8248,14 +8248,16 @@ _Py_slot_tp_getattr_hook(PyObject *self, PyObject *name) (Py_IS_TYPE(getattribute, &PyWrapperDescr_Type) && ((PyWrapperDescrObject *)getattribute)->d_wrapped == (void *)PyObject_GenericGetAttr)) - res = PyObject_GenericGetAttr(self, name); + /* finding nothing is reasonable when __getattr__ is defined */ + res = PyObject_GenericTryGetAttr(self, name); else { Py_INCREF(getattribute); res = call_attribute(self, getattribute, name); Py_DECREF(getattribute); } - if (res == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) { - PyErr_Clear(); + if (res == NULL) { + if (PyErr_ExceptionMatches(PyExc_AttributeError)) + PyErr_Clear(); res = call_attribute(self, getattr, name); } Py_DECREF(getattr); From 970566aae236503222260ade8a54cec8feb963f2 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 26 Feb 2023 13:12:56 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst b/Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst new file mode 100644 index 00000000000000..976e2dde0d3b03 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst @@ -0,0 +1 @@ +Fix performance loss when accessing an object's attributes with `__getattr__` defined. From f95d84421dc91ad6bc51d7b8792bc1633964b9b5 Mon Sep 17 00:00:00 2001 From: wangxiang Date: Fri, 10 Mar 2023 11:31:59 +0800 Subject: [PATCH 3/3] optimize implementation according to Fidget-Spinner's advise --- Include/internal/pycore_object.h | 1 + Include/object.h | 1 - .../2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst | 2 +- Objects/object.c | 2 +- Objects/typeobject.c | 5 +++-- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 8796dfe2f6b8cf..292649cdecd78e 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -353,6 +353,7 @@ extern void _PyObject_FreeInstanceAttributes(PyObject *obj); extern int _PyObject_IsInstanceDictEmpty(PyObject *); extern int _PyType_HasSubclasses(PyTypeObject *); extern PyObject* _PyType_GetSubclasses(PyTypeObject *); +extern PyObject* _PyObject_GenericTryGetAttr(PyObject *, PyObject *); // Access macro to the members which are floating "behind" the object static inline PyMemberDef* _PyHeapType_GET_MEMBERS(PyHeapTypeObject *etype) { diff --git a/Include/object.h b/Include/object.h index 4623e99f2b9019..3774f126730005 100644 --- a/Include/object.h +++ b/Include/object.h @@ -304,7 +304,6 @@ PyAPI_FUNC(int) PyObject_SetAttr(PyObject *, PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_HasAttr(PyObject *, PyObject *); PyAPI_FUNC(PyObject *) PyObject_SelfIter(PyObject *); PyAPI_FUNC(PyObject *) PyObject_GenericGetAttr(PyObject *, PyObject *); -PyAPI_FUNC(PyObject *) PyObject_GenericTryGetAttr(PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_GenericSetAttr(PyObject *, PyObject *, PyObject *); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03030000 PyAPI_FUNC(int) PyObject_GenericSetDict(PyObject *, PyObject *, void *); diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst b/Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst index 976e2dde0d3b03..997bef226e713f 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst @@ -1 +1 @@ -Fix performance loss when accessing an object's attributes with `__getattr__` defined. +Fix performance loss when accessing an object's attributes with ``__getattr__`` defined. diff --git a/Objects/object.c b/Objects/object.c index 5d0068df6a7684..3caa5415347a3f 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1365,7 +1365,7 @@ PyObject_GenericGetAttr(PyObject *obj, PyObject *name) } PyObject * -PyObject_GenericTryGetAttr(PyObject *obj, PyObject *name) +_PyObject_GenericTryGetAttr(PyObject *obj, PyObject *name) { return _PyObject_GenericGetAttrWithDict(obj, name, NULL, 1); } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7870ea437f8026..b91e328dbf4d52 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -8249,15 +8249,16 @@ _Py_slot_tp_getattr_hook(PyObject *self, PyObject *name) ((PyWrapperDescrObject *)getattribute)->d_wrapped == (void *)PyObject_GenericGetAttr)) /* finding nothing is reasonable when __getattr__ is defined */ - res = PyObject_GenericTryGetAttr(self, name); + res = _PyObject_GenericTryGetAttr(self, name); else { Py_INCREF(getattribute); res = call_attribute(self, getattribute, name); Py_DECREF(getattribute); } if (res == NULL) { - if (PyErr_ExceptionMatches(PyExc_AttributeError)) + if (PyErr_ExceptionMatches(PyExc_AttributeError)) { PyErr_Clear(); + } res = call_attribute(self, getattr, name); } Py_DECREF(getattr);