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

Remove the _PyOptimizer APIs #126599

Open
brandtbucher opened this issue Nov 8, 2024 · 7 comments
Open

Remove the _PyOptimizer APIs #126599

brandtbucher opened this issue Nov 8, 2024 · 7 comments
Labels
3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-JIT

Comments

@brandtbucher
Copy link
Member

brandtbucher commented Nov 8, 2024

We should rip the _PyOptimizer API out, and just keep all of the code that does that actual optimizing.

The API itself is unstable, constantly changing, poorly-defined, and undocumented. There's a bunch of infrastructure required just to test the API (not the actual optimizations we perform), and it introduces indirection and artificial boundaries into some pretty performance-sensitive stuff. Nobody is using it that we're aware of, nobody we've talked to is planning on using it, and frankly we don't want anyone to start using it. So let's remove it.

Linked PRs

@brandtbucher brandtbucher added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.14 new features, bugs and security fixes topic-JIT labels Nov 8, 2024
@xuantengh
Copy link
Contributor

Hi can I take and try this issue?

@rruuaanng
Copy link
Contributor

Don't rush, please wait for the discussion result :)

@brandtbucher
Copy link
Member Author

This has already been discussed quite a bit offline with the relevant parties. I think we can probably move ahead with it.

It's a pretty big project, but help is welcome if you really feel confident tackling it. It's not just deleting things, but also refactoring the code to remove the current API boundary between the interpreter and the optimizer. I'm happy to help out if you get stuck or need any advice, too.

@rruuaanng
Copy link
Contributor

@xuantengh Would you like to challenge?

If you don't have the confidence to give it to me, I will finish it when I have time.

@xuantengh
Copy link
Contributor

Would you like to challenge?

Hi I would like to give it a try.

@xuantengh
Copy link
Contributor

xuantengh commented Nov 11, 2024

Hi @brandtbucher, after some preliminary investigation, this issue aims to remove the following 3 internal C APIs:

  • _PyOptimizer_NewCounter
  • _PyOptimizer_NewUOpOptimizer
  • _PyOptimizer_Optimize

where the first two are only used for tests (_testinternalcapi.c), and it's trival to rip them. For the last, it's used in instruction JUMP_BACKWARD as well as tier2 OPs _EXIT_TRACE and _DYNAMIC_EXIT. So we need to refactor the code to let these instruction/OPs directly conduct the optimizations?

Please correct me if something wrong or missing.

@rruuaanng
Copy link
Contributor

@xuantengh Maybe you should submit a PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-JIT
Projects
None yet
Development

No branches or pull requests

3 participants