GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-102213: Optimizing the Performance of __getattr__ #102213 by wangxiang-hz · Pull Request #102248 · python/cpython

Conversation

@wangxiang-hz

@wangxiang-hz wangxiang-hz commented Feb 25, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

  • Issue: Improve the Efficiency of Python3.11.1 __getattr__ #102213

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
lolyu and tommyleo2 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@wangxiang-hz
wangxiang-hz requested a review from markshannon as a code owner February 25, 2023 16:11
@bedevere-bot bedevere-bot added the awaiting review label Feb 25, 2023
@bedevere-bot bedevere-bot mentioned this pull request Feb 25, 2023
Improve the Efficiency of Python3.11.1 __getattr__ #102213
Closed
@bedevere-bot

bedevere-bot commented Feb 25, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ghost

ghost commented Feb 25, 2023 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
wangxiang @wangxiang-hz
Fix wangxiang-hz/getattr_optimization #102213
1a4b2e8 
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

bedevere-bot commented Feb 26, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@blurb-it
📜🤖 Added by blurb_it.
970566a
@arhadthedev arhadthedev added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 26, 2023
@AlexWaygood
AlexWaygood requested a review from Fidget-Spinner March 8, 2023 09:04
Fidget-Spinner
Fidget-Spinner reviewed Mar 8, 2023
View reviewed changes

@Fidget-Spinner Fidget-Spinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/object.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core and Builtins/2023-02-26-13-12-55.gh-issue-102213.fTH8X7.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/object.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/typeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
wangxiang
optimize implementation according to Fidget-Spinner's advise
f95d844
@wangxiang-hz

wangxiang-hz commented Mar 10, 2023

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner
Fidget-Spinner reviewed Mar 10, 2023
View reviewed changes
Comment thread Objects/typeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner

Fidget-Spinner commented Mar 10, 2023

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner
Fidget-Spinner merged commit aa0a73d into python:main Mar 11, 2023
@bedevere-bot bedevere-bot removed the awaiting review label Mar 11, 2023
@Fidget-Spinner

Fidget-Spinner commented Mar 11, 2023

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@wangxiang-hz

wangxiang-hz commented Mar 11, 2023

Copy link
Copy Markdown
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:)
AlexWaygood and bgruening reacted with heart emoji AlexWaygood and Fidget-Spinner reacted with rocket emoji
All reactions
  • ❤️ 2 reactions
  • 🚀 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel pushed a commit to iritkatriel/cpython that referenced this pull request Mar 12, 2023
@wangxiang-hz @iritkatriel
pythongh-102213: Optimize the performance of __getattr__ (pythonGH-…
fc48167 
…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
@sobolevn
Revert "pythongh-102213: Optimize the performance of __getattr__ (p…
96946fd 
…ythonGH-102248)"

This reverts commit aa0a73d.
Fidget-Spinner pushed a commit that referenced this pull request Apr 7, 2023
@sobolevn
gh-102213: Revert "gh-102213: Optimize the performance of `__getattr_…
059bb04 
…_` (GH-102248)" (GH-103332)

This reverts commit aa0a73d.
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@sobolevn @warsaw
pythongh-102213: Revert "pythongh-102213: Optimize the performance of…
ef37048 
… `__getattr__` (pythonGH-102248)" (pythonGH-103332)

This reverts commit aa0a73d.
sunmy2019 added a commit to sunmy2019/cpython that referenced this pull request Apr 24, 2023
@sunmy2019
Optimize the performance of __getattr__
ac81fd6 
This is the corrected version of pythonGH-102248
@sunmy2019 sunmy2019 mentioned this pull request Apr 24, 2023
gh-102213: Optimize the performance of __getattr__ #103761
Merged
ambv added a commit to sunmy2019/cpython that referenced this pull request Apr 25, 2023
@ambv
Merge branch 'main' into pythongh-102248-corrected
c90be9d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hauntsaninja hauntsaninja hauntsaninja left review comments
@Fidget-Spinner Fidget-Spinner Fidget-Spinner left review comments
@sunmy2019 sunmy2019 sunmy2019 left review comments
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@wangxiang-hz @bedevere-bot @Fidget-Spinner @hauntsaninja @sunmy2019 @arhadthedev
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.