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

gh-102213: Optimizing the Performance of __getattr__ #102213 #102248

Merged
merged 3 commits into from
Mar 11, 2023
Merged

gh-102213: Optimizing the Performance of __getattr__ #102213 #102248

merged 3 commits into from
Mar 11, 2023

Conversation

wangxiang-hz
Copy link
Contributor

@wangxiang-hz wangxiang-hz commented 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.

Using this test code:

import time
import sys

NUM = 1000000

class AttrObj(object):
    def __init__(self) -> None:
        super(AttrObj, self).__init__()
        self.pps = 2

    def __getattr__(self, name):
        return 4

def main():
    start = time.time()
    for i in range(1, NUM):
        pass
    end = time.time()
    peer = end - start
    o = AttrObj()
    print(f"Python version of {sys.version}")
    start = time.time()
    for i in range(1, NUM):
        s = o.ppp
    end = time.time()
    print(f"Call __getattr__ spend time: {end - start - peer}")

if __name__ == "__main__":
    main()

Now the result is: Call __getattr__ spend time: 0.4704132080078125
After this modification, the result is: Call __getattr__ spend time: 0.07422256469726562

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Feb 25, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

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.
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@arhadthedev arhadthedev added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 26, 2023
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work. Just a few things I think we should tidy up.

Include/object.h Outdated Show resolved Hide resolved
Objects/object.c Outdated Show resolved Hide resolved
Objects/typeobject.c Outdated Show resolved Hide resolved
@wangxiang-hz
Copy link
Contributor Author

Good work. Just a few things I think we should tidy up.

Thank you for your advice, it was very helpful. I also think that modifying the public C API could cause issues, but I was unsure whether to put the new function together with PyObject_GenericGetAttr. I think you're right.
I have updated my pull request, could you please take a look when you have time?
Thank you very much.

@Fidget-Spinner
Copy link
Member

Also, thanks for addressing my review! In the future once you address the comments, you can just click "resolve conversation".

@Fidget-Spinner Fidget-Spinner merged commit aa0a73d into python:main Mar 11, 2023
@Fidget-Spinner
Copy link
Member

Thanks! Congrats on your first commit to CPython @wangxiang-hz !

@wangxiang-hz
Copy link
Contributor Author

Also, thanks for addressing my review! In the future once you address the comments, you can just click "resolve conversation".

Thanks! Congrats on your first commit to CPython @wangxiang-hz !

Thank you! It's really my honor to participate in the development of CPython! Looking forward to make more pull requests in the future:)

iritkatriel pushed a commit to iritkatriel/cpython that referenced this pull request 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 pull request Apr 7, 2023
Fidget-Spinner pushed a commit that referenced this pull request Apr 7, 2023
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
sunmy2019 added a commit to sunmy2019/cpython that referenced this pull request Apr 24, 2023
ambv added a commit to sunmy2019/cpython that referenced this pull request Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants