NotificationsYou must be signed in to change notification settings
Fork32.1k
Star67.3k

gh-102213: Optimize the performance of`getattr`#103761

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

Fidget-Spinner merged 8 commits intopython:mainfromsunmy2019:gh-102248-corrected

May 1, 2023

Merged

gh-102213: Optimize the performance of`getattr`#103761

Fidget-Spinner merged 8 commits intopython:mainfromsunmy2019:gh-102248-corrected

May 1, 2023

Conversation

Copy link

Member

sunmy2019 commentedApr 24, 2023•
edited by bedevere-bot
Loading

This is the logically corrected version ofGH-102248.

GH-102248 caused several issues and had been reverted. This PR (I think) correctly implements it.

Tests passed on Ubuntu 22.04 (WSL) gcc-11 with--pydebug.

I also do the perf tests in the original PR, (--enable-optimizations with my AMD 5900X)
Before,

Python version of 3.12.0a7+ (heads/gh-102248-corrected:af53046995, Apr 24 2023, 22:42:22) [GCC 11.3.0]Call __getattr__ spend time: 0.2576024532318115

After,

Python version of 3.12.0a7+ (heads/gh-102248-corrected:ac81fd6c7e, Apr 24 2023, 22:39:39) [GCC 11.3.0]Call __getattr__ spend time: 0.04635906219482422

cc:@Fidget-Spinner

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

Optimize the performance of__getattr__

ac81fd6

This is the corrected version ofpythonGH-102248

sunmy2019 requested a review frommarkshannon as acode owner

April 24, 2023 14:44

bedevere-bot added the awaiting review label

Apr 24, 2023

Eclips4 reviewed

Apr 24, 2023

View reviewed changes

Objects/typeobject.c OutdatedShow resolvedHide resolved

Update Objects/typeobject.c

f6c3704

Co-authored-by: Kirill <80244920+Eclips4@users.noreply.github.com>

sunmy2019 commented

Apr 24, 2023

View reviewed changes

Objects/typeobject.c OutdatedShow resolvedHide resolved

Update Objects/typeobject.c

91aa490

sunmy2019 changed the title~~Optimize the performance of__getattr__~~gh-102213: Optimize the performance of__getattr__

Apr 24, 2023

bedevere-bot mentioned this pull request

Apr 24, 2023

Improve the Efficiency of Python3.11.1 __getattr__#102213

Closed

Copy link

Member

Fidget-Spinner commentedApr 24, 2023

Note to myself or whoever who merges this. Please add@wangxiang-hz to co-authors when merging as this is the fixed version of their PR, and the work should also be attributed to them.

Copy link

Member

Fidget-Spinner commentedApr 24, 2023

I will review and hopefully merge this on Friday when I have more time. Thanks.

Fidget-Spinner self-assigned this

Apr 24, 2023

Copy link

bluetech commentedApr 24, 2023

#103551 has a simple test case for the issue with the original PR, might be useful to add as a unit test for this PR.

Merge branch 'main' intopythongh-102248-corrected

c90be9d

Copy link

MemberAuthor

sunmy2019 commentedApr 25, 2023

#103551 has a simple test case for the issue with the original PR, might be useful to add as a unit test for this PR.

@property raises and__getattr__ not called --> added in#103358
@property calls__getattr__, and__getattr__ raises --> added in#103336

The test of#103551 should be covered by#103358. But a little duplication is okay. I will add it later.

add test case forpythongh-103551

c3aebf6

Fidget-Spinner reviewed

Apr 27, 2023

View reviewed changes

Objects/typeobject.c OutdatedShow resolvedHide resolved

Fidget-Spinner added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

Apr 27, 2023

Copy link

bedevere-bot commentedApr 27, 2023

🤖 New build scheduled with the buildbot fleet by@Fidget-Spinner for commitc3aebf6 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

Apr 27, 2023

Update Objects/typeobject.c

75d9a70

Copy link

MemberAuthor

sunmy2019 commentedApr 27, 2023•
edited
Loading

Does anyone have any idea why this fails?
https://buildbot.python.org/all/#/builders/738/builds/3112
The failure is a timing test. This PR seems to have changed the time usage of one particular test on ARM.

Got a timeout here:https://buildbot.python.org/all/#/builders/403/builds/1066

all builds:https://buildbot.python.org/all/#/changes/22158

Copy link

Member

Fidget-Spinner commentedApr 28, 2023

Got a timeout here:https://buildbot.python.org/all/#/builders/403/builds/1066

This specific test seems flaky. I would ignore it.

Copy link

MemberAuthor

sunmy2019 commentedApr 30, 2023•
edited
Loading

@Fidget-Spinner How is it going on? We are running out of time. New features will soon be frozen in May. 7th.
https://peps.python.org/pep-0693/

Or will this be considered as a bug fix?

Fidget-Spinner added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

Apr 30, 2023

Copy link

bedevere-bot commentedApr 30, 2023

🤖 New build scheduled with the buildbot fleet by@Fidget-Spinner for commit75d9a70 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

Apr 30, 2023

Copy link

Member

Fidget-Spinner commentedApr 30, 2023

@Fidget-Spinner How is it going on? We are running out of time. New features will soon be frozen in May. 7th.https://peps.python.org/pep-0693/
Or will this be considered as a bug fix?

I'm triggering a buildbot rerun to see if that specific buildbot is actually impacted.

Copy link

MemberAuthor

sunmy2019 commentedMay 1, 2023

https://buildbot.python.org/all/#builders/178/builds/1189

test_monitoring failure is tracked here
#103845
test_import failure is tracked here
#103879 (comment)

Seems no more failures

Copy link

Member

Fidget-Spinner commentedMay 1, 2023

Please add news. You can copy the previous PR's news item. I will attribute the commit to both of you.

Fidget-Spinner removed request forencukou,asvetlov,ericvsmith,abalkin,JelleZijlstra,ncoghlan,iritkatriel,ericsnowcurrently,hugovk,pganssle,rhettinger,willingc,gvanrossum,corona10,ethanfurman,pablogsal,erlend-aasland,terryjreedy,lysnikolaou,ezio-melotti,brandtbucher,kumaraditya303 andAlexWaygood

May 1, 2023 09:27

Fidget-Spinner merged commit59c27fa intopython:main

May 1, 2023

bedevere-bot removed the awaiting review label

May 1, 2023

Copy link

Member

arhadthedev commentedMay 1, 2023

Please, don't worry about 229 random files reported as changed. It's a GitHub after-merge artifact, the final commit59c27fa squash-merged intomain is fine.

carljm added a commit to carljm/cpython that referenced this pull request

May 1, 2023

Merge branch 'main' into supermon

159b2aa

* main: (26 commits)pythongh-104028: Reduce object creation while calling callback function from gc (pythongh-104030)pythongh-104036: Fix direct invocation of test_typing (python#104037)pythongh-102213: Optimize the performance of `__getattr__` (pythonGH-103761)pythongh-103895: Improve how invalid `Exception.__notes__` are displayed (python#103897)  Adjust expression from `==` to `!=` in alignment with the meaning of the paragraph. (pythonGH-104021)pythongh-88496: Fix IDLE test hang on macOS (python#104025)  Improve int test coverage (python#104024)pythongh-88773: Added teleport method to Turtle library (python#103974)pythongh-104015: Fix direct invocation of `test_dataclasses` (python#104017)pythongh-104012: Ensure test_calendar.CalendarTestCase.test_deprecation_warning consistently passes (python#104014)pythongh-103977: compile re expressions in platform.py only if required (python#103981)pythongh-98003: Inline call frames for CALL_FUNCTION_EX (pythonGH-98004)  Replace Netlify with Read the Docs build previews (python#103843)  Update name in acknowledgements and add mailmap (python#103696)pythongh-82054: allow test runner to split test_asyncio to execute in parallel by sharding. (python#103927)  Remove non-existing tools from Sundry skiplist (python#103991)pythongh-103793: Defer formatting task name (python#103767)pythongh-87092: change assembler to use instruction sequence instead of CFG (python#103933)pythongh-103636: issue warning for deprecated calendar constants (python#103833)  Various small fixes to dis docs (python#103923)  ...

carljm added a commit to carljm/cpython that referenced this pull request

May 1, 2023

Merge branch 'main' into inlinecomp2

1274e2b

* main: (463 commits)pythongh-104057: Fix direct invocation of test_super (python#104064)pythongh-87092: Expose assembler to unit tests (python#103988)pythongh-97696: asyncio eager tasks factory (python#102853)pythongh-84436: Immortalize in _PyStructSequence_InitBuiltinWithFlags() (pythongh-104054)pythongh-104057: Fix direct invocation of test_module (pythonGH-104059)pythongh-100458: Clarify Enum.__format__() change of mixed-in types in the whatsnew/3.11.rst (pythonGH-100387)pythongh-104018: disallow "z" format specifier in %-format of byte strings (pythonGH-104033)pythongh-104016: Fixed off by 1 error in f string tokenizer (python#104047)pythonGH-103629: Update Unpack's repr in compliance with PEP 692 (python#104048)pythongh-102799: replace sys.exc_info by sys.exception in inspect and traceback modules (python#104032)  Fix typo in "expected" word in few source files (python#104034)pythongh-103824: fix use-after-free error in Parser/tokenizer.c (python#103993)pythongh-104035: Do not ignore user-defined `__{get,set}state__` in slotted frozen dataclasses (python#104041)pythongh-104028: Reduce object creation while calling callback function from gc (pythongh-104030)pythongh-104036: Fix direct invocation of test_typing (python#104037)pythongh-102213: Optimize the performance of `__getattr__` (pythonGH-103761)pythongh-103895: Improve how invalid `Exception.__notes__` are displayed (python#103897)  Adjust expression from `==` to `!=` in alignment with the meaning of the paragraph. (pythonGH-104021)pythongh-88496: Fix IDLE test hang on macOS (python#104025)  Improve int test coverage (python#104024)  ...

Copy link

Contributor

wangxiang-hz commentedMay 4, 2023

Sorry guys, I just noticed this pull request. I was buy with my work these weeks and just finished a major update at the end of April..
I think@sunmy2019 is right and his commits is better.call_attribute should only be called when attribute is not found, then we call it, and clear thePyExc_AttributeError being raised.
I made a mistake,PyObject_GenericGetAttr may not raise attribute error but other scripts may still raise other exceptions. When this happen,call_attribute should not be called.
Thank you all for finding this and sorry for the bugs triggered.