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

Move 'get_project_imports' to Rust extension [18.8x speedup] #82

Merged
merged 26 commits into from
May 29, 2024

Conversation

emdoyle
Copy link
Member

@emdoyle emdoyle commented May 24, 2024

By far most of the time spent by tach check or tach sync is in get_project_imports, particularly on large projects.
tach_python_sentry_flamegraph
[this is tach sync on the Sentry codebase (~3000 Python files)]

You can also see in that flamegraph that most of the time spent within import scanning is simply visiting all the nodes (even for essentially generic no-ops) and building the ASTs themselves.

This PR moves the implementation of get_project_imports to our Rust extension library, using the ruff Python AST parser and node visitor crates. The functionality is otherwise generally a direct translation of our Python code.

image
[this is tach sync on the Sentry codebase (~3000 Python files) using Rust for import scanning]

@emdoyle emdoyle changed the title Move 'get_project_imports' to Rust extension Move 'get_project_imports' to Rust extension [8.8x speedup] May 27, 2024
@emdoyle emdoyle marked this pull request as ready for review May 27, 2024 20:30
Base automatically changed from rust-extension to main May 27, 2024 21:27
@emdoyle emdoyle changed the title Move 'get_project_imports' to Rust extension [8.8x speedup] Move 'get_project_imports' to Rust extension [18.8x speedup] May 28, 2024
@emdoyle emdoyle merged commit 1f136bd into main May 29, 2024
5 checks passed
@emdoyle emdoyle deleted the find-imports-in-rust branch May 29, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants