Skip to content

Commit

Permalink
Avoid parsing with Salsa (#13437)
Browse files Browse the repository at this point in the history
## 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`.
  • Loading branch information
charliermarsh authored Sep 21, 2024
1 parent 6c303b2 commit 3018303
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 26 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/ruff_graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ruff_db = { workspace = true, features = ["os", "serde"] }
ruff_linter = { workspace = true }
ruff_macros = { workspace = true }
ruff_python_ast = { workspace = true }
ruff_python_parser = { workspace = true }

anyhow = { workspace = true }
clap = { workspace = true, optional = true }
Expand Down
10 changes: 6 additions & 4 deletions crates/ruff_graph/src/collector.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use red_knot_python_semantic::ModuleName;
use ruff_python_ast::visitor::source_order::{walk_body, walk_expr, walk_stmt, SourceOrderVisitor};
use ruff_python_ast::{self as ast, Expr, ModModule, Stmt};
use ruff_python_ast::visitor::source_order::{
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.
#[derive(Default, Debug)]
Expand All @@ -23,8 +25,8 @@ impl<'a> Collector<'a> {
}

#[must_use]
pub(crate) fn collect(mut self, module: &ModModule) -> Vec<CollectedImport> {
walk_body(&mut self, &module.body);
pub(crate) fn collect(mut self, module: &Mod) -> Vec<CollectedImport> {
walk_module(&mut self, module);
self.imports
}
}
Expand Down
12 changes: 5 additions & 7 deletions crates/ruff_graph/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ pub use crate::db::ModuleDb;
use crate::resolver::Resolver;
pub use crate::settings::{AnalyzeSettings, Direction};
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_python_ast::helpers::to_module_path;
use ruff_python_parser::{parse, Mode};
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet};

Expand All @@ -29,11 +27,11 @@ impl ModuleImports {
string_imports: bool,
) -> Result<Self> {
// Read and parse the source code.
let file = system_path_to_file(db, path)?;
let parsed = parsed_module(db, file);
let source = std::fs::read_to_string(path)?;
let parsed = parse(&source, Mode::Module)?;

let module_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.
let imports =
Expand All @@ -42,7 +40,7 @@ impl ModuleImports {
// Resolve the imports.
let mut resolved_imports = ModuleImports::default();
for import in imports {
let Some(resolved) = Resolver::new(&model).resolve(import) else {
let Some(resolved) = Resolver::new(db).resolve(import) else {
continue;
};
let Some(path) = resolved.as_system_path() else {
Expand Down
29 changes: 14 additions & 15 deletions crates/ruff_graph/src/resolver.rs
Original file line number Diff line number Diff line change
@@ -1,37 +1,36 @@
use red_knot_python_semantic::SemanticModel;
use red_knot_python_semantic::resolve_module;
use ruff_db::files::FilePath;

use crate::collector::CollectedImport;
use crate::ModuleDb;

/// Collect all imports for a given Python file.
pub(crate) struct Resolver<'a> {
semantic: &'a SemanticModel<'a>,
db: &'a ModuleDb,
}

impl<'a> Resolver<'a> {
/// Initialize a [`Resolver`] with a given [`SemanticModel`].
pub(crate) fn new(semantic: &'a SemanticModel<'a>) -> Self {
Self { semantic }
/// Initialize a [`Resolver`] with a given [`ModuleDb`].
pub(crate) fn new(db: &'a ModuleDb) -> Self {
Self { db }
}

/// Resolve the [`CollectedImport`] into a [`FilePath`].
pub(crate) fn resolve(&self, import: CollectedImport) -> Option<&'a FilePath> {
match import {
CollectedImport::Import(import) => self
.semantic
.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))
}
CollectedImport::ImportFrom(import) => {
// Attempt to resolve the member (e.g., given `from foo import bar`, look for `foo.bar`).
let parent = import.parent();
self.semantic
.resolve_module(import)
.map(|module| module.file().path(self.semantic.db()))

resolve_module(self.db, import)
.map(|module| module.file().path(self.db))
.or_else(|| {
// Attempt to resolve the module (e.g., given `from foo import bar`, look for `foo`).
self.semantic
.resolve_module(parent?)
.map(|module| module.file().path(self.semantic.db()))

resolve_module(self.db, parent?).map(|module| module.file().path(self.db))
})
}
}
Expand Down

0 comments on commit 3018303

Please # to comment.