perf: skip expanding exclude globs (denoland#21817)

We were calling `expand_glob` on our excludes, which is very expensive
and unnecessary because we can pattern match while traversing instead.

1. Doesn't expand "exclude" globs. Instead pattern matches while walking
the directory.
2. Splits up the "include" into base paths and applicable file patterns.
This causes less pattern matching to occur because we're only pattern
matching on patterns that might match and not ones in completely
unrelated directories.

cli/args/flags.rs

@@ -34,6 +34,35 @@ pub struct FileFlags {
  pub include: Vec<PathBuf>,
 }
 
+impl FileFlags {
+ pub fn with_absolute_paths(self, base: &Path) -> Self {
+ fn to_absolute_path(path: PathBuf, base: &Path) -> PathBuf {
+ // todo(dsherret): don't store URLs in PathBufs
+ if path.starts_with("http:")
+ || path.starts_with("https:")
+ || path.starts_with("file:")
+ {
+ path
+ } else {
+ base.join(path)
+ }
+ }
+
+ Self {
+ include: self
+ .include
+ .into_iter()
+ .map(|p| to_absolute_path(p, base))
+ .collect(),
+ ignore: self
+ .ignore
+ .into_iter()
+ .map(|p| to_absolute_path(p, base))
+ .collect(),
+ }
+ }
+}
+
 #[derive(Clone, Debug, Default, Eq, PartialEq)]
 pub struct BenchFlags {
  pub files: FileFlags,

cli/args/mod.rs

@@ -69,7 +69,8 @@ use thiserror::Error;
 
 use crate::file_fetcher::FileFetcher;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
-use crate::util::glob::expand_globs;
+use crate::util::glob::FilePatterns;
+use crate::util::glob::PathOrPatternSet;
 use crate::version;
 
 use deno_config::FmtConfig;
@@ -217,7 +218,7 @@ impl CacheSetting {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct BenchOptions {
- pub files: FilesConfig,
+ pub files: FilePatterns,
  pub filter: Option<String>,
  pub json: bool,
  pub no_run: bool,
@@ -227,12 +228,14 @@ impl BenchOptions {
  pub fn resolve(
  maybe_bench_config: Option<BenchConfig>,
  maybe_bench_flags: Option<BenchFlags>,
+ initial_cwd: &Path,
  ) -> Result<Self, AnyError> {
  let bench_flags = maybe_bench_flags.unwrap_or_default();
  Ok(Self {
  files: resolve_files(
  maybe_bench_config.map(|c| c.files),
  Some(bench_flags.files),
+ initial_cwd,
  )?,
  filter: bench_flags.filter,
  json: bench_flags.json,
@@ -245,13 +248,14 @@ impl BenchOptions {
 pub struct FmtOptions {
  pub check: bool,
  pub options: FmtOptionsConfig,
- pub files: FilesConfig,
+ pub files: FilePatterns,
 }
 
 impl FmtOptions {
  pub fn resolve(
  maybe_fmt_config: Option<FmtConfig>,
  maybe_fmt_flags: Option<FmtFlags>,
+ initial_cwd: &Path,
  ) -> Result<Self, AnyError> {
  let (maybe_config_options, maybe_config_files) =
  maybe_fmt_config.map(|c| (c.options, c.files)).unzip();
@@ -265,6 +269,7 @@ impl FmtOptions {
  files: resolve_files(
  maybe_config_files,
  maybe_fmt_flags.map(|f| f.files),
+ initial_cwd,
  )?,
  })
  }
@@ -311,26 +316,9 @@ fn resolve_fmt_options(
  options
 }
 
-#[derive(Clone, Debug, Default)]
-pub struct CheckOptions {
- pub exclude: Vec<PathBuf>,
-}
-
-impl CheckOptions {
- pub fn resolve(
- maybe_files_config: Option<FilesConfig>,
- ) -> Result<Self, AnyError> {
- Ok(Self {
- exclude: expand_globs(
- maybe_files_config.map(|c| c.exclude).unwrap_or_default(),
- )?,
- })
- }
-}
-
 #[derive(Clone)]
 pub struct TestOptions {
- pub files: FilesConfig,
+ pub files: FilePatterns,
  pub doc: bool,
  pub no_run: bool,
  pub fail_fast: Option<NonZeroUsize>,
@@ -347,13 +335,15 @@ impl TestOptions {
  pub fn resolve(
  maybe_test_config: Option<TestConfig>,
  maybe_test_flags: Option<TestFlags>,
+ initial_cwd: &Path,
  ) -> Result<Self, AnyError> {
  let test_flags = maybe_test_flags.unwrap_or_default();
 
  Ok(Self {
  files: resolve_files(
  maybe_test_config.map(|c| c.files),
  Some(test_flags.files),
+ initial_cwd,
  )?,
  allow_none: test_flags.allow_none,
  concurrent_jobs: test_flags
@@ -382,14 +372,15 @@ pub enum LintReporterKind {
 #[derive(Clone, Debug, Default)]
 pub struct LintOptions {
  pub rules: LintRulesConfig,
- pub files: FilesConfig,
+ pub files: FilePatterns,
  pub reporter_kind: LintReporterKind,
 }
 
 impl LintOptions {
  pub fn resolve(
  maybe_lint_config: Option<LintConfig>,
  maybe_lint_flags: Option<LintFlags>,
+ initial_cwd: &Path,
  ) -> Result<Self, AnyError> {
  let mut maybe_reporter_kind =
  maybe_lint_flags.as_ref().and_then(|lint_flags| {
@@ -437,7 +428,11 @@ impl LintOptions {
  maybe_lint_config.map(|c| (c.files, c.rules)).unzip();
  Ok(Self {
  reporter_kind: maybe_reporter_kind.unwrap_or_default(),
- files: resolve_files(maybe_config_files, Some(maybe_file_flags))?,
+ files: resolve_files(
+ maybe_config_files,
+ Some(maybe_file_flags),
+ initial_cwd,
+ )?,
  rules: resolve_lint_rules_options(
  maybe_config_rules,
  maybe_rules_tags,
@@ -1184,7 +1179,7 @@ impl CliOptions {
  } else {
  None
  };
- FmtOptions::resolve(maybe_fmt_config, Some(fmt_flags))
+ FmtOptions::resolve(maybe_fmt_config, Some(fmt_flags), &self.initial_cwd)
  }
 
  pub fn resolve_lint_options(
@@ -1196,17 +1191,20 @@ impl CliOptions {
  } else {
  None
  };
- LintOptions::resolve(maybe_lint_config, Some(lint_flags))
+ LintOptions::resolve(maybe_lint_config, Some(lint_flags), &self.initial_cwd)
  }
 
- pub fn resolve_check_options(&self) -> Result<CheckOptions, AnyError> {
+ pub fn resolve_config_excludes(&self) -> Result<PathOrPatternSet, AnyError> {
  let maybe_files_config = if let Some(config_file) = &self.maybe_config_file
  {
  config_file.to_files_config()?
  } else {
  None
  };
- CheckOptions::resolve(maybe_files_config)
+ PathOrPatternSet::from_absolute_paths(
+ maybe_files_config.map(|c| c.exclude).unwrap_or_default(),
+ )
+ .context("Invalid config file exclude pattern.")
  }
 
  pub fn resolve_test_options(
@@ -1218,7 +1216,7 @@ impl CliOptions {
  } else {
  None
  };
- TestOptions::resolve(maybe_test_config, Some(test_flags))
+ TestOptions::resolve(maybe_test_config, Some(test_flags), &self.initial_cwd)
  }
 
  pub fn resolve_bench_options(
@@ -1231,7 +1229,11 @@ impl CliOptions {
  } else {
  None
  };
- BenchOptions::resolve(maybe_bench_config, Some(bench_flags))
+ BenchOptions::resolve(
+ maybe_bench_config,
+ Some(bench_flags),
+ &self.initial_cwd,
+ )
  }
 
  /// Vector of user script CLI arguments.
@@ -1655,24 +1657,29 @@ impl StorageKeyResolver {
 fn resolve_files(
  maybe_files_config: Option<FilesConfig>,
  maybe_file_flags: Option<FileFlags>,
-) -> Result<FilesConfig, AnyError> {
- let mut result = maybe_files_config.unwrap_or_default();
+ initial_cwd: &Path,
+) -> Result<FilePatterns, AnyError> {
+ let mut maybe_files_config = maybe_files_config.unwrap_or_default();
  if let Some(file_flags) = maybe_file_flags {
+ let file_flags = file_flags.with_absolute_paths(initial_cwd);
  if !file_flags.include.is_empty() {
- result.include = Some(file_flags.include);
+ maybe_files_config.include = Some(file_flags.include);
  }
  if !file_flags.ignore.is_empty() {
- result.exclude = file_flags.ignore;
+ maybe_files_config.exclude = file_flags.ignore
  }
  }
- // Now expand globs if there are any
- result.include = match result.include {
- Some(include) => Some(expand_globs(include)?),
- None => None,
- };
- result.exclude = expand_globs(result.exclude)?;
-
- Ok(result)
+ Ok(FilePatterns {
+ include: {
+ let files = match maybe_files_config.include {
+ Some(include) => include,
+ None => vec![initial_cwd.to_path_buf()],
+ };
+ Some(PathOrPatternSet::from_absolute_paths(files)?)
+ },
+ exclude: PathOrPatternSet::from_absolute_paths(maybe_files_config.exclude)
+ .context("Invalid exclude.")?,
+ })
 }
 
 /// Resolves the no_prompt value based on the cli flags and environment.
@@ -1694,6 +1701,8 @@ pub fn npm_pkg_req_ref_to_binary_command(
 
 #[cfg(test)]
 mod test {
+ use crate::util::fs::FileCollector;
+
  use super::*;
  use pretty_assertions::assert_eq;
 
@@ -1887,6 +1896,7 @@ mod test {
  exclude: vec![],
  }),
  None,
+ temp_dir_path,
  )
  .unwrap_err();
  assert!(error.to_string().starts_with("Failed to expand glob"));
@@ -1902,32 +1912,36 @@ mod test {
  exclude: vec![temp_dir_path.join("nested/**/*bazz.ts")],
  }),
  None,
+ temp_dir_path,
  )
  .unwrap();
 
+ let mut files = FileCollector::new(|_, _| true)
+ .ignore_git_folder()
+ .ignore_node_modules()
+ .ignore_vendor_folder()
+ .collect_file_patterns(resolved_files)
+ .unwrap();
+
+ files.sort();
+
  assert_eq!(
- resolved_files.include,
- Some(vec![
- temp_dir_path.join("data/test1.js"),
- temp_dir_path.join("data/test1.ts"),
- temp_dir_path.join("nested/foo/bar.ts"),
- temp_dir_path.join("nested/foo/bazz.ts"),
- temp_dir_path.join("nested/foo/fizz.ts"),
- temp_dir_path.join("nested/foo/foo.ts"),
- temp_dir_path.join("nested/fizz/bar.ts"),
- temp_dir_path.join("nested/fizz/bazz.ts"),
- temp_dir_path.join("nested/fizz/fizz.ts"),
- temp_dir_path.join("nested/fizz/foo.ts"),
- temp_dir_path.join("pages/[id].ts"),
- ])
- );
- assert_eq!(
- resolved_files.exclude,
+ files,
  vec![
- temp_dir_path.join("nested/fizz/bazz.ts"),
- temp_dir_path.join("nested/foo/bazz.ts"),
+ "data/test1.js",
+ "data/test1.ts",
+ "nested/fizz/bar.ts",
+ "nested/fizz/fizz.ts",
+ "nested/fizz/foo.ts",
+ "nested/foo/bar.ts",
+ "nested/foo/fizz.ts",
+ "nested/foo/foo.ts",
+ "pages/[id].ts",
  ]
- )
+ .into_iter()
+ .map(|p| normalize_path(temp_dir_path.join(p)))
+ .collect::<Vec<_>>()
+ );
  }
 
  #[test]

cli/graph_util.rs

@@ -16,6 +16,8 @@ use crate::resolver::SloppyImportsResolver;
 use crate::tools::check;
 use crate::tools::check::TypeChecker;
 use crate::util::file_watcher::WatcherCommunicator;
+use crate::util::fs::canonicalize_path;
+use crate::util::path::specifier_to_file_path;
 use crate::util::sync::TaskQueue;
 use crate::util::sync::TaskQueuePermit;
 
@@ -677,7 +679,7 @@ impl ModuleGraphContainer {
 pub fn has_graph_root_local_dependent_changed(
  graph: &ModuleGraph,
  root: &ModuleSpecifier,
- changed_specifiers: &HashSet<ModuleSpecifier>,
+ canonicalized_changed_paths: &HashSet<PathBuf>,
 ) -> bool {
  let roots = vec![root.clone()];
  let mut dependent_specifiers = graph.walk(
@@ -689,11 +691,15 @@ pub fn has_graph_root_local_dependent_changed(
  },
  );
  while let Some((s, _)) = dependent_specifiers.next() {
- if s.scheme() != "file" {
+ if let Ok(path) = specifier_to_file_path(s) {
+ if let Ok(path) = canonicalize_path(&path) {
+ if canonicalized_changed_paths.contains(&path) {
+ return true;
+ }
+ }
+ } else {
  // skip walking this remote module's dependencies
  dependent_specifiers.skip_previous_dependencies();
- } else if changed_specifiers.contains(s) {
- return true;
  }
  }
  false