Skip to content

Remove _DYNAMIC_EXIT #129715

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

Open
brandtbucher opened this issue Feb 6, 2025 · 1 comment
Open

Remove _DYNAMIC_EXIT #129715

brandtbucher opened this issue Feb 6, 2025 · 1 comment
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage topic-JIT type-feature A feature request or enhancement

Comments

@brandtbucher
Copy link
Member

brandtbucher commented Feb 6, 2025

Looking at the latest benchmarking comparison for JIT on vs. JIT off, we currently have a number of benchmarks that are slowed down significantly, with 16 benchmarks slowing down over 5%, 3 slowing down over 15%, and one (generators) slowing down over 25%. Clearly, there are some code patterns that we just aren't handling well. If we want people to turn on the JIT, we need to minimize this potential downside.

It appears the culprit is, well, generators. In particular, _DYNAMIC_EXIT. Long story short, I think we should ditch _DYNAMIC_EXIT, at least for now. It was always a bit of a not-quite-complete solution to a hard tracing problem, and getting rid of it is enough to cancel out the slowdown on all of our slowest benchmarks (only 10 benchmarks are slowed down more than 5%, and none slowed down more than 10%).

We use _DYNAMIC_EXIT for a couple of code patterns:

  • Tracing into a generator, via FOR_ITER_GEN or SEND_GEN. This will no longer successfully project a trace.
  • Tracing into a function, via LOAD_ATTR_PROPERTY or BINARY_SUBSCR_GETITEM. These caches actually have the necessary information to continue tracing, so _DYNAMIC_EXIT isn't really needed anymore.
  • Tracing into a function that isn't in the reverse-lookup-by-version cache, which should be uncommon.

We may also want to fail to create traces that end in YIELD_VALUE, but that can be considered separately as a follow up.

Once this is done, we can work on a new solution for tracing generators that's more robust (perhaps by using normal side exits, and starting each trace with a guard on the instruction pointer). We should probably only merge a solution once it's proven to work in practice, since the JIT is beginning to mature and stabilize.

(Thanks @mdboom for the idea.)

Linked PRs

@brandtbucher brandtbucher added 3.14 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage topic-JIT labels Feb 6, 2025
@brandtbucher brandtbucher self-assigned this Feb 6, 2025
@brandtbucher
Copy link
Member Author

brandtbucher commented Feb 6, 2025

After talking with @markshannon about this, we've come up with the following path forward once _DYNAMIC_EXIT is out.

  • Don't project traces that end in underflow for now (this includes all YIELD_VALUEs).
  • Update the tracing machinery to handle LOAD_ATTR_PROPERTY and BINARY_SUBSCR_GETITEM. which have all of the info we need to continue projecting in their caches.
  • Update FOR_ITER_GEN and SEND_GEN to specialize for a specific code object and instruction pointer, and teach the tier two optimizer how to guard and trace through them (the code object guard can be replaced with a watcher, so we just do a single pointer comparison on the new instruction pointer as our only guard). If there are multiple yield points, they will chain the normal way. Also, un-skip test_for_iter_gen.
  • Then, do the same thing for YIELD_VALUE.
  • Then, do the same thing for RETURN_VALUE and RETURN_GENERATOR when they underflow.
  • Then, try decreasing JIT thresholds again.

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 topic-JIT type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants