8000 GH-118093: Better handling of short and mid-loop traces by brandtbucher · Pull Request #122252 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

GH-118093: Better handling of short and mid-loop traces #122252

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.

Sign up for GitHub

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
Merged
merged 10 commits into from
Jul 29, 2024
Merged

GH-118093: Better handling of short and mid-loop traces #122252

merged 10 commits into from
Jul 29, 2024

Conversation

@brandtbucher
Copy link
Member
@brandtbucher brandtbucher commented Jul 25, 2024 •
edited by bedevere-app bot
Loading

This makes two changes to trace projection:

  • Handle a single backward jump anywhere in a trace, rather than just at the beginning.
  • Rather than having a minimum trace length, just ensure that at least one instruction is fully translated. This prevents us from requiring the translation of several tiny instructions or "successfully" half-translating a large instruction.

Perf is neutral, but stats move in the right direction: 83% reduction in "inner loop found", 7% reduction in "trace too short", and 5% fewer optimization attempts overall. We execute 0.5% more uops and 0.5% fewer traces. 2% fewer tier one instructions executed overall.

@brandtbucher brandtbucher added performance Performance or resource usage skip news interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jul 25, 2024
@brandtbucher brandtbucher self-assigned this Jul 25, 2024
@bedevere-app bedevere-app bot mentioned this pull request Jul 25, 2024
Open
3 tasks
markshannon
markshannon reviewed Jul 25, 2024
View reviewed changes
Copy link
Member
@markshannon markshannon left a com 8000 ment

Choose a reason for hiding this comment

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

A couple of questions

markshannon
markshannon requested changes Jul 29, 2024
View reviewed changes
Copy link
Member
@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Can you:

  • Go back to removing the asserts, and leave is_resume alone.
  • Add a comment linking to #122390

Consider it approved once you've done that

@bedevere-app
Copy link
bedevere-app bot commented Jul 29, 2024

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@brandtbucher brandtbucher merged commit 7797182 into python:main Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon requested changes

Assignees

@brandtbucher brandtbucher

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@brandtbucher @markshannon
0