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

Avoid parsing with Salsa #13437

Merged
merged 3 commits into from
Sep 21, 2024
Merged

Avoid parsing with Salsa #13437

merged 3 commits into from
Sep 21, 2024

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Sep 21, 2024

Summary

For reasons I haven't investigated, this speeds up the resolver about 2x (from 6.404s to 3.612s on an extremely large codebase).

Test Plan

\cc @BurntSushi

[andrew@duff rippling]$ time ruff analyze graph --preview > /dev/null

real    3.274
user    16.039
sys     7.609
maxmem  11631 MB
faults  0
[andrew@duff rippling]$ time ruff-patch analyze graph --preview > /dev/null

real    1.841
user    14.625
sys     3.639
maxmem  7173 MB
faults  0
[andrew@duff rippling]$ time ruff-patch2 analyze graph --preview > /dev/null

real    2.087
user    15.333
sys     4.869
maxmem  8642 MB
faults  0

Where that's main, then (ruff-patch) using the version with no File, no SemanticModel, then (ruff-patch2) using File.

.resolve_module(import)
.map(|module| module.file().path(self.semantic.db())),
CollectedImport::Import(import) => {
resolve_module(self.db, import).map(|module| module.file().path(self.db))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaReiser -- Would you prefer that I create a struct that exposes this? (I can't use SemanticModel because that requires a File, and I no longer have a File upfront.)

@charliermarsh charliermarsh added the performance Potential performance improvement label Sep 21, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) September 21, 2024 13:47
@charliermarsh charliermarsh merged commit 3018303 into main Sep 21, 2024
19 checks passed
@charliermarsh charliermarsh deleted the charlie/parse-db branch September 21, 2024 13:52
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Potential performance improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant