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

Jump to bottom

Finish up LOAD_ATTR specialisation #100288

Open
1 of 2 tasks
Fidget-Spinner opened this issue Dec 16, 2022 · 6 comments
Open
1 of 2 tasks

Finish up LOAD_ATTR specialisation #100288

Fidget-Spinner opened this issue Dec 16, 2022 · 6 comments
Labels
performance Performance or resource usage type-feature A feature request or enhancement

Comments

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Dec 16, 2022 •
edited by bedevere-bot

We should target the following specialisation failures:

  • has managed dict
  • not in dict (how does this even happen??)

With these two we can hit >90% specialisation successes.
If we're feeling really ambitious, we could aim for "not managed dict" failure too. But we don't need that to achieve >90% successes.

I'm doing the first one. Is anyone interested in investigating the second specialisation failure?

Linked PRs
@Fidget-Spinner Fidget-Spinner added the type-feature A feature request or enhancement label Dec 16, 2022
@bedevere-bot bedevere-bot mentioned this issue Dec 16, 2022
Merged
@Fidget-Spinner Fidget-Spinner added the performance Performance or resource usage label Dec 16, 2022
@corona10
Copy link
Member

corona10 commented Dec 16, 2022

Oh I am interested in if you don't mind?

@Fidget-Spinner
Copy link
Member Author

Fidget-Spinner commented Dec 16, 2022

Sure! Go ahead!

@Fidget-Spinner
Copy link
Member Author

Fidget-Spinner commented Dec 16, 2022

CC @markshannon just these two added specialisations should net us 10% all specialisation failures/successes, bringing us to over 90% specialisation successes.

@Fidget-Spinner
Copy link
Member Author

Fidget-Spinner commented Dec 16, 2022

@corona10 seems like the stats were misleading, not in dict is actually the following:

class C: 
    i = 0

c = C()
c.i # this fails to specialise

So it's a class attribute lookup, which makes more sense.

@corona10
Copy link
Member

corona10 commented Dec 16, 2022

Thanks I read the PR too: #100295

@markshannon
Copy link
Member

markshannon commented Dec 16, 2022

Regarding "not managed dict" failures, I think the approach should be to reduce the number of objects that don't have managed dicts, not specialize for them.

We can tweak the management of cached keys, and do some static analysis in the compiler to do that.

Fidget-Spinner added a commit that referenced this issue Dec 23, 2022
@Fidget-Spinner
gh-100288: Specialise LOAD_ATTR_METHOD for managed dictionaries (GH-1…
c3c7848 
…00289)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@corona10 @markshannon @Fidget-Spinner