From db53640552791b830cbde96db3eb3bd4cb7fe4c8 Mon Sep 17 00:00:00 2001 From: neverland Date: Thu, 22 Oct 2026 08:49:14 +0800 Subject: [PATCH 1/4] perf(fmt): reuse compiled gitignore rules and release traversal state --- crates/rstack-binding/src/lib.rs | 6 + crates/rstack-ignore/src/lib.rs | 52 +++++++- packages/rstack/binding.d.cts | 2 + packages/rstack/src/fmt/discoverPaths.ts | 114 +++++++++++------- .../rstack/tests/fmt/discoveryCleanup.test.ts | 75 ++++++++++++ 5 files changed, 200 insertions(+), 49 deletions(-) create mode 100644 packages/rstack/tests/fmt/discoveryCleanup.test.ts diff --git a/crates/rstack-binding/src/lib.rs b/crates/rstack-binding/src/lib.rs index 68596e1c..1dda092f 100644 --- a/crates/rstack-binding/src/lib.rs +++ b/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 { diff --git a/crates/rstack-ignore/src/lib.rs b/crates/rstack-ignore/src/lib.rs index 8a81a500..98047c2a 100644 --- a/crates/rstack-ignore/src/lib.rs +++ b/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, Arc>, matchers: HashMap, GitIgnoreSourceMatcher>, // Traversal checks every file's parent, so cache both ignored and included directories. ignored_directories: HashMap, 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 { 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, directory_states: HashMap, Option>, } impl GitIgnoreSourceMatcher { - fn new(matcher: Gitignore) -> Self { + fn new(matcher: Arc) -> Self { Self { matcher, directory_states: HashMap::new(), @@ -550,6 +564,36 @@ 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*.log\n!keep.log\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)); + assert!(!matcher.is_ignored(&format!("{root}/keep.log"), 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(); + matcher.clear(); + assert!(!matcher.is_ignored("dist/index.js", false)); + matcher.add_source("", "*.log\n").unwrap(); + assert!(!matcher.is_ignored("dist/index.js", false)); + assert!(matcher.is_ignored("debug.log", false)); + } + #[test] fn keeps_independent_ignore_sources_isolated() { let mut matcher = IgnoreMatcher::new([ diff --git a/packages/rstack/binding.d.cts b/packages/rstack/binding.d.cts index 86d74c88..f870cac4 100644 --- a/packages/rstack/binding.d.cts +++ b/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. */ diff --git a/packages/rstack/src/fmt/discoverPaths.ts b/packages/rstack/src/fmt/discoverPaths.ts index 9381d275..18209643 100644 --- a/packages/rstack/src/fmt/discoverPaths.ts +++ b/packages/rstack/src/fmt/discoverPaths.ts @@ -115,8 +115,22 @@ class GitIgnoreFiles { static async create(cwd: string): Promise { const matcher = new GitIgnoreFiles(await findGitRoot(cwd)); - await matcher.loadThrough(cwd); - return matcher; + try { + await matcher.loadThrough(cwd); + return matcher; + } catch (error) { + await matcher.dispose(); + throw error; + } + } + + async dispose(): Promise { + // Initialization may reject while other ignore files are still loading. + await Promise.allSettled(this.#loads.values()); + this.#matcher?.clear(); + this.#matcher = undefined; + this.#loads.clear(); + this.#hasRules = false; } async loadThrough(directoryPath: string): Promise { @@ -642,53 +656,63 @@ 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, + ); + }); + + try { + // A rejected traversal must not release rules still used by another root. + const results = await Promise.all(traversals).catch( + async (error: unknown) => { + await Promise.allSettled(traversals); + throw error; + }, + ); - for (const files of results) { - for (const filePath of files) { - candidates.add(filePath); + for (const files of results) { + for (const filePath of files) { + candidates.add(filePath); + } } + } finally { + await gitIgnore.dispose(); } } diff --git a/packages/rstack/tests/fmt/discoveryCleanup.test.ts b/packages/rstack/tests/fmt/discoveryCleanup.test.ts new file mode 100644 index 00000000..cbe8e66d --- /dev/null +++ b/packages/rstack/tests/fmt/discoveryCleanup.test.ts @@ -0,0 +1,75 @@ +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.each([false, true])( + 'releases rules only after all traversals settle (failure: %s)', + async (fail) => { + 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(); + 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 fail + ? Promise.reject(error) + : Promise.resolve({ files: [firstFile] }); + } + started.resolve(); + return pending.promise; + }); + + const discovery = discoverFmtPaths({ + cwd: rootPath, + patterns: ['first', 'second'], + }); + const completion = fail + ? expect(discovery).rejects.toBe(error) + : expect(discovery).resolves.toEqual([firstFile, secondFile]); + 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); + }); +}); From 627d9d6821f4c2d29490cf8899dd3d10db6bb5d2 Mon Sep 17 00:00:00 2001 From: neverland Date: Thu, 22 Oct 2026 10:42:33 +0800 Subject: [PATCH 2/4] test(fmt): simplify gitignore reuse and cleanup coverage --- crates/rstack-ignore/src/lib.rs | 7 +- .../rstack/tests/fmt/discoveryCleanup.test.ts | 77 +++++++++---------- 2 files changed, 36 insertions(+), 48 deletions(-) diff --git a/crates/rstack-ignore/src/lib.rs b/crates/rstack-ignore/src/lib.rs index 98047c2a..d2558e0e 100644 --- a/crates/rstack-ignore/src/lib.rs +++ b/crates/rstack-ignore/src/lib.rs @@ -567,13 +567,12 @@ mod tests { #[test] fn identical_gitignore_contents_keep_independent_roots() { let mut matcher = GitIgnoreMatcher::new(); - let patterns = "/dist/\n*.log\n!keep.log\n"; + 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)); - assert!(!matcher.is_ignored(&format!("{root}/keep.log"), false)); } matcher.add_source("a", "").unwrap(); assert!(!matcher.is_ignored("a/dist/index.js", false)); @@ -587,11 +586,7 @@ mod tests { matcher.add_source("", "dist/\n").unwrap(); assert!(matcher.is_ignored("dist/index.js", false)); matcher.clear(); - matcher.clear(); - assert!(!matcher.is_ignored("dist/index.js", false)); - matcher.add_source("", "*.log\n").unwrap(); assert!(!matcher.is_ignored("dist/index.js", false)); - assert!(matcher.is_ignored("debug.log", false)); } #[test] diff --git a/packages/rstack/tests/fmt/discoveryCleanup.test.ts b/packages/rstack/tests/fmt/discoveryCleanup.test.ts index cbe8e66d..48170196 100644 --- a/packages/rstack/tests/fmt/discoveryCleanup.test.ts +++ b/packages/rstack/tests/fmt/discoveryCleanup.test.ts @@ -13,51 +13,44 @@ afterEach(() => { mocks.readdir.mockReset(); }); -test.each([false, true])( - 'releases rules only after all traversals settle (failure: %s)', - async (fail) => { - 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(); - 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 fail - ? Promise.reject(error) - : Promise.resolve({ files: [firstFile] }); - } - started.resolve(); - return pending.promise; - }); +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(); + const error = new Error('Traversal failed'); + const clear = rs.spyOn( + loadNativeBinding().GitIgnoreMatcher.prototype, + 'clear', + ); - const discovery = discoverFmtPaths({ - cwd: rootPath, - patterns: ['first', 'second'], - }); - const completion = fail - ? expect(discovery).rejects.toBe(error) - : expect(discovery).resolves.toEqual([firstFile, secondFile]); - 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; + mocks.readdir.mockImplementation((directory: string) => { + if (directory === path.dirname(firstFile)) { + return Promise.reject(error); } - expect(clear).toHaveBeenCalledTimes(1); + 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) => { From 2c65523e14c6e4f7d0d5cecab00ffcfd0e839c62 Mon Sep 17 00:00:00 2001 From: neverland Date: Thu, 22 Oct 2026 10:48:17 +0800 Subject: [PATCH 3/4] perf(fmt): skip settled load draining after successful traversal --- packages/rstack/src/fmt/discoverPaths.ts | 26 +++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/rstack/src/fmt/discoverPaths.ts b/packages/rstack/src/fmt/discoverPaths.ts index 18209643..d4c85b60 100644 --- a/packages/rstack/src/fmt/discoverPaths.ts +++ b/packages/rstack/src/fmt/discoverPaths.ts @@ -119,18 +119,21 @@ class GitIgnoreFiles { await matcher.loadThrough(cwd); return matcher; } catch (error) { - await matcher.dispose(); + await matcher.waitForLoads(); + matcher.dispose(); throw error; } } - async dispose(): Promise { - // Initialization may reject while other ignore files are still loading. + async waitForLoads(): Promise { + // 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(); - this.#hasRules = false; } async loadThrough(directoryPath: string): Promise { @@ -698,21 +701,20 @@ const discoverFmtPaths = async ({ }); try { - // A rejected traversal must not release rules still used by another root. - const results = await Promise.all(traversals).catch( - async (error: unknown) => { - await Promise.allSettled(traversals); - throw error; - }, - ); + 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 { - await gitIgnore.dispose(); + gitIgnore.dispose(); } } From 0a5e53a873b0a7828da80d7847e399ab9b7f87e3 Mon Sep 17 00:00:00 2001 From: neverland Date: Thu, 22 Oct 2026 11:01:20 +0800 Subject: [PATCH 4/4] chore: retry CI