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-115775: Use __static_attributes__ to initialize shared keys #118468

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented May 1, 2024

@markshannon
Copy link
Member Author

No need for news item as it is covered by #117909

@markshannon markshannon force-pushed the use-attributes-to-guide-inline-values-size branch from dedffa7 to 5032ad9 Compare May 1, 2024 13:56
@markshannon markshannon marked this pull request as ready for review May 1, 2024 20:38
@markshannon markshannon requested a review from methane as a code owner May 1, 2024 20:38
@markshannon markshannon force-pushed the use-attributes-to-guide-inline-values-size branch from 846a7ef to a7b858b Compare May 2, 2024 10:47
Python/specialize.c Outdated Show resolved Hide resolved
@markshannon markshannon force-pushed the use-attributes-to-guide-inline-values-size branch from b64de3d to d54afaf Compare May 3, 2024 18:30
@markshannon
Copy link
Member Author

markshannon commented May 3, 2024

The stats are not obviously better.

This PR reduces misses in LOAD_ATTR specializations, but it also increases the number of deferred operations due to shadowing.

Both STORE_ATTR_WITH_HINT and LOAD_ATTR_WITH_HINT are reduced in number considerably, being replaced by STORE_ATTR_INSTANCE_VALUE and LOAD_ATTR_INSTANCE_VALUE.

From this, it appears that this PR does what it aims to do. It also appears that it would be worth improving our handling of shadowed attributes.

I plan to fix the shadowing issue before merging this, so that it becomes a clear improvement

@markshannon
Copy link
Member Author

markshannon commented Aug 23, 2024

Now that #123040 is fixed things look much better.

Performance shows no change apart from the "go" benchmark speeds up by 35%. The "go" benchmark creates many objects before initializing them, which is a pattern that this PR allows us to optimize.

The stats show a small increase in specialization of LOAD_ATTR with a 95% reduction of "not in dict" specialization failure. Almost certainly most of that is from the "go" benchmark.

@markshannon markshannon merged commit 89328f7 into python:main Aug 27, 2024
35 checks passed
@markshannon markshannon deleted the use-attributes-to-guide-inline-values-size branch August 27, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants