-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
[discussion] Use a rust crate to make astroid faster, or part of astroid faster #2014
Comments
A few notes:
The major problem I see is that if you move away from Python's |
I think the main issue with contributing to |
I think it's a given that breaking the API will need to be done:
But what I want to discuss here is:
|
I don't think At the moment I think it will only limit the number of potential contributors (venn diagram of people interested in helping |
One way that I see rust working out is by moving the intensive logic into rust. That worked for libcst very well in my experience. Personally, I think moving the inference engine to rust is something to think about. i.e. things like By doing so, not only are you moving the complex, highly recursive logic to a type safe, memory safe language that's easier to maintain, you're also probably speeding up inference many times over and solving recursion limit bugs like pylint-dev/pylint#7898 Proper separation of code and being able to rewrite inference to a cleaner codebase is added bonus. |
Totally agree, but until we have a modern codebase I don't think we have a very good idea of what that area would be. I really don't want to squash anybody's hopes of doing this because I am in favour, but I think we should first have a much better design in order to, for example, not end up with the globally shared caching issues like we do now. |
Something else to consider is compiling with Mypyc along the lines of https://ichard26.github.io/blog/2022/05/compiling-black-with-mypyc-part-1/. Obviously doing that will require getting the codebase correctly typed. Correct typing would also likely be a preliminary step for fitting in a Rust module. However, the existing codebase is manifestly type-incorrect, and that type-incorrect behavior is guaranteed as part of the API in perpetuity. Fixing the type errors is impossible, since the type errors are considered correct by definition. So I agree with @DanielNoord that development should be moved towards version 3, and sooner rather than later. Is there any reason not to do it? |
Definitely. And seeing how hard it is to type astroid at the moment, the first step is probably to do some design and breaking changes to make our lives easier first. (I'm thinking in particular of not having post-init modification on nodes in order to have less Optional everywhere, but I'm sure Daniel will have a lot of ideas about what to break exactly) |
We recently opened a thread to start the discussion about it, but I wonder if it will be feasible to track the changes necessary. |
I actually have one idea for this already, which I just go during my evening running round. Let me catch up with some of my emails and then I'll work on it and create a PR. |
Well that wasn't so fruitful. I tried to fix the issue of how to type |
I've been thinking and astroid's tree rebuilder seems another part of the code that could work well for that. The input from the ast module is well typed at least. Also the message store and message id store in pylint which only deal with simple strings. Thoughts ? |
Here's some inspiration from a earily similar situation : https://www.gauge.sh/blog/python-extensions-should-be-lazy |
I was reading this article lately. With pydantic, ruff and other python package using rust to provide faster python using binding offered by https://github.com/PyO3/pyo3 . I've been wondering if we should explore the possibility to do that too.
Potential contributor to astroid are scarce, so we rejected using cython earlier (In #606 (comment) and elsewhere with hippo91 but I can't find the link right now).
However:
ruff
performance proves it's possible as a linter and also that it's a massive boost (10x or more) and a game changerProblem I foresee:
Maybe it's possible to use rust on the bottleneck only, i.e. on inference.
The text was updated successfully, but these errors were encountered: