From f06d44e6e5cea8d83b904ed06dcbf721fcd3bc5f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 21 Sep 2024 13:00:02 -0400 Subject: [PATCH] Use `forget` for module resolver database (#13438) ## Summary A tiny bit faster and the `red-knot` CLI does the same thing. --- crates/ruff/src/commands/analyze_graph.rs | 185 +++++++++++----------- 1 file changed, 95 insertions(+), 90 deletions(-) diff --git a/crates/ruff/src/commands/analyze_graph.rs b/crates/ruff/src/commands/analyze_graph.rs index ce10ed69d3995..4b5ce6ee994ce 100644 --- a/crates/ruff/src/commands/analyze_graph.rs +++ b/crates/ruff/src/commands/analyze_graph.rs @@ -67,102 +67,105 @@ pub(crate) fn analyze_graph( .into(), )?; - // Create a cache for resolved globs. - let glob_resolver = Arc::new(Mutex::new(GlobResolver::default())); - - // Collect and resolve the imports for each file. - let result = Arc::new(Mutex::new(Vec::new())); - let inner_result = Arc::clone(&result); - - rayon::scope(move |scope| { - for resolved_file in paths { - let Ok(resolved_file) = resolved_file else { - continue; - }; + let imports = { + // Create a cache for resolved globs. + let glob_resolver = Arc::new(Mutex::new(GlobResolver::default())); + + // Collect and resolve the imports for each file. + let result = Arc::new(Mutex::new(Vec::new())); + let inner_result = Arc::clone(&result); + let db = db.snapshot(); + + rayon::scope(move |scope| { + for resolved_file in paths { + let Ok(resolved_file) = resolved_file else { + continue; + }; + + let path = resolved_file.path(); + let package = path + .parent() + .and_then(|parent| package_roots.get(parent)) + .and_then(Clone::clone); + + // Resolve the per-file settings. + let settings = resolver.resolve(path); + let string_imports = settings.analyze.detect_string_imports; + let include_dependencies = settings.analyze.include_dependencies.get(path).cloned(); + + // Skip excluded files. + if (settings.file_resolver.force_exclude || !resolved_file.is_root()) + && match_exclusion( + resolved_file.path(), + resolved_file.file_name(), + &settings.analyze.exclude, + ) + { + continue; + } - let path = resolved_file.path(); - let package = path - .parent() - .and_then(|parent| package_roots.get(parent)) - .and_then(Clone::clone); - - // Resolve the per-file settings. - let settings = resolver.resolve(path); - let string_imports = settings.analyze.detect_string_imports; - let include_dependencies = settings.analyze.include_dependencies.get(path).cloned(); - - // Skip excluded files. - if (settings.file_resolver.force_exclude || !resolved_file.is_root()) - && match_exclusion( - resolved_file.path(), - resolved_file.file_name(), - &settings.analyze.exclude, - ) - { - continue; - } + // Ignore non-Python files. + let source_type = match settings.analyze.extension.get(path) { + None => match SourceType::from(&path) { + SourceType::Python(source_type) => source_type, + SourceType::Toml(_) => { + debug!("Ignoring TOML file: {}", path.display()); + continue; + } + }, + Some(language) => PySourceType::from(language), + }; + if matches!(source_type, PySourceType::Ipynb) { + debug!("Ignoring Jupyter notebook: {}", path.display()); + continue; + } - // Ignore non-Python files. - let source_type = match settings.analyze.extension.get(path) { - None => match SourceType::from(&path) { - SourceType::Python(source_type) => source_type, - SourceType::Toml(_) => { - debug!("Ignoring TOML file: {}", path.display()); - continue; + // Convert to system paths. + let Ok(package) = package.map(SystemPathBuf::from_path_buf).transpose() else { + warn!("Failed to convert package to system path"); + continue; + }; + let Ok(path) = SystemPathBuf::from_path_buf(resolved_file.into_path()) else { + warn!("Failed to convert path to system path"); + continue; + }; + + let db = db.snapshot(); + let glob_resolver = glob_resolver.clone(); + let root = root.clone(); + let result = inner_result.clone(); + scope.spawn(move |_| { + // Identify any imports via static analysis. + let mut imports = + ModuleImports::detect(&db, &path, package.as_deref(), string_imports) + .unwrap_or_else(|err| { + warn!("Failed to generate import map for {path}: {err}"); + ModuleImports::default() + }); + + debug!("Discovered {} imports for {}", imports.len(), path); + + // Append any imports that were statically defined in the configuration. + if let Some((root, globs)) = include_dependencies { + let mut glob_resolver = glob_resolver.lock().unwrap(); + imports.extend(glob_resolver.resolve(root, globs)); } - }, - Some(language) => PySourceType::from(language), - }; - if matches!(source_type, PySourceType::Ipynb) { - debug!("Ignoring Jupyter notebook: {}", path.display()); - continue; - } - - // Convert to system paths. - let Ok(package) = package.map(SystemPathBuf::from_path_buf).transpose() else { - warn!("Failed to convert package to system path"); - continue; - }; - let Ok(path) = SystemPathBuf::from_path_buf(resolved_file.into_path()) else { - warn!("Failed to convert path to system path"); - continue; - }; - let db = db.snapshot(); - let glob_resolver = glob_resolver.clone(); - let root = root.clone(); - let result = inner_result.clone(); - scope.spawn(move |_| { - // Identify any imports via static analysis. - let mut imports = - ModuleImports::detect(&db, &path, package.as_deref(), string_imports) - .unwrap_or_else(|err| { - warn!("Failed to generate import map for {path}: {err}"); - ModuleImports::default() - }); - - debug!("Discovered {} imports for {}", imports.len(), path); - - // Append any imports that were statically defined in the configuration. - if let Some((root, globs)) = include_dependencies { - let mut glob_resolver = glob_resolver.lock().unwrap(); - imports.extend(glob_resolver.resolve(root, globs)); - } + // Convert the path (and imports) to be relative to the working directory. + let path = path + .strip_prefix(&root) + .map(SystemPath::to_path_buf) + .unwrap_or(path); + let imports = imports.relative_to(&root); - // Convert the path (and imports) to be relative to the working directory. - let path = path - .strip_prefix(&root) - .map(SystemPath::to_path_buf) - .unwrap_or(path); - let imports = imports.relative_to(&root); - - result.lock().unwrap().push((path, imports)); - }); - } - }); + result.lock().unwrap().push((path, imports)); + }); + } + }); - // Collect the results. - let imports = Arc::into_inner(result).unwrap().into_inner()?; + // Collect the results. + Arc::into_inner(result).unwrap().into_inner()? + }; // Generate the import map. let import_map = match args.direction { @@ -173,6 +176,8 @@ pub(crate) fn analyze_graph( // Print to JSON. println!("{}", serde_json::to_string_pretty(&import_map)?); + std::mem::forget(db); + Ok(ExitStatus::Success) }