From 8b2144739bc9a2101649bd524f9176483e373796 Mon Sep 17 00:00:00 2001 From: alexander-akait <4567934+alexander-akait@users.noreply.github.com> Date: Mon, 14 Sep 2026 13:03:40 +0000 Subject: [PATCH] perf: walk the file system off the thread webpack builds on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectFromFileSystem` walked the whole tree with `globSync` from inside the `compilation` hook, so every compilation stopped the loop for as long as the walk took — 6.0 ms over a thousand files, and the walk is made again on every rebuild. `canWatch` read each candidate directory with `readdirSync`, and `parseFoldersToGlobs` stated each pattern, both from places that can wait. All three are asynchronous now. Two compilers asking for the same walk at the same moment share it rather than reading the tree twice, which is also what leaves their checks a run to share. Over a thousand files the longest stall in a watch rebuild falls from a median of 10.2 ms to 5.8 ms, measured over nine rebuilds twice. What stays synchronous is TypeScript's: its host is called for a source file and waits for one, so `versionOf` and `ts.sys` have nothing to await into — and since the check moved to a worker that is not this loop. --- .../nothing-sync-on-the-build-thread.md | 5 + src/checks/typescript-program.js | 2 + src/index.js | 134 ++++++++++++------ src/utils.js | 47 +++--- test/utils.test.js | 28 ++-- types/index.d.ts | 4 +- types/utils.d.ts | 4 +- 7 files changed, 146 insertions(+), 78 deletions(-) create mode 100644 .changeset/nothing-sync-on-the-build-thread.md diff --git a/.changeset/nothing-sync-on-the-build-thread.md b/.changeset/nothing-sync-on-the-build-thread.md new file mode 100644 index 00000000..f22e6842 --- /dev/null +++ b/.changeset/nothing-sync-on-the-build-thread.md @@ -0,0 +1,5 @@ +--- +"diagnostics-webpack-plugin": patch +--- + +Walk the file system for a check's files without blocking the thread webpack builds on, and read the tree once where two compilers want the same walk. diff --git a/src/checks/typescript-program.js b/src/checks/typescript-program.js index 08f03d4f..feadc50f 100644 --- a/src/checks/typescript-program.js +++ b/src/checks/typescript-program.js @@ -63,6 +63,8 @@ function getHeld(compiler, id) { } /** + * Read synchronously because TypeScript's own host is: the compiler calls + * `getSourceFile` and waits for a source file, with nothing to await into. * @param {string} file the file to read the state of * @returns {string | undefined} what tells one write of it from the next */ diff --git a/src/index.js b/src/index.js index bae584b6..a51c0122 100644 --- a/src/index.js +++ b/src/index.js @@ -1,9 +1,10 @@ -import { readdirSync } from "node:fs"; +import { readdir } from "node:fs/promises"; import { isAbsolute, join, normalize, relative } from "node:path"; import picomatch from "picomatch"; -import { globSync } from "tinyglobby"; +import { glob } from "tinyglobby"; +import DiagnosticError from "./DiagnosticError.js"; import createCheckRunner from "./check.js"; import { getOptions, reportedAs, validateOptions } from "./options.js"; import { @@ -46,22 +47,45 @@ const EARLY_BATCH = 64; let compilerId = 0; +// A walk two compilers are making at the same moment over the same patterns, +// so that the tree is read once and both of them go on together — which is +// what leaves their checks a run to share. +/** @type {Map>} */ +const walks = new Map(); + +/** + * @param {string[]} wanted the globs of the files to walk for + * @param {string[]} exclude the globs of the files to leave out + * @returns {Promise} what the tree holds of them + */ +function walk(wanted, exclude) { + const key = JSON.stringify([wanted, exclude]); + const running = walks.get(key); + + if (running) return running; + + const started = glob(wanted, { absolute: true, dot: true, ignore: exclude }); + const forget = () => walks.delete(key); + + walks.set(key, started); + started.then(forget, forget); + + return started; +} + /** * Walks the file system for the files a check wants, and says which of them * webpack has just seen change. * @param {Compiler} compiler compiler * @param {ResolvedCheck} check the check to collect the files of - * @returns {{ lint: string[], keep: string[] }} the files to lint, and the ones to report from the last compilation + * @returns {Promise<{ lint: string[], keep: string[] }>} the files to lint, and the ones to report from the last compilation */ -function collectFromFileSystem(compiler, { adapter, wanted, exclude }) { +async function collectFromFileSystem(compiler, { adapter, wanted, exclude }) { + // Read before the walk, which is what the build goes on building through. + const { modifiedFiles } = compiler; // The walk is what says which files there are: one webpack never built is // one it cannot report as added, changed or gone either. - const found = globSync(wanted, { - absolute: true, - dot: true, - ignore: exclude, - }); - const { modifiedFiles } = compiler; + const found = await walk(wanted, exclude); // A check that cannot say which file a result came from has nothing to report // a file it was not given from, so it is given all of them every time. One @@ -132,15 +156,15 @@ function globRoots({ filesSource, wanted }) { * @param {Compiler} compiler compiler * @param {ResolvedCheck} check the check that reads it * @param {string[]} writes the paths the check itself writes - * @returns {boolean} whether the whole of it can be watched + * @returns {Promise} whether the whole of it can be watched */ -function canWatch(directory, compiler, check, writes) { +async function canWatch(directory, compiler, check, writes) { const written = [compiler.outputPath, ...writes]; if (written.some((path) => contains(directory, path))) return false; try { - for (const entry of readdirSync(directory, { withFileTypes: true })) { + for (const entry of await readdir(directory, { withFileTypes: true })) { if ( entry.isDirectory() && check.isExcluded(join(directory, entry.name)) @@ -199,7 +223,7 @@ class DiagnosticsWebpackPlugin { validateOptions(compiler, this.given, this.options.checks); }); - /** @type {ResolvedCheck[] | undefined} */ + /** @type {Promise | undefined} */ let checks; // Resolved on the first build rather than here, so that an option the @@ -208,8 +232,10 @@ class DiagnosticsWebpackPlugin { if (!checks) { const context = this.getContext(compiler); - checks = this.options.checks.map((check) => - this.resolveCheck(compiler, context, check), + checks = Promise.all( + this.options.checks.map((check) => + this.resolveCheck(compiler, context, check), + ), ); } @@ -218,22 +244,22 @@ class DiagnosticsWebpackPlugin { // A build is nothing but a first compilation, so `lintOnStart` cannot // silence one without silencing the plugin. - compiler.hooks.run.tapPromise(this.key, (compiler) => - this.run(compiler, getChecks()), + compiler.hooks.run.tapPromise(this.key, async (compiler) => + this.run(compiler, await getChecks()), ); // A lint integration whose bundler reaches a file only once something // requests it defaults this off; webpack's first build walks all of them. let skipping = !this.options.lintOnStart; - compiler.hooks.watchRun.tapPromise(this.key, (compiler) => { + compiler.hooks.watchRun.tapPromise(this.key, async (compiler) => { if (skipping) { skipping = false; - return Promise.resolve(); + return; } - return this.run(compiler, getChecks()); + return this.run(compiler, await getChecks()); }); } @@ -241,9 +267,9 @@ class DiagnosticsWebpackPlugin { * @param {Compiler} compiler compiler * @param {string} context context * @param {EnabledCheck} check the check to resolve the globs of - * @returns {ResolvedCheck} the check with its globs resolved + * @returns {Promise} the check with its globs resolved */ - resolveCheck(compiler, context, { id, name, adapter, options }) { + async resolveCheck(compiler, context, { id, name, adapter, options }) { const resourceQueries = arrify(options.resourceQueryExclude || []); /** @type {CheckOptions} */ @@ -265,13 +291,13 @@ class DiagnosticsWebpackPlugin { ), }; - const wanted = parseFoldersToGlobs( - /** @type {string[]} */ (resolved.files), - resolved.extensions, - ); - const exclude = parseFoldersToGlobs( - /** @type {string[]} */ (resolved.exclude), - ); + const [wanted, exclude] = await Promise.all([ + parseFoldersToGlobs( + /** @type {string[]} */ (resolved.files), + resolved.extensions, + ), + parseFoldersToGlobs(/** @type {string[]} */ (resolved.exclude)), + ]); return { id, @@ -413,6 +439,9 @@ class DiagnosticsWebpackPlugin { handOver, late, runner, + // The walk this check's files come from, for the ones that walk. + /** @type {Promise | undefined} */ + collecting: undefined, }; }); @@ -468,15 +497,23 @@ class DiagnosticsWebpackPlugin { } } - // Nothing globbed from the file system waits on the module graph. + // Nothing globbed from the file system waits on the module graph, and + // the walk itself is what webpack goes on building through. for (const check of runners) { if (check.filesSource === "modules") continue; - const collected = collectFromFileSystem(compiler, check); - - check.pending.push(...collected.lint); - check.kept.push(...collected.keep); - check.flush(true); + check.collecting = collectFromFileSystem(compiler, check).then( + (collected) => { + check.pending.push(...collected.lint); + check.kept.push(...collected.keep); + check.flush(true); + }, + (err) => { + compilation.errors.push( + new DiagnosticError(check.name, err.message), + ); + }, + ); } compilation.hooks.finishModules.tap(this.key, () => { @@ -487,6 +524,10 @@ class DiagnosticsWebpackPlugin { compilation.hooks.processAssets.tapAsync( this.key, async (_, callback) => { + // Every walk is done by here, so a check has the files it covers + // before it is asked what it found in them. + await Promise.all(runners.map((check) => check.collecting)); + /** @type {Map} */ const outputReports = new Map(); @@ -495,8 +536,12 @@ class DiagnosticsWebpackPlugin { * configured to, which is not always the same set of files. * @param {ResolvedCheck} check the check that read them * @param {Dependencies} dependencies what a run of it read + * @returns {Promise} when the watcher has been told */ - const watch = (check, { read, missing, directories, writes }) => { + const watch = async ( + check, + { read, missing, directories, writes }, + ) => { // Each one is spelled the way the platform does: a watcher looks a // change up under the path it joined, not the one it was given. for (const file of read) { @@ -513,9 +558,13 @@ class DiagnosticsWebpackPlugin { // A file that does not exist yet is under no watch of its own, so // the directory a check would find it in answers for it. - const roots = [...globRoots(check), ...directories].filter( - (directory) => canWatch(directory, compiler, check, writes), + const candidates = [...globRoots(check), ...directories]; + const watchable = await Promise.all( + candidates.map((directory) => + canWatch(directory, compiler, check, writes), + ), ); + const roots = candidates.filter((_, at) => watchable[at]); for (const directory of roots) { // Watching a directory covers what is under it, so one inside @@ -540,7 +589,10 @@ class DiagnosticsWebpackPlugin { // reads this time is not known until it is done, by which point // the watcher has been handed its list. if (check.late) { - watch(check, /** @type {Dependencies} */ (readLast.get(id))); + await watch( + check, + /** @type {Dependencies} */ (readLast.get(id)), + ); afterBuild.push(() => { check.handOver(); @@ -562,7 +614,7 @@ class DiagnosticsWebpackPlugin { } = report; readLast.set(id, { read, missing, directories, writes }); - watch(check, report); + await watch(check, report); // `reportAs` has already dropped whatever it reports as `false`, // so what is left only needs putting where it belongs. diff --git a/src/utils.js b/src/utils.js index a52a6021..47600b74 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,7 +1,7 @@ // eslint-disable-next-line jsdoc/reject-any-type /** @typedef {any} EXPECTED_ANY */ -import { statSync } from "node:fs"; +import { stat } from "node:fs/promises"; import { createRequire } from "node:module"; import { dirname, isAbsolute, resolve } from "node:path"; @@ -96,9 +96,9 @@ function parseFiles(files, context) { /** * @param {string | string[]} patterns patterns * @param {string | string[]} extensions extensions - * @returns {string[]} globs + * @returns {Promise} globs */ -function parseFoldersToGlobs(patterns, extensions = []) { +async function parseFoldersToGlobs(patterns, extensions = []) { const extensionsList = arrify(extensions); const [prefix, postfix] = extensionsList.length > 1 ? ["{", "}"] : ["", ""]; const extensionsGlob = extensionsList @@ -115,24 +115,29 @@ function parseFoldersToGlobs(patterns, extensions = []) { `/**${extensionsGlob ? `/*.${prefix + extensionsGlob + postfix}` : ""}`, ); - return arrify(patterns).flatMap((/** @type {string} */ pattern) => { - try { - // The patterns are absolute because they are prepended with the context. - // A folder is read as one whatever its name is made of, so this is asked - // before the pattern is: `[symbols]` is a directory and a character class. - if (statSync(pattern).isDirectory()) return asFolder(pattern); - - return pattern; - } catch { - // A glob already says what it covers, whether or not anything is there. - if (picomatch.scan(pattern).isGlob) return pattern; - - // A path that is not there yet is one the build may go on to write, and - // the globs are read once. Naming it both ways is what covers a folder - // that appears later without missing a file of the same name. - return [pattern, asFolder(pattern)]; - } - }); + const read = await Promise.all( + arrify(patterns).map(async (/** @type {string} */ pattern) => { + try { + // The patterns are absolute because they are prepended with the + // context. A folder is read as one whatever its name is made of, so + // this is asked before the pattern is: `[symbols]` is a directory and + // a character class. + if ((await stat(pattern)).isDirectory()) return asFolder(pattern); + + return pattern; + } catch { + // A glob already says what it covers, whether or not anything is there. + if (picomatch.scan(pattern).isGlob) return pattern; + + // A path that is not there yet is one the build may go on to write, and + // the globs are read once. Naming it both ways is what covers a folder + // that appears later without missing a file of the same name. + return [pattern, asFolder(pattern)]; + } + }), + ); + + return read.flat(); } /** diff --git a/test/utils.test.js b/test/utils.test.js index df909bba..dc6e570a 100644 --- a/test/utils.test.js +++ b/test/utils.test.js @@ -9,7 +9,7 @@ import { toPosixPath, } from "../src/utils.js"; -// `parseFoldersToGlobs` stats what it is given, so the fixtures have to exist. +// `parseFoldersToGlobs` reads what it is given, so the fixtures have to exist. const directory = join(import.meta.dirname, "fixtures"); const file = join(import.meta.dirname, "fixtures", "good.js"); @@ -51,16 +51,16 @@ describe("utils", () => { assert.ok(packageB.endsWith("main/package-b/src/**")); }); - it("parseFoldersToGlobs should return globs for folders", () => { - assert.deepStrictEqual(parseFoldersToGlobs(directory, "js"), [ + it("parseFoldersToGlobs should return globs for folders", async () => { + assert.deepStrictEqual(await parseFoldersToGlobs(directory, "js"), [ `${directory}/**/*.js`, ]); - assert.deepStrictEqual(parseFoldersToGlobs(`${directory}/`, "js"), [ + assert.deepStrictEqual(await parseFoldersToGlobs(`${directory}/`, "js"), [ `${directory}/**/*.js`, ]); assert.deepStrictEqual( - parseFoldersToGlobs( + await parseFoldersToGlobs( [directory, `${directory}/`, file], ["js", "cjs", "mjs"], ), @@ -71,26 +71,30 @@ describe("utils", () => { ], ); - assert.deepStrictEqual(parseFoldersToGlobs(directory), [`${directory}/**`]); - assert.deepStrictEqual(parseFoldersToGlobs(`${directory}/`), [ + assert.deepStrictEqual(await parseFoldersToGlobs(directory), [ + `${directory}/**`, + ]); + assert.deepStrictEqual(await parseFoldersToGlobs(`${directory}/`), [ `${directory}/**`, ]); }); - it("parseFoldersToGlobs should return unmodified globs for globs (ignoring extensions)", () => { - assert.deepStrictEqual(parseFoldersToGlobs("**.notjs", "js"), ["**.notjs"]); + it("parseFoldersToGlobs should return unmodified globs for globs (ignoring extensions)", async () => { + assert.deepStrictEqual(await parseFoldersToGlobs("**.notjs", "js"), [ + "**.notjs", + ]); }); - it("parseFoldersToGlobs should cover a path that is not there yet both ways", () => { + it("parseFoldersToGlobs should cover a path that is not there yet both ways", async () => { const absent = join(directory, "not-written-yet"); // Nothing says whether a path the build has still to write is a file or a // folder, and the globs are read once. - assert.deepStrictEqual(parseFoldersToGlobs(absent, "js"), [ + assert.deepStrictEqual(await parseFoldersToGlobs(absent, "js"), [ absent, `${absent}/**/*.js`, ]); - assert.deepStrictEqual(parseFoldersToGlobs(absent), [ + assert.deepStrictEqual(await parseFoldersToGlobs(absent), [ absent, `${absent}/**`, ]); diff --git a/types/index.d.ts b/types/index.d.ts index 2a269935..4bf69698 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -69,13 +69,13 @@ declare class DiagnosticsWebpackPlugin { * @param {Compiler} compiler compiler * @param {string} context context * @param {EnabledCheck} check the check to resolve the globs of - * @returns {ResolvedCheck} the check with its globs resolved + * @returns {Promise} the check with its globs resolved */ resolveCheck( compiler: Compiler, context: string, { id, name, adapter, options }: EnabledCheck, - ): ResolvedCheck; + ): Promise; /** * @param {ResolvedCheck} check the check to create a runner for * @param {Compilation} compilation compilation diff --git a/types/utils.d.ts b/types/utils.d.ts index ea6e284f..128428b0 100644 --- a/types/utils.d.ts +++ b/types/utils.d.ts @@ -73,12 +73,12 @@ export function parseFiles(files: string | string[], context: string): string[]; /** * @param {string | string[]} patterns patterns * @param {string | string[]} extensions extensions - * @returns {string[]} globs + * @returns {Promise} globs */ export function parseFoldersToGlobs( patterns: string | string[], extensions?: string | string[], -): string[]; +): Promise; /** * Globs only know the forward slash, so a path is compared and matched as one. * @param {string} file a path