Speed up inherited configuration resolution

crates/ruff_linter/src/settings/types.rs

@@ -4,9 +4,10 @@ use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use std::string::ToString;
+use std::sync::{Arc, OnceLock};
 
 use anyhow::{Context, Result, bail};
-use globset::{Glob, GlobMatcher, GlobSet, GlobSetBuilder};
+use globset::{Candidate, Glob, GlobMatcher, GlobSet, GlobSetBuilder};
 use log::debug;
 use pep440_rs::{VersionSpecifier, VersionSpecifiers};
 use ruff_db::diagnostic::DiagnosticFormat;
@@ -318,14 +319,23 @@ impl CacheKey for FilePatternSet {
 /// A glob pattern and associated data for matching file paths.
 #[derive(Debug, Clone)]
 pub struct PerFile<T> {
+    pattern: Arc<PerFilePattern>,
+    /// The per-file data associated with these glob patterns.
+    data: T,
+}
+
+/// A pattern shared by configurations that inherit the same per-file setting.
+#[derive(Debug)]
+struct PerFilePattern {
     /// The glob pattern used to construct the [`PerFile`].
     basename: String,
     /// The same pattern as `basename` but normalized to the project root directory.
     absolute: GlobPath,
     /// Whether the glob pattern should be negated (e.g. `!*.ipynb`)
     negated: bool,
-    /// The per-file data associated with these glob patterns.
-    data: T,
+    // Compile only when settings are resolved, after configuration overrides have been applied.
+    absolute_matcher: OnceLock<Result<GlobMatcher, globset::Error>>,
+    basename_matcher: OnceLock<Result<GlobMatcher, globset::Error>>,
 }
 
 impl<T> PerFile<T> {
@@ -342,9 +352,13 @@ impl<T> PerFile<T> {
         let project_root = project_root.unwrap_or(fs::get_cwd());
 
         Self {
-            absolute: GlobPath::normalize(&pattern, project_root),
-            basename: pattern,
-            negated,
+            pattern: Arc::new(PerFilePattern {
+                absolute: GlobPath::normalize(&pattern, project_root),
+                basename: pattern,
+                negated,
+                absolute_matcher: OnceLock::new(),
+                basename_matcher: OnceLock::new(),
+            }),
             data,
         }
     }
@@ -802,20 +816,30 @@ impl<T> CompiledPerFileList<T> {
         let inner: Result<Vec<_>> = per_file_items
             .into_iter()
             .map(|per_file_ignore| {
+                let pattern = &per_file_ignore.pattern;
+                // Cloning matchers shares the compiled regex but gives each settings object
+                // its own scratch space for matching.
                 // Construct absolute path matcher.
-                let absolute_matcher = Glob::new(&per_file_ignore.absolute.to_string_lossy())
-                    .with_context(|| format!("invalid glob {:?}", per_file_ignore.absolute))?
-                    .compile_matcher();
+                let absolute_matcher = pattern
+                    .absolute_matcher
+                    .get_or_init(|| {
+                        Glob::new(&pattern.absolute.to_string_lossy())
+                            .map(|glob| glob.compile_matcher())
+                    })
+                    .clone()
+                    .with_context(|| format!("invalid glob {:?}", pattern.absolute))?;
 
                 // Construct basename matcher.
-                let basename_matcher = Glob::new(&per_file_ignore.basename)
-                    .with_context(|| format!("invalid glob {:?}", per_file_ignore.basename))?
-                    .compile_matcher();
+                let basename_matcher = pattern
+                    .basename_matcher
+                    .get_or_init(|| Glob::new(&pattern.basename).map(|glob| glob.compile_matcher()))
+                    .clone()
+                    .with_context(|| format!("invalid glob {:?}", pattern.basename))?;
 
                 Ok(CompiledPerFile::new(
                     absolute_matcher,
                     basename_matcher,
-                    per_file_ignore.negated,
+                    pattern.negated,
                     per_file_ignore.data,
                 ))
             })
@@ -841,8 +865,10 @@ impl<T: std::fmt::Debug> CompiledPerFileList<T> {
         'a: 'p,
     {
         let file_name = path.file_name().expect("Unable to parse filename");
+        let basename = Candidate::new(file_name);
+        let absolute = Candidate::new(path);
         self.inner.iter().filter_map(move |entry| {
-            if entry.basename_matcher.is_match(file_name) {
+            if entry.basename_matcher.is_match_candidate(&basename) {
                 if entry.negated {
                     None
                 } else {
@@ -855,7 +881,7 @@ impl<T: std::fmt::Debug> CompiledPerFileList<T> {
                     );
                     Some(&entry.data)
                 }
-            } else if entry.absolute_matcher.is_match(path) {
+            } else if entry.absolute_matcher.is_match_candidate(&absolute) {
                 if entry.negated {
                     None
                 } else {
@@ -914,9 +940,7 @@ impl CompiledPerFileIgnoreList {
         let mut resolution_error = None;
         let list = CompiledPerFileList::resolve(per_file_ignores.into_iter().map(|ignore| {
             let PerFile {
-                basename,
-                absolute,
-                negated,
+                pattern,
                 data: selectors,
             } = ignore.0;
             // Rules in preview are included here via `all_rules` even if preview mode is disabled.
@@ -938,12 +962,7 @@ impl CompiledPerFileIgnoreList {
                 })
                 .flat_map(|selector| selector.all_rules())
                 .collect();
-            PerFile {
-                basename,
-                absolute,
-                negated,
-                data,
-            }
+            PerFile { pattern, data }
         }))?;
 
         if let Some(error) = resolution_error {
@@ -994,6 +1013,10 @@ impl CompiledPerFileTargetVersionList {
     }
 
     pub fn is_match(&self, path: &Path) -> Option<ast::PythonVersion> {
+        if self.0.is_empty() {
+            return None;
+        }
+
         self.0
             .iter_matches(path, "Setting Python version")
             .next()

crates/ruff_workspace/src/resolver.rs

@@ -5,7 +5,7 @@ use std::cmp::Ordering;
 use std::collections::BTreeSet;
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
-use std::sync::RwLock;
+use std::sync::{Arc, RwLock};
 
 use anyhow::{Context, Result};
 use anyhow::{anyhow, bail};
@@ -301,16 +301,57 @@ pub trait ConfigurationTransformer {
     fn transform(&self, config: Configuration) -> Configuration;
 }
 
+/// Configurations shared during one traversal, before inheritance and overrides.
+///
+/// Paths are already normalized, so the project root is part of the cache key.
+/// Target-version fallbacks and CLI overrides remain specific to each chain.
+#[derive(Default)]
+struct ConfigurationCache {
+    configurations: RwLock<FxHashMap<(PathBuf, PathBuf), Arc<Configuration>>>,
+}
+
+impl ConfigurationCache {
+    fn get_or_try_insert_with(
+        &self,
+        path: &Path,
+        project_root: &Path,
+        load: impl FnOnce() -> Result<Configuration>,
+    ) -> Result<Configuration> {
+        let key = (path.to_path_buf(), project_root.to_path_buf());
+        let cached = self.configurations.read().unwrap().get(&key).cloned();
+        if let Some(configuration) = cached {
+            return Ok((*configuration).clone());
+        }
+
+        // Parsing, conversion, and cloning stay outside the lock so unrelated
+        // configurations can load in parallel.
+        let configuration = Arc::new(load()?);
+        let shared = self
+            .configurations
+            .write()
+            .unwrap()
+            .entry(key)
+            .or_insert_with(|| Arc::clone(&configuration))
+            .clone();
+        Ok((*shared).clone())
+    }
+}
+
 /// Recursively resolve a [`Configuration`] from a `pyproject.toml` file at the
 /// specified [`Path`].
-// TODO(charlie): This whole system could do with some caching. Right now, if a
-// configuration file extends another in the same path, we'll re-parse the same
-// file at least twice (possibly more than twice, since we'll also parse it when
-// resolving the "default" configuration).
 pub fn resolve_configuration(
     initial_config_path: &Path,
     transformer: &dyn ConfigurationTransformer,
     origin: ConfigurationOrigin,
+) -> Result<Configuration> {
+    resolve_configuration_with_cache(initial_config_path, transformer, origin, None)
+}
+
+fn resolve_configuration_with_cache(
+    initial_config_path: &Path,
+    transformer: &dyn ConfigurationTransformer,
+    origin: ConfigurationOrigin,
+    configuration_cache: Option<&ConfigurationCache>,
 ) -> Result<Configuration> {
     let relativity = Relativity::from(origin);
     let mut configurations = indexmap::IndexMap::new();
@@ -327,27 +368,33 @@ pub fn resolve_configuration(
             ));
         }
 
-        let options = pyproject::load_options(&path).with_context(|| {
-            if configurations.is_empty() {
-                format!(
-                    "Failed to load configuration `{path}`",
-                    path = path.display()
-                )
-            } else {
-                let chain = configurations
-                    .keys()
-                    .chain([&path])
-                    .map(|p| format!("`{}`", p.display()))
-                    .join(" extends ");
-                format!(
-                    "Failed to load extended configuration `{path}` ({chain})",
-                    path = path.display()
-                )
-            }
-        })?;
-
         let project_root = relativity.resolve(&path);
-        let configuration = Configuration::from_options(options, Some(&path), project_root)?;
+        let load = || {
+            let options = pyproject::load_options(&path).with_context(|| {
+                if configurations.is_empty() {
+                    format!(
+                        "Failed to load configuration `{path}`",
+                        path = path.display()
+                    )
+                } else {
+                    let chain = configurations
+                        .keys()
+                        .chain([&path])
+                        .map(|p| format!("`{}`", p.display()))
+                        .join(" extends ");
+                    format!(
+                        "Failed to load extended configuration `{path}` ({chain})",
+                        path = path.display()
+                    )
+                }
+            })?;
+            Configuration::from_options(options, Some(&path), project_root)
+        };
+        let configuration = if let Some(cache) = configuration_cache {
+            cache.get_or_try_insert_with(&path, project_root, load)?
+        } else {
+            load()?
+        };
 
         // If extending, continue to collect.
         next = configuration.extend.as_ref().map(|extend| {
@@ -381,10 +428,12 @@ fn resolve_scoped_settings(
     pyproject: &Path,
     transformer: &dyn ConfigurationTransformer,
     origin: ConfigurationOrigin,
+    configuration_cache: Option<&ConfigurationCache>,
 ) -> Result<(PathBuf, Settings)> {
     let relativity = Relativity::from(origin);
 
-    let configuration = resolve_configuration(pyproject, transformer, origin)?;
+    let configuration =
+        resolve_configuration_with_cache(pyproject, transformer, origin, configuration_cache)?;
     let project_root = relativity.resolve(pyproject);
     let settings = configuration.into_settings(project_root)?;
     Ok((project_root.to_path_buf(), settings))
@@ -397,7 +446,7 @@ pub fn resolve_root_settings(
     transformer: &dyn ConfigurationTransformer,
     origin: ConfigurationOrigin,
 ) -> Result<Settings> {
-    let (_project_root, settings) = resolve_scoped_settings(pyproject, transformer, origin)?;
+    let (_project_root, settings) = resolve_scoped_settings(pyproject, transformer, origin, None)?;
     Ok(settings)
 }
 
@@ -437,6 +486,7 @@ pub fn project_files_in_path<'a>(
     // Search for `pyproject.toml` files in all parent directories.
     let mut resolver = Resolver::new(pyproject_config);
     let mut seen = FxHashSet::default();
+    let configuration_cache = ConfigurationCache::default();
 
     // Insert the path to the root configuration to avoid parsing the configuration a second time.
     if let Some(config_path) = &pyproject_config.path {
@@ -452,6 +502,7 @@ pub fn project_files_in_path<'a>(
                             &pyproject,
                             transformer,
                             ConfigurationOrigin::Ancestor,
+                            Some(&configuration_cache),
                         )?;
                         resolver.add(&root, settings, pyproject);
                         // We found the closest configuration.
@@ -499,7 +550,7 @@ pub fn project_files_in_path<'a>(
 
     // Run the `WalkParallel` to collect all files.
     let state = WalkPythonFilesState::new(resolver);
-    let mut visitor = PythonFilesVisitorBuilder::new(transformer, &state);
+    let mut visitor = PythonFilesVisitorBuilder::new(transformer, &state, &configuration_cache);
     walker.visit(&mut visitor);
 
     state.finish()
@@ -565,14 +616,20 @@ fn deduplicate_files(mut files: ResolvedFiles) -> ResolvedFiles {
 struct PythonFilesVisitorBuilder<'s, 'config> {
     state: &'s WalkPythonFilesState<'config>,
     transformer: &'s (dyn ConfigurationTransformer + Sync),
+    configuration_cache: &'s ConfigurationCache,
 }
 
 impl<'s, 'config> PythonFilesVisitorBuilder<'s, 'config> {
     fn new(
         transformer: &'s (dyn ConfigurationTransformer + Sync),
         state: &'s WalkPythonFilesState<'config>,
+        configuration_cache: &'s ConfigurationCache,
     ) -> Self {
-        Self { state, transformer }
+        Self {
+            state,
+            transformer,
+            configuration_cache,
+        }
     }
 }
 
@@ -581,6 +638,7 @@ struct PythonFilesVisitor<'s, 'config> {
     local_error: Result<()>,
     global: &'s WalkPythonFilesState<'config>,
     transformer: &'s (dyn ConfigurationTransformer + Sync),
+    configuration_cache: &'s ConfigurationCache,
 }
 
 impl<'config, 's> ignore::ParallelVisitorBuilder<'s> for PythonFilesVisitorBuilder<'s, 'config>
@@ -593,6 +651,7 @@ where
             local_error: Ok(()),
             global: self.state,
             transformer: self.transformer,
+            configuration_cache: self.configuration_cache,
         })
     }
 }
@@ -643,6 +702,7 @@ impl ParallelVisitor for PythonFilesVisitor<'_, '_> {
                             &pyproject,
                             self.transformer,
                             ConfigurationOrigin::Ancestor,
+                            Some(self.configuration_cache),
                         ) {
                             Ok((root, settings)) => {
                                 self.global
@@ -767,8 +827,12 @@ pub fn project_file_at_path(
     if resolver.is_hierarchical() {
         for ancestor in path.ancestors() {
             if let Some(pyproject) = settings_toml(ancestor)? {
-                let (root, settings) =
-                    resolve_scoped_settings(&pyproject, transformer, ConfigurationOrigin::Unknown)?;
+                let (root, settings) = resolve_scoped_settings(
+                    &pyproject,
+                    transformer,
+                    ConfigurationOrigin::Unknown,
+                    None,
+                )?;
                 resolver.add(&root, settings, pyproject);
                 break;
             }