Skip to content

Conversation

brandtbucher
Copy link
Member

@brandtbucher brandtbucher commented Jul 25, 2024

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
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.

A couple of questions

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
55 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants