Avoid parsing with Salsa (#13437)

## 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`.
This commit is contained in:
Charlie Marsh 2024-09-21 09:52:16 -04:00 committed by GitHub
parent 6c303b2445
commit 3018303c87
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 27 additions and 26 deletions

1
Cargo.lock generated
View file

@ -2456,6 +2456,7 @@ dependencies = [
"ruff_linter", "ruff_linter",
"ruff_macros", "ruff_macros",
"ruff_python_ast", "ruff_python_ast",
"ruff_python_parser",
"salsa", "salsa",
"schemars", "schemars",
"serde", "serde",

View file

@ -16,6 +16,7 @@ ruff_db = { workspace = true, features = ["os", "serde"] }
ruff_linter = { workspace = true } ruff_linter = { workspace = true }
ruff_macros = { workspace = true } ruff_macros = { workspace = true }
ruff_python_ast = { workspace = true } ruff_python_ast = { workspace = true }
ruff_python_parser = { workspace = true }
anyhow = { workspace = true } anyhow = { workspace = true }
clap = { workspace = true, optional = true } clap = { workspace = true, optional = true }

View file

@ -1,6 +1,8 @@
use red_knot_python_semantic::ModuleName; use red_knot_python_semantic::ModuleName;
use ruff_python_ast::visitor::source_order::{walk_body, walk_expr, walk_stmt, SourceOrderVisitor}; use ruff_python_ast::visitor::source_order::{
use ruff_python_ast::{self as ast, Expr, ModModule, Stmt}; walk_expr, walk_module, walk_stmt, SourceOrderVisitor,
};
use ruff_python_ast::{self as ast, Expr, Mod, Stmt};
/// Collect all imports for a given Python file. /// Collect all imports for a given Python file.
#[derive(Default, Debug)] #[derive(Default, Debug)]
@ -23,8 +25,8 @@ impl<'a> Collector<'a> {
} }
#[must_use] #[must_use]
pub(crate) fn collect(mut self, module: &ModModule) -> Vec<CollectedImport> { pub(crate) fn collect(mut self, module: &Mod) -> Vec<CollectedImport> {
walk_body(&mut self, &module.body); walk_module(&mut self, module);
self.imports self.imports
} }
} }

View file

@ -3,11 +3,9 @@ pub use crate::db::ModuleDb;
use crate::resolver::Resolver; use crate::resolver::Resolver;
pub use crate::settings::{AnalyzeSettings, Direction}; pub use crate::settings::{AnalyzeSettings, Direction};
use anyhow::Result; use anyhow::Result;
use red_knot_python_semantic::SemanticModel;
use ruff_db::files::system_path_to_file;
use ruff_db::parsed::parsed_module;
use ruff_db::system::{SystemPath, SystemPathBuf}; use ruff_db::system::{SystemPath, SystemPathBuf};
use ruff_python_ast::helpers::to_module_path; use ruff_python_ast::helpers::to_module_path;
use ruff_python_parser::{parse, Mode};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet}; use std::collections::{BTreeMap, BTreeSet};
@ -29,11 +27,11 @@ impl ModuleImports {
string_imports: bool, string_imports: bool,
) -> Result<Self> { ) -> Result<Self> {
// Read and parse the source code. // Read and parse the source code.
let file = system_path_to_file(db, path)?; let source = std::fs::read_to_string(path)?;
let parsed = parsed_module(db, file); let parsed = parse(&source, Mode::Module)?;
let module_path = let module_path =
package.and_then(|package| to_module_path(package.as_std_path(), path.as_std_path())); package.and_then(|package| to_module_path(package.as_std_path(), path.as_std_path()));
let model = SemanticModel::new(db, file);
// Collect the imports. // Collect the imports.
let imports = let imports =
@ -42,7 +40,7 @@ impl ModuleImports {
// Resolve the imports. // Resolve the imports.
let mut resolved_imports = ModuleImports::default(); let mut resolved_imports = ModuleImports::default();
for import in imports { for import in imports {
let Some(resolved) = Resolver::new(&model).resolve(import) else { let Some(resolved) = Resolver::new(db).resolve(import) else {
continue; continue;
}; };
let Some(path) = resolved.as_system_path() else { let Some(path) = resolved.as_system_path() else {

View file

@ -1,37 +1,36 @@
use red_knot_python_semantic::SemanticModel; use red_knot_python_semantic::resolve_module;
use ruff_db::files::FilePath; use ruff_db::files::FilePath;
use crate::collector::CollectedImport; use crate::collector::CollectedImport;
use crate::ModuleDb;
/// Collect all imports for a given Python file. /// Collect all imports for a given Python file.
pub(crate) struct Resolver<'a> { pub(crate) struct Resolver<'a> {
semantic: &'a SemanticModel<'a>, db: &'a ModuleDb,
} }
impl<'a> Resolver<'a> { impl<'a> Resolver<'a> {
/// Initialize a [`Resolver`] with a given [`SemanticModel`]. /// Initialize a [`Resolver`] with a given [`ModuleDb`].
pub(crate) fn new(semantic: &'a SemanticModel<'a>) -> Self { pub(crate) fn new(db: &'a ModuleDb) -> Self {
Self { semantic } Self { db }
} }
/// Resolve the [`CollectedImport`] into a [`FilePath`]. /// Resolve the [`CollectedImport`] into a [`FilePath`].
pub(crate) fn resolve(&self, import: CollectedImport) -> Option<&'a FilePath> { pub(crate) fn resolve(&self, import: CollectedImport) -> Option<&'a FilePath> {
match import { match import {
CollectedImport::Import(import) => self CollectedImport::Import(import) => {
.semantic resolve_module(self.db, import).map(|module| module.file().path(self.db))
.resolve_module(import) }
.map(|module| module.file().path(self.semantic.db())),
CollectedImport::ImportFrom(import) => { CollectedImport::ImportFrom(import) => {
// Attempt to resolve the member (e.g., given `from foo import bar`, look for `foo.bar`). // Attempt to resolve the member (e.g., given `from foo import bar`, look for `foo.bar`).
let parent = import.parent(); let parent = import.parent();
self.semantic
.resolve_module(import) resolve_module(self.db, import)
.map(|module| module.file().path(self.semantic.db())) .map(|module| module.file().path(self.db))
.or_else(|| { .or_else(|| {
// Attempt to resolve the module (e.g., given `from foo import bar`, look for `foo`). // Attempt to resolve the module (e.g., given `from foo import bar`, look for `foo`).
self.semantic
.resolve_module(parent?) resolve_module(self.db, parent?).map(|module| module.file().path(self.db))
.map(|module| module.file().path(self.semantic.db()))
}) })
} }
} }