From 878ce6b11afb1b18073aa672a0307e249b29ddf9 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 17 May 2021 13:49:43 +0200 Subject: [PATCH] respect multi compiler `dependencies` and `parallelism` when using `invalidate` --- lib/MultiCompiler.js | 58 +++++++------- lib/Watching.js | 27 +++---- test/MultiCompiler.test.js | 155 +++++++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 42 deletions(-) diff --git a/lib/MultiCompiler.js b/lib/MultiCompiler.js index 99b67e04f..9b3eab5d3 100644 --- a/lib/MultiCompiler.js +++ b/lib/MultiCompiler.js @@ -318,16 +318,17 @@ module.exports = class MultiCompiler { * @returns {SetupResult[]} result of setup */ _runGraph(setup, run, callback) { - /** @typedef {{ compiler: Compiler, result: Stats, state: "pending" | "blocked" | "queued" | "running" | "running-outdated" | "done", children: Node[], parents: Node[] }} Node */ + /** @typedef {{ compiler: Compiler, result: Stats, state: "pending" | "blocked" | "queued" | "starting" | "running" | "running-outdated" | "done", children: Node[], parents: Node[] }} Node */ // State transitions for nodes: // -> blocked (initial) - // blocked -> queued [add to queue] (when all parents done) - // queued -> running [running++] (when processing the queue) + // blocked -> starting [running++] (when all parents done) + // queued -> starting [running++] (when processing the queue) + // starting -> running (when run has been called) // running -> done [running--] (when compilation is done) // done -> pending (when invalidated from file change) - // pending -> blocked (when invalidated from aggregated changes) - // done -> blocked (when invalidated, from parent invalidation) + // pending -> blocked [add to queue] (when invalidated from aggregated changes) + // done -> blocked [add to queue] (when invalidated, from parent invalidation) // running -> running-outdated (when invalidated, either from change or parent invalidation) // running-outdated -> blocked [running--] (when compilation is done) @@ -351,6 +352,7 @@ module.exports = class MultiCompiler { parent.children.push(node); } } + /** @type {ArrayQueue} */ const queue = new ArrayQueue(); for (const node of nodes) { if (node.parents.length === 0) { @@ -388,13 +390,13 @@ module.exports = class MultiCompiler { if (node.state === "running") { node.state = "done"; for (const child of node.children) { - checkUnblocked(child); + if (child.state === "blocked") queue.enqueue(child); } } else if (node.state === "running-outdated") { node.state = "blocked"; - checkUnblocked(node); + queue.enqueue(node); } - process.nextTick(processQueue); + processQueue(); }; /** * @param {Node} node node @@ -433,20 +435,9 @@ module.exports = class MultiCompiler { if (node.state === "pending") { node.state = "blocked"; } - checkUnblocked(node); - processQueue(); - }; - /** - * @param {Node} node node - * @returns {void} - */ - const checkUnblocked = node => { - if ( - node.state === "blocked" && - node.parents.every(p => p.state === "done") - ) { - node.state = "queued"; + if (node.state === "blocked") { queue.enqueue(node); + processQueue(); } }; @@ -457,20 +448,33 @@ module.exports = class MultiCompiler { node.compiler, i, nodeDone.bind(null, node), - () => node.state !== "done" && node.state !== "running", + () => node.state !== "starting" && node.state !== "running", () => nodeChange(node), () => nodeInvalid(node) ) ); }); + let processing = true; const processQueue = () => { + if (processing) return; + processing = true; + process.nextTick(processQueueWorker); + }; + const processQueueWorker = () => { while (running < parallelism && queue.length > 0 && !errored) { const node = queue.dequeue(); - if (node.state !== "queued") continue; - running++; - node.state = "running"; - run(node.compiler, nodeDone.bind(null, node)); + if ( + node.state === "queued" || + (node.state === "blocked" && + node.parents.every(p => p.state === "done")) + ) { + running++; + node.state = "starting"; + run(node.compiler, nodeDone.bind(null, node)); + node.state = "running"; + } } + processing = false; if ( !errored && running === 0 && @@ -489,7 +493,7 @@ module.exports = class MultiCompiler { } } }; - processQueue(); + processQueueWorker(); return setupResults; } diff --git a/lib/Watching.js b/lib/Watching.js index 61a747d73..2b252b444 100644 --- a/lib/Watching.js +++ b/lib/Watching.js @@ -159,13 +159,16 @@ class Watching { let stats = null; - const handleError = err => { + const handleError = (err, cbs) => { this.compiler.hooks.failed.call(err); this.compiler.cache.beginIdle(); this.compiler.idle = true; this.handler(err, stats); - for (const cb of this.callbacks) cb(); - this.callbacks.length = 0; + if (!cbs) { + cbs = this.callbacks; + this.callbacks = []; + } + for (const cb of cbs) cb(err); }; if ( @@ -197,17 +200,19 @@ class Watching { } if (err) return handleError(err); + const cbs = this.callbacks; + this.callbacks = []; logger.time("done hook"); this.compiler.hooks.done.callAsync(stats, err => { logger.timeEnd("done hook"); - if (err) return handleError(err); + if (err) return handleError(err, cbs); this.handler(null, stats); logger.time("storeBuildDependencies"); this.compiler.cache.storeBuildDependencies( compilation.buildDependencies, err => { logger.timeEnd("storeBuildDependencies"); - if (err) return handleError(err); + if (err) return handleError(err, cbs); logger.time("beginIdle"); this.compiler.cache.beginIdle(); this.compiler.idle = true; @@ -221,8 +226,7 @@ class Watching { ); } }); - for (const cb of this.callbacks) cb(); - this.callbacks.length = 0; + for (const cb of cbs) cb(null); this.compiler.hooks.afterDone.call(stats); } ); @@ -293,6 +297,7 @@ class Watching { this._invalidReported = true; this.compiler.hooks.invalid.call(null, Date.now()); } + this._onChange(); this._invalidate(); } @@ -335,14 +340,6 @@ class Watching { } } - _checkUnblocked() { - if (this.blocked && !this._isBlocked()) { - this.blocked = false; - this._needWatcherInfo = true; - this._invalidate(); - } - } - /** * @param {Callback} callback signals when the watcher is closed * @returns {void} diff --git a/test/MultiCompiler.test.js b/test/MultiCompiler.test.js index 0c785d440..59965dd3e 100644 --- a/test/MultiCompiler.test.js +++ b/test/MultiCompiler.test.js @@ -375,6 +375,161 @@ describe("MultiCompiler", function () { }); }); + it("should respect parallelism when using invalidate", done => { + const configs = [ + { + name: "a", + mode: "development", + entry: { a: "./a.js" }, + context: path.join(__dirname, "fixtures") + }, + { + name: "b", + mode: "development", + entry: { b: "./b.js" }, + context: path.join(__dirname, "fixtures") + } + ]; + configs.parallelism = 1; + const compiler = webpack(configs); + + const events = []; + compiler.compilers.forEach(c => { + c.hooks.invalid.tap("test", () => { + events.push(`${c.name} invalid`); + }); + c.hooks.watchRun.tap("test", () => { + events.push(`${c.name} run`); + }); + c.hooks.done.tap("test", () => { + events.push(`${c.name} done`); + }); + }); + + compiler.watchFileSystem = { watch() {} }; + compiler.outputFileSystem = createFsFromVolume(new Volume()); + + let state = 0; + const watching = compiler.watch({}, error => { + if (error) { + done(error); + return; + } + if (state !== 0) return; + state++; + + expect(events).toMatchInlineSnapshot(` + Array [ + "a run", + "a done", + "b run", + "b done", + ] + `); + events.length = 0; + + watching.invalidate(err => { + try { + if (err) return done(err); + + expect(events).toMatchInlineSnapshot(` + Array [ + "a invalid", + "b invalid", + "a run", + "a done", + "b run", + "b done", + ] + `); + events.length = 0; + expect(state).toBe(1); + setTimeout(done, 1000); + } catch (e) { + console.error(e); + done(e); + } + }); + }); + }, 2000); + + it("should respect dependencies when using invalidate", done => { + const compiler = webpack([ + { + name: "a", + mode: "development", + entry: { a: "./a.js" }, + context: path.join(__dirname, "fixtures"), + dependencies: ["b"] + }, + { + name: "b", + mode: "development", + entry: { b: "./b.js" }, + context: path.join(__dirname, "fixtures") + } + ]); + + const events = []; + compiler.compilers.forEach(c => { + c.hooks.invalid.tap("test", () => { + events.push(`${c.name} invalid`); + }); + c.hooks.watchRun.tap("test", () => { + events.push(`${c.name} run`); + }); + c.hooks.done.tap("test", () => { + events.push(`${c.name} done`); + }); + }); + + compiler.watchFileSystem = { watch() {} }; + compiler.outputFileSystem = createFsFromVolume(new Volume()); + + let state = 0; + const watching = compiler.watch({}, error => { + if (error) { + done(error); + return; + } + if (state !== 0) return; + state++; + + expect(events).toMatchInlineSnapshot(` + Array [ + "b run", + "b done", + "a run", + "a done", + ] + `); + events.length = 0; + + watching.invalidate(err => { + try { + if (err) return done(err); + + expect(events).toMatchInlineSnapshot(` + Array [ + "a invalid", + "b invalid", + "b run", + "b done", + "a run", + "a done", + ] + `); + events.length = 0; + expect(state).toBe(1); + setTimeout(done, 1000); + } catch (e) { + console.error(e); + done(e); + } + }); + }); + }, 2000); + it("shouldn't hang when invalidating watchers", done => { const entriesA = { a: "./a.js" }; const entriesB = { b: "./b.js" };