perf(fmt): reuse compiled gitignore rules and release traversal state (#590)

Author: chenjiahan

crates/rstack-binding/src/lib.rs

@@ -137,6 +137,12 @@ impl GitIgnoreMatcher {
         Self::default()
     }
 
+    /// Releases compiled rules and matching state after directory traversal.
+    #[napi]
+    pub fn clear(&mut self) {
+        self.inner.clear();
+    }
+
     /// Compiles or replaces rules rooted at a repository-relative POSIX directory.
     #[napi]
     pub fn add_source(&mut self, relative_root: String, patterns: String) -> napi::Result<bool> {

crates/rstack-ignore/src/lib.rs

@@ -1,6 +1,6 @@
 #![deny(clippy::all)]
 
-use std::{borrow::Cow, collections::HashMap, path::Path, path::PathBuf};
+use std::{borrow::Cow, collections::HashMap, path::Path, path::PathBuf, sync::Arc};
 
 use ignore::{
     gitignore::{Gitignore, GitignoreBuilder},
@@ -138,6 +138,7 @@ impl IgnoreMatcher {
 /// A hierarchy of repository `.gitignore` files keyed by their root-relative directories.
 #[derive(Default)]
 pub struct GitIgnoreMatcher {
+    compiled_patterns: HashMap<Box<str>, Arc<Gitignore>>,
     matchers: HashMap<Box<str>, GitIgnoreSourceMatcher>,
     // Traversal checks every file's parent, so cache both ignored and included directories.
     ignored_directories: HashMap<Box<str>, bool>,
@@ -149,6 +150,11 @@ impl GitIgnoreMatcher {
         Self::default()
     }
 
+    /// Releases compiled rules and matching state after directory traversal.
+    pub fn clear(&mut self) {
+        *self = Self::default();
+    }
+
     /// Adds or replaces the patterns rooted at a POSIX, repository-relative directory.
     ///
     /// Returns whether the hierarchy contains any effective rules after the update.
@@ -158,7 +164,15 @@ impl GitIgnoreMatcher {
         patterns: &str,
     ) -> Result<bool, ignore::Error> {
         let relative_root = normalize_relative_path(relative_root);
-        let matcher = compile_patterns(patterns)?;
+        // Compiled rules use relative paths, so identical contents can share them across roots.
+        let matcher = if let Some(matcher) = self.compiled_patterns.get(patterns) {
+            Arc::clone(matcher)
+        } else {
+            let matcher = Arc::new(compile_patterns(patterns)?);
+            self.compiled_patterns
+                .insert(patterns.into(), Arc::clone(&matcher));
+            matcher
+        };
 
         self.invalidate_directory_cache(relative_root);
         if matcher.is_empty() {
@@ -329,12 +343,12 @@ impl GitIgnoreMatcher {
 /// One `.gitignore` source with the directory state needed to reproduce
 /// `ignore.test(path)` without re-walking ancestors for every file.
 struct GitIgnoreSourceMatcher {
-    matcher: Gitignore,
+    matcher: Arc<Gitignore>,
     directory_states: HashMap<Box<str>, Option<bool>>,
 }
 
 impl GitIgnoreSourceMatcher {
-    fn new(matcher: Gitignore) -> Self {
+    fn new(matcher: Arc<Gitignore>) -> Self {
         Self {
             matcher,
             directory_states: HashMap::new(),
@@ -550,6 +564,31 @@ mod tests {
     use super::{GitIgnoreMatcher, IgnoreMatcher, IgnoreSource};
     use std::path::Path;
 
+    #[test]
+    fn identical_gitignore_contents_keep_independent_roots() {
+        let mut matcher = GitIgnoreMatcher::new();
+        let patterns = "/dist/\n";
+        matcher.add_source("a", patterns).unwrap();
+        matcher.add_source("b", patterns).unwrap();
+        for root in ["a", "b"] {
+            assert!(matcher.is_ignored(&format!("{root}/dist/index.js"), false));
+            assert!(!matcher.is_ignored(&format!("{root}/nested/dist/index.js"), false));
+        }
+        matcher.add_source("a", "").unwrap();
+        assert!(!matcher.is_ignored("a/dist/index.js", false));
+        assert!(matcher.is_ignored("b/dist/index.js", false));
+        assert!(!matcher.is_ignored("dist/index.js", false));
+    }
+
+    #[test]
+    fn clearing_gitignore_removes_rules_and_directory_state() {
+        let mut matcher = GitIgnoreMatcher::new();
+        matcher.add_source("", "dist/\n").unwrap();
+        assert!(matcher.is_ignored("dist/index.js", false));
+        matcher.clear();
+        assert!(!matcher.is_ignored("dist/index.js", false));
+    }
+
     #[test]
     fn keeps_independent_ignore_sources_isolated() {
         let mut matcher = IgnoreMatcher::new([

packages/rstack/binding.d.cts

@@ -13,6 +13,8 @@ export declare const __napiBindingTarget: 'native' | 'wasm32-wasi' | 'wasm32-was
 export declare class GitIgnoreMatcher {
   /** Creates an empty matcher whose sources can be added during directory traversal. */
   constructor()
+  /** Releases compiled rules and matching state after directory traversal. */
+  clear(): void
   /** Compiles or replaces rules rooted at a repository-relative POSIX directory. */
   addSource(relativeRoot: string, patterns: string): boolean
   /** Returns whether one repository-relative POSIX path is ignored. */

packages/rstack/src/fmt/discoverPaths.ts

@@ -115,8 +115,25 @@ class GitIgnoreFiles {
 
   static async create(cwd: string): Promise<GitIgnoreFiles> {
     const matcher = new GitIgnoreFiles(await findGitRoot(cwd));
-    await matcher.loadThrough(cwd);
-    return matcher;
+    try {
+      await matcher.loadThrough(cwd);
+      return matcher;
+    } catch (error) {
+      await matcher.waitForLoads();
+      matcher.dispose();
+      throw error;
+    }
+  }
+
+  async waitForLoads(): Promise<void> {
+    // A failed operation may leave other ignore files still loading.
+    await Promise.allSettled(this.#loads.values());
+  }
+
+  dispose(): void {
+    this.#matcher?.clear();
+    this.#matcher = undefined;
+    this.#loads.clear();
   }
 
   async loadThrough(directoryPath: string): Promise<void> {
@@ -642,53 +659,62 @@ const discoverFmtPaths = async ({
 
   if (traversalRoots.length) {
     const gitIgnore = await GitIgnoreFiles.create(cwd);
-    const results = await Promise.all(
-      traversalRoots.map(async (rootPath) => {
-        const stats = await lstatSafe(rootPath);
-        if (!stats?.isDirectory()) {
-          return [];
-        }
+    const traversals = traversalRoots.map(async (rootPath) => {
+      const stats = await lstatSafe(rootPath);
+      if (!stats?.isDirectory()) {
+        return [];
+      }
 
-        await gitIgnore.loadThrough(rootPath);
-        if (
-          gitIgnore.isIgnored(rootPath, true) ||
-          isIgnored?.(rootPath, true) === true
-        ) {
-          return [];
-        }
+      await gitIgnore.loadThrough(rootPath);
+      if (
+        gitIgnore.isIgnored(rootPath, true) ||
+        isIgnored?.(rootPath, true) === true
+      ) {
+        return [];
+      }
 
-        const includesAll = directoryRoots.some((directoryPath) =>
-          isPathInside(directoryPath, rootPath),
-        );
-        const isIncluded = includesAll
-          ? undefined
-          : (filePath: string): boolean => {
-              if (
-                directoryRoots.some((directoryPath) =>
-                  isPathInside(directoryPath, filePath),
-                )
-              ) {
-                return true;
-              }
-
-              const relativePath = toPosixPath(resolveRelativePath(filePath));
-              return globMatchers.some((matches) => matches(relativePath));
-            };
-
-        return discoverDirectoryFiles(
-          rootPath,
-          gitIgnore,
-          ignoredDirNames,
-          isIncluded,
-          isIgnored,
-        );
-      }),
-    );
+      const includesAll = directoryRoots.some((directoryPath) =>
+        isPathInside(directoryPath, rootPath),
+      );
+      const isIncluded = includesAll
+        ? undefined
+        : (filePath: string): boolean => {
+            if (
+              directoryRoots.some((directoryPath) =>
+                isPathInside(directoryPath, filePath),
+              )
+            ) {
+              return true;
+            }
+
+            const relativePath = toPosixPath(resolveRelativePath(filePath));
+            return globMatchers.some((matches) => matches(relativePath));
+          };
+
+      return discoverDirectoryFiles(
+        rootPath,
+        gitIgnore,
+        ignoredDirNames,
+        isIncluded,
+        isIgnored,
+      );
+    });
 
-    for (const files of results) {
-      for (const filePath of files) {
-        candidates.add(filePath);
+    try {
+      const results = await Promise.all(traversals);
+
+      for (const files of results) {
+        for (const filePath of files) {
+          candidates.add(filePath);
+        }
       }
+    } catch (error) {
+      // A rejected traversal must not release rules still used by another root.
+      await Promise.allSettled(traversals);
+      await gitIgnore.waitForLoads();
+      throw error;
+    } finally {
+      gitIgnore.dispose();
     }
   }
 

packages/rstack/tests/fmt/discoveryCleanup.test.ts

@@ -0,0 +1,68 @@
+import path from 'node:path';
+import { setImmediate } from 'node:timers/promises';
+import { afterEach, expect, rs, test } from 'rstack/test';
+import { discoverFmtPaths } from '../../src/fmt/discoverPaths.ts';
+import { loadNativeBinding } from '../../src/native/index.ts';
+import { withTempProject, writeProjectFile } from './helpers.ts';
+
+const mocks = rs.hoisted(() => ({ readdir: rs.fn() }));
+rs.mock('tiny-readdir', () => ({ default: mocks.readdir }));
+
+afterEach(() => {
+  rs.restoreAllMocks();
+  mocks.readdir.mockReset();
+});
+
+test('waits for pending traversals before releasing rules after a failure', async () => {
+  await withTempProject(async (rootPath) => {
+    writeProjectFile(rootPath, '.gitignore', '*.log\n');
+    const firstFile = writeProjectFile(rootPath, 'first/index.ts');
+    const secondFile = writeProjectFile(rootPath, 'second/index.ts');
+    const pending = Promise.withResolvers<{ files: string[] }>();
+    const started = Promise.withResolvers<void>();
+    const error = new Error('Traversal failed');
+    const clear = rs.spyOn(
+      loadNativeBinding().GitIgnoreMatcher.prototype,
+      'clear',
+    );
+
+    mocks.readdir.mockImplementation((directory: string) => {
+      if (directory === path.dirname(firstFile)) {
+        return Promise.reject(error);
+      }
+      started.resolve();
+      return pending.promise;
+    });
+
+    const discovery = discoverFmtPaths({
+      cwd: rootPath,
+      patterns: ['first', 'second'],
+    });
+    const completion = expect(discovery).rejects.toBe(error);
+    try {
+      await started.promise;
+      // Let a sibling rejection propagate while this traversal is still pending.
+      await setImmediate();
+      expect(clear).not.toHaveBeenCalled();
+    } finally {
+      pending.resolve({ files: [secondFile] });
+      await completion;
+    }
+    expect(clear).toHaveBeenCalledTimes(1);
+  });
+});
+
+test('releases rules when initialization fails', async () => {
+  await withTempProject(async (rootPath) => {
+    writeProjectFile(rootPath, '.gitignore', '*.log\n');
+    const prototype = loadNativeBinding().GitIgnoreMatcher.prototype;
+    const error = new Error('Invalid ignore source');
+    rs.spyOn(prototype, 'addSource').mockImplementation(() => {
+      throw error;
+    });
+    const clear = rs.spyOn(prototype, 'clear');
+
+    await expect(discoverFmtPaths({ cwd: rootPath })).rejects.toBe(error);
+    expect(clear).toHaveBeenCalledTimes(1);
+  });
+});