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

Improve the Efficiency of Python3.11.1 __getattr__ #102213

Closed
wangxiang-hz opened this issue Feb 24, 2023 · 4 comments
Closed

Improve the Efficiency of Python3.11.1 __getattr__ #102213

wangxiang-hz opened this issue Feb 24, 2023 · 4 comments
Labels
performance Performance or resource usage type-feature A feature request or enhancement

Comments

@wangxiang-hz
Copy link
Contributor

wangxiang-hz commented Feb 24, 2023

Feature or enhancement

__getattr__ in Python3.11.1 is much slower than @Property and visiting an object's attribute. It's even slower than Python3.10.4.
_PyObject_GenericGetAttrWithDict is the key reason. If Python3 fails finding an attribute in normal ways, it will return NULL and raise an exception. But raising an exception has performance cost. Python3.11.1 add set_attribute_error_context to support Fine Grained Error Locations in Tracebacks. It makes things worser.

Pitch

We can use this test code:

import time
import sys

class A:
    def foo(self):
        print("Call A.foo!")

    def __getattr__(self, name):
        return 2

    @property
    def ppp(self):
        return 3

class B(A):
    def foo(self):
        print("Call B.foo!")


class C(B):

    def __init__(self) -> None:
        self.pps = 1

    def foo(self):
        print("Call C.foo!")


def main():
    start = time.time()
    for i in range(1, 1000000):
        pass
    end = time.time()
    peer = end - start
    c = C()
    print(f"Python version of {sys.version}")
    start = time.time()
    for i in range(1, 1000000):
        s = c.pps
    end = time.time()
    print(f"Normal getattr spend time: {end - start - peer}")
    start = time.time()
    for i in range(1, 1000000):
        s = c.ppa
    end = time.time()
    print(f"Call __getattr__ spend time: {end - start - peer}")
    start = time.time()
    for i in range(1, 1000000):
        s = c.ppp
    end = time.time()
    print(f"Call property spend time: {end - start - peer}")

if __name__ == "__main__":
    main()

The result shows how slow __getattr__ is:

Python version of 3.11.1 (main, Dec 26 2022, 16:32:50) [GCC 8.3.0]
Normal getattr spend time: 0.03204226493835449
Call __getattr__ spend time: 0.4767305850982666
Call property spend time: 0.06345891952514648

When we define __getattr__, failed to find an attribute is what we expected. If we can get this result and then call __getattr__ without exception handling, it will be faster.
I tried to modify Python3.11.1 like this:

  1. add a new function in object.c:
PyObject *
PyObject_GenericTryGetAttr(PyObject *obj, PyObject *name)
{
    return _PyObject_GenericGetAttrWithDict(obj, name, NULL, 1);
}
  1. change typeobject.c :
if (getattribute == NULL ||
        (Py_IS_TYPE(getattribute, &PyWrapperDescr_Type) &&
         ((PyWrapperDescrObject *)getattribute)->d_wrapped ==
         (void *)PyObject_GenericGetAttr))
        // res = PyObject_GenericGetAttr(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))
            PyErr_Clear();
        res = call_attribute(self, getattr, name);
    }
    Py_DECREF(getattr);
    return res;

Rebuild python, it really become faster: spend time: 0.13772845268249512.

Previous discussion


getattr is much slower in Python3.11

Linked PRs

@wangxiang-hz wangxiang-hz added the type-feature A feature request or enhancement label Feb 24, 2023
@AlexWaygood AlexWaygood added the performance Performance or resource usage label Feb 24, 2023
@Fidget-Spinner
Copy link
Member

Could you submit a pull request please? I'll try to review it.

wangxiang-hz added a commit to wangxiang-hz/cpython that referenced this issue Feb 25, 2023
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.
wangxiang-hz added a commit to wangxiang-hz/cpython that referenced this issue Feb 26, 2023
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.
@wangxiang-hz
Copy link
Contributor Author

Could you submit a pull request please? I'll try to review it.

I have already submitted a pull request, but the default assigned reviewer is too busy to review it. Could you please review it and provide some suggestions when you are available? Thank you very much.

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Mar 8, 2023

Sorry. I didn't see the pull request (pull requests linked to an issue don't ping me even though I'm subscribe to the issue). I'll review it. Thanks!

@wangxiang-hz
Copy link
Contributor Author

Sorry. I didn't see the pull request (pull requests linked to an issue don't ping me even though I'm subscribe to the issue). I'll review it. Thanks!

Thank you. I know that contributors in open source communities are very busy, so I didn't want to bother you with a “polite ping” :) I really appreciate your review.

Fidget-Spinner pushed a commit that referenced this issue Mar 11, 2023
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.
iritkatriel pushed a commit to iritkatriel/cpython that referenced this issue Mar 12, 2023
…102248)

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.
sobolevn added a commit to sobolevn/cpython that referenced this issue Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants