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

[3.11] GH-93354: Use exponential backoff to avoid excessive specialization attempts (GH-93355) #93379

Open
wants to merge 2 commits into
base: 3.11
Choose a base branch
from
Open

[3.11] GH-93354: Use exponential backoff to avoid excessive specialization attempts (GH-93355) #93379

wants to merge 2 commits into from
+95 −48

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented May 31, 2022

Backport of #93355

This doesn't fix any obvious bugs in 3.11, but it is a preventative fix.

Specialization of some instructions can be quite expensive (relative to the cost of executing the instruction), so we want to avoid excessive numbers of failed specializations for corner cases that we might not yet be aware of.

@pablogsal

@markshannon
[3.11] GH-93354: Use exponential backoff to avoid excessive specializ…
852dca8 
…ation attempts. (GH-93355)

(cherry picked from commit eb618d5)

Co-authored-by: Mark Shannon <mark@hotpy.org>
@bedevere-bot bedevere-bot mentioned this pull request May 31, 2022
Merged
@pablogsal
Copy link
Member

@pablogsal pablogsal commented May 31, 2022

Seems there is a bunch of compiler warnings (check the "Files" tab in this PR). Also, there is a lot of code here and I am not sure I feel very confident merging this so I suppose it depends on how bd is the regression we are fixing. Could you elaborate with some benchmarks on the cases this is fixing (I know there is an explanation on the issue, but I want some discussion here specific for 3.11).

Also, if we decided to go with this, we are going to need at least 2 other core devs to review and approve this change.

@markshannon
Copy link
Member Author

@markshannon markshannon commented May 31, 2022

Seems there is a bunch of compiler warnings

Cherry picker messed up the merge. I'll fix that.

Fidget-Spinner
Fidget-Spinner approved these changes Jun 11, 2022
View changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

This LGTM.

Re: what Pablo said. IMO the risk of introducing bugs with this change is low. At worst we degrade performance a little. I don't see how this could introduce anything majorly breaking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fidget-Spinner Fidget-Spinner

@brandtbucher brandtbucher

Assignees
No one assigned
Labels
awaiting merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@markshannon @pablogsal @Fidget-Spinner @bedevere-bot