Skip to content

Conversation

@markshannon
Copy link
Member

@markshannon markshannon commented Aug 22, 2024

This PR specializes LOAD_ATTR for shadowed attributes, where the shadowing doesn't change the attribute.

Stats show a 30% reduction in specialization failures for LOAD_ATTR.

Performance might show a slight speedup, but is more likely just noise.

Copy link
Member

@iritkatriel iritkatriel left a comment

Choose a reason for hiding this comment

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

Stats show a 30% reduction in specialization failures for LOAD_ATTR.

Performance might show a slight speedup, but is more likely just noise.

Why is it useful to improve specialisation stats if it's not also improving performance?

@markshannon
Copy link
Member Author

markshannon commented Aug 22, 2024

Why is it useful to improve specialisation stats if it's not also improving performance?

It probably does improve performance, and we would expect it to, but it's too small to measure.
That's why we rely on the stats, as they are much less noisy.

We also expect the difference to be magnified on tier 2, once that is more complete.

@iritkatriel
Copy link
Member

LGTM, but I think another reviewer might make sense, @brandtbucher or @Fidget-Spinner maybe.

Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

This is some pretty subtle stuff, but it looks correct to me! I wasn't able to make it do anything incorrect with a local build.

@markshannon markshannon merged commit 5d3201f into python:main Aug 23, 2024
@markshannon markshannon deleted the specialize-shadowed-load-attr branch August 23, 2024 09:48
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