diff --git a/.changeset/stylelint-pool-by-size.md b/.changeset/stylelint-pool-by-size.md new file mode 100644 index 0000000..8907220 --- /dev/null +++ b/.changeset/stylelint-pool-by-size.md @@ -0,0 +1,5 @@ +--- +"diagnostics-webpack-plugin": patch +--- + +Lint stylesheets on webpack's thread unless a batch adds up to 2 MB, rather than over a pool of workers whatever its size: each worker loads Stylelint and its config again, which costs more than small stylesheets take to lint. Over 791 stylesheets on four cores, a cold build went from 12.4–13.7 s and 654–668 MB to 10.5–11.3 s and 506–533 MB. The README now says to set `transpileOnly: true` on `ts-loader` when moving from `fork-ts-checker-webpack-plugin`, which `ts-loader` otherwise turned on by itself. diff --git a/README.md b/README.md index e82b5c6..ad797ba 100644 --- a/README.md +++ b/README.md @@ -342,7 +342,8 @@ many; `false`, `0` or `1` keeps the work on webpack's own thread. A check that threads its own work is asked to, and one that cannot is run in a pool of workers — so the option means the same thing whatever is being checked, and a check added later gets it for nothing. ESLint threads its own work from -9.34.0 under flat config; Stylelint threads none of its own, so it is pooled. +9.34.0 under flat config; Stylelint threads none of its own, so it is pooled +once a batch is big enough to pay for the workers — see [Stylelint](#stylelint). TypeScript reads a program whole rather than a file at a time, so it is run on one worker: what a thread buys it is webpack's own thread back. The compilers of a `MultiCompiler` whose TypeScript checks are configured alike share that worker @@ -707,6 +708,13 @@ Run with `{ use: "stylelint" }`, and requires `stylelint >= 17`. It lints every Alongside the shared options you can pass any [Stylelint option](https://stylelint.io/user-guide/usage/node-api#options) — they are handed to `stylelint.lint()` as they are. +Under [`threads`](#threads), the stylesheets of a batch are linted over a pool +of workers only once they add up to 2 MB, and on webpack's thread below that: +each worker loads Stylelint and its config again, which costs more than small +stylesheets take to lint. Over 791 files with `stylelint-config-standard-scss` +on four cores, a pool was 1–2 s slower and 130–190 MB bigger up to 0.9 MB of +stylesheets, and 4 s faster at 4.5 MB. + ### `stylelintPath` - Type: @@ -1095,6 +1103,10 @@ plugin decide how much of it is paid again. Each was measured on four cores. file the program reads is most of a first build; see [TypeScript](#typescript). +**`transpileOnly` for `ts-loader` beside the TypeScript check.** Otherwise the +program is type checked twice; see +[From `fork-ts-checker-webpack-plugin`](#from-fork-ts-checker-webpack-plugin). + **`cacheStrategy: "content"` where CI restores the cache.** ESLint keys its cache on each file's time and size by default, and a fresh checkout gives every file a new time, so a cache restored in CI answers for nothing. Keyed on what @@ -1328,7 +1340,30 @@ is the thread rather than anything clever. wrote — the `fork-ts-checker-webpack-plugin` example that `ts-loader` ships, which is React and TSX through `ts-loader`. Swapping the plugin for `{ use: "typescript" }` and nothing else reported the same `TS2322` at the same -line and column. Three things to expect while doing it: +line and column. Four things to expect while doing it: + +- **Set `transpileOnly: true` on `ts-loader`.** `ts-loader` turns it on by + itself when it sees `ForkTsCheckerWebpackPlugin` in the config, so a config + that never wrote it starts type checking the whole program inside the build + again once that plugin is gone — next to the check, which does the same work + — and reports every type error twice. Over 859 files of React and SCSS, with + typed ESLint and Stylelint beside it, writing it took a cold build from + 24.5–25.2 s and 1.56 GB to 19.1–19.2 s and 1.3–1.4 GB, where the build with + that plugin took 17.6–20.3 s and 1.28–1.39 GB before the migration: + + ```js + module.exports = { + module: { + rules: [ + { + test: /\.tsx?$/, + loader: "ts-loader", + options: { transpileOnly: true }, + }, + ], + }, + }; + ``` - **webpack must be 5.106 or newer**, which is what the plugin requires at all. - **`npm` refuses to install alongside ESLint 8.** The `eslint` peer here is diff --git a/src/checks/stylelint.js b/src/checks/stylelint.js index eb8fef1..a4735d0 100644 --- a/src/checks/stylelint.js +++ b/src/checks/stylelint.js @@ -1,6 +1,7 @@ // eslint-disable-next-line jsdoc/reject-any-type /** @typedef {any} EXPECTED_ANY */ +import { statSync } from "node:fs"; import { createRequire } from "node:module"; import { fileURLToPath } from "node:url"; @@ -107,6 +108,32 @@ function arrifyFiles(files) { return Array.isArray(files) ? files : [files]; } +/** + * What a batch of stylesheets adds up to before a pool pays for starting it: + * each worker loads Stylelint and its config again. Over 791 files with + * `stylelint-config-standard-scss`, a pool was 1–2 s slower and 130–190 MB + * bigger up to 0.9 MB of stylesheets, and 4 s faster at 4.5 MB. + */ +const POOL_WORTH_STARTING = 2 * 1024 * 1024; + +/** + * @param {string[]} files the files of a batch + * @returns {number} what they add up to, in bytes + */ +function sizeOf(files) { + let size = 0; + + for (const file of files) { + try { + size += statSync(file).size; + } catch { + // Gone, which Stylelint says when it reads it. + } + } + + return size; +} + /** * @param {string} cacheKey the key the loaded stylelint is cached under * @param {number} poolSize number of workers @@ -116,27 +143,37 @@ function arrifyFiles(files) { function loadStylelintThreaded(cacheKey, poolSize, options) { const source = fileURLToPath(import.meta.resolve("./stylelint-worker.js")); const local = loadStylelint(options); - - // Stylelint threads nothing of its own, so the plugin's pool runs it — the - // same pool every check that cannot thread itself is spread over. - let pool = createPool(source, poolSize, [ - options, - getStylelintOptions(options), - ]); - - if (!pool) return local; + /** + * Started by the first batch big enough to pay for it, and kept for the + * next one; `null` once it cannot be. + * @type {import("../threads.js").Pool | null | undefined} + */ + let pool; /** @type {Loaded} */ const context = { ...local, threads: poolSize, - lintFiles: async (files) => - /* istanbul ignore next */ - pool ? pool.lintFiles(arrifyFiles(files)) : local.lintFiles(files), + lintFiles: async (files) => { + const list = arrifyFiles(files); + + if (sizeOf(list) < POOL_WORTH_STARTING) return local.lintFiles(list); + + // Stylelint threads nothing of its own, so the plugin's pool runs it — + // the same pool every check that cannot thread itself is spread over. + if (pool === undefined) { + pool = createPool(source, poolSize, [ + options, + getStylelintOptions(options), + ]); + } + + return pool ? pool.lintFiles(list) : local.lintFiles(list); + }, cleanup: async () => { cache[cacheKey] = local; context.lintFiles = (files) => local.lintFiles(files); - /* istanbul ignore next */ + if (pool) { await pool.end(); pool = null; diff --git a/test/stylelint/threads.test.js b/test/stylelint/threads.test.js index a369b18..3a49f26 100644 --- a/test/stylelint/threads.test.js +++ b/test/stylelint/threads.test.js @@ -1,5 +1,7 @@ import assert from "node:assert/strict"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, it } from "node:test"; @@ -51,6 +53,63 @@ describe("Threading", () => { } }); + it("should lint a small batch on webpack's thread and a big one over the pool", async () => { + const dir = mkdtempSync(join(tmpdir(), "stylelint-pool-")); + const small = toPosixPath(join(dir, "small.scss")); + const big = toPosixPath(join(dir, "big.scss")); + const stylelintPath = join(import.meta.dirname, "mock/stylelint-recorder"); + + // What the recorder saw on this thread: a batch the pool lints is recorded + // in the worker's copy of it instead. + const mock = require(stylelintPath); + + writeFileSync(small, "a { color: red; }\n"); + writeFileSync(big, `/* ${"x".repeat(3 * 1024 * 1024)} */\n`); + mock._reset(); + + const threaded = getLoadedStylelint("sized", { threads: 2, stylelintPath }); + + try { + await threaded.lintFiles([small, join(dir, "gone.scss")]); + + assert.strictEqual(mock._calls.length, 1); + + await threaded.lintFiles([big]); + + assert.strictEqual(mock._calls.length, 1); + } finally { + await threaded.cleanup(); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("should lint a big batch on webpack's thread when its options cannot reach a worker", async () => { + const dir = mkdtempSync(join(tmpdir(), "stylelint-pool-")); + const big = toPosixPath(join(dir, "big.scss")); + const stylelintPath = join(import.meta.dirname, "mock/stylelint-recorder"); + + const mock = require(stylelintPath); + + writeFileSync(big, `/* ${"x".repeat(3 * 1024 * 1024)} */\n`); + mock._reset(); + + // A function is not something a worker can be handed. + const threaded = getLoadedStylelint("with-function", { + threads: 2, + stylelintPath, + formatter: () => "", + }); + + try { + await threaded.lintFiles([big]); + + assert.strictEqual(mock._calls.length, 1); + } finally { + await threaded.cleanup(); + rmSync(dir, { recursive: true, force: true }); + } + }); + describe("worker coverage", () => { it("worker can start", async () => { const mockStylelintPath = join(