From ab5b12dc0c5dd3c3dd6338abb68628aae5ef97f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ole=C5=9B?= Date: Mon, 18 Jul 2022 08:30:07 +0200 Subject: [PATCH] perf: abort stale type-checks on fast recompilations (#764) When user saves changes faster than type-checking process, we can end-up with a queue of type-checks that could be skipped. This commit ensures that we queue only 1 type-check per compilation and abort previous. --- package.json | 5 +- src/hooks/tap-done-to-async-get-issues.ts | 9 ++- src/hooks/tap-error-to-log-message.ts | 5 ++ src/hooks/tap-start-to-run-workers.ts | 83 +++++++++++++++++------ src/plugin-state.ts | 6 ++ src/utils/async/abort-error.ts | 16 +++++ src/utils/async/pool.ts | 15 ++-- yarn.lock | 5 ++ 8 files changed, 115 insertions(+), 29 deletions(-) create mode 100644 src/utils/async/abort-error.ts diff --git a/package.json b/package.json index f924e780..31b906ea 100644 --- a/package.json +++ b/package.json @@ -63,14 +63,15 @@ "fs-extra": "^10.0.0", "memfs": "^3.4.1", "minimatch": "^3.0.4", + "node-abort-controller": "^3.0.1", "schema-utils": "^3.1.1", "semver": "^7.3.5", "tapable": "^2.2.1" }, "peerDependencies": { "typescript": ">3.6.0", - "webpack": "^5.11.0", - "vue-template-compiler": "*" + "vue-template-compiler": "*", + "webpack": "^5.11.0" }, "peerDependenciesMeta": { "vue-template-compiler": { diff --git a/src/hooks/tap-done-to-async-get-issues.ts b/src/hooks/tap-done-to-async-get-issues.ts index 77446c18..82a4577c 100644 --- a/src/hooks/tap-done-to-async-get-issues.ts +++ b/src/hooks/tap-done-to-async-get-issues.ts @@ -39,17 +39,20 @@ function tapDoneToAsyncGetIssues( } issues = await issuesPromise; - debug('Got issues from getIssuesWorker.', issues?.length); } catch (error) { hooks.error.call(error, stats.compilation); return; } - if (!issues) { - // some error has been thrown or it was canceled + if ( + !issues || // some error has been thrown + state.issuesPromise !== issuesPromise // we have a new request - don't show results for the old one + ) { return; } + debug(`Got ${issues?.length || 0} issues from getIssuesWorker.`); + // filter list of issues by provided issue predicate issues = issues.filter(config.issue.predicate); diff --git a/src/hooks/tap-error-to-log-message.ts b/src/hooks/tap-error-to-log-message.ts index 3145af11..d8fa5a35 100644 --- a/src/hooks/tap-error-to-log-message.ts +++ b/src/hooks/tap-error-to-log-message.ts @@ -4,6 +4,7 @@ import type webpack from 'webpack'; import type { ForkTsCheckerWebpackPluginConfig } from '../plugin-config'; import { getPluginHooks } from '../plugin-hooks'; import { RpcExitError } from '../rpc'; +import { AbortError } from '../utils/async/abort-error'; function tapErrorToLogMessage( compiler: webpack.Compiler, @@ -12,6 +13,10 @@ function tapErrorToLogMessage( const hooks = getPluginHooks(compiler); hooks.error.tap('ForkTsCheckerWebpackPlugin', (error) => { + if (error instanceof AbortError) { + return; + } + config.logger.error(String(error)); if (error instanceof RpcExitError) { diff --git a/src/hooks/tap-start-to-run-workers.ts b/src/hooks/tap-start-to-run-workers.ts index 6231119f..62b0efe2 100644 --- a/src/hooks/tap-start-to-run-workers.ts +++ b/src/hooks/tap-start-to-run-workers.ts @@ -1,7 +1,8 @@ +import { AbortController } from 'node-abort-controller'; import type * as webpack from 'webpack'; import type { FilesChange } from '../files-change'; -import { consumeFilesChange } from '../files-change'; +import { aggregateFilesChanges, consumeFilesChange } from '../files-change'; import { getInfrastructureLogger } from '../infrastructure-logger'; import type { ForkTsCheckerWebpackPluginConfig } from '../plugin-config'; import { getPluginHooks } from '../plugin-hooks'; @@ -59,49 +60,91 @@ function tapStartToRunWorkers( return; } + // get current iteration number const iteration = ++state.iteration; - let change: FilesChange = {}; + // abort previous iteration + if (state.abortController) { + debug(`Aborting iteration ${iteration - 1}.`); + state.abortController.abort(); + } + + // create new abort controller for the new iteration + const abortController = new AbortController(); + state.abortController = abortController; + + let filesChange: FilesChange = {}; if (state.watching) { - change = consumeFilesChange(compiler); + filesChange = consumeFilesChange(compiler); log( [ 'Calling reporter service for incremental check.', - ` Changed files: ${JSON.stringify(change.changedFiles)}`, - ` Deleted files: ${JSON.stringify(change.deletedFiles)}`, + ` Changed files: ${JSON.stringify(filesChange.changedFiles)}`, + ` Deleted files: ${JSON.stringify(filesChange.deletedFiles)}`, ].join('\n') ); } else { log('Calling reporter service for single check.'); } - change = await hooks.start.promise(change, compilation); + filesChange = await hooks.start.promise(filesChange, compilation); + let aggregatedFilesChange = filesChange; + if (state.aggregatedFilesChange) { + aggregatedFilesChange = aggregateFilesChanges([aggregatedFilesChange, filesChange]); + debug( + [ + `Aggregating with previous files change, iteration ${iteration}.`, + ` Changed files: ${JSON.stringify(aggregatedFilesChange.changedFiles)}`, + ` Deleted files: ${JSON.stringify(aggregatedFilesChange.deletedFiles)}`, + ].join('\n') + ); + } + state.aggregatedFilesChange = aggregatedFilesChange; + + // submit one at a time for a single compiler + state.issuesPromise = (state.issuesPromise || Promise.resolve()) + // resolve to undefined on error + .catch(() => undefined) + .then(() => { + // early return + if (abortController.signal.aborted) { + return undefined; + } + + debug(`Submitting the getIssuesWorker to the pool, iteration ${iteration}.`); + return issuesPool.submit(async () => { + try { + debug(`Running the getIssuesWorker, iteration ${iteration}.`); + const issues = await getIssuesWorker(aggregatedFilesChange, state.watching); + if (state.aggregatedFilesChange === aggregatedFilesChange) { + state.aggregatedFilesChange = undefined; + } + if (state.abortController === abortController) { + state.abortController = undefined; + } + return issues; + } catch (error) { + hooks.error.call(error, compilation); + return undefined; + } finally { + debug(`The getIssuesWorker finished its job, iteration ${iteration}.`); + } + }, abortController.signal); + }); - debug(`Submitting the getIssuesWorker to the pool, iteration ${iteration}.`); - state.issuesPromise = issuesPool.submit(async () => { - try { - debug(`Running the getIssuesWorker, iteration ${iteration}.`); - return await getIssuesWorker(change, state.watching); - } catch (error) { - hooks.error.call(error, compilation); - return undefined; - } finally { - debug(`The getIssuesWorker finished its job, iteration ${iteration}.`); - } - }); debug(`Submitting the getDependenciesWorker to the pool, iteration ${iteration}.`); state.dependenciesPromise = dependenciesPool.submit(async () => { try { debug(`Running the getDependenciesWorker, iteration ${iteration}.`); - return await getDependenciesWorker(change); + return await getDependenciesWorker(filesChange); } catch (error) { hooks.error.call(error, compilation); return undefined; } finally { debug(`The getDependenciesWorker finished its job, iteration ${iteration}.`); } - }); + }); // don't pass abortController.signal because getDependencies() is blocking }); } diff --git a/src/plugin-state.ts b/src/plugin-state.ts index 7a8aa840..845ba540 100644 --- a/src/plugin-state.ts +++ b/src/plugin-state.ts @@ -1,11 +1,15 @@ +import type { AbortController } from 'node-abort-controller'; import type { FullTap } from 'tapable'; +import type { FilesChange } from './files-change'; import type { FilesMatch } from './files-match'; import type { Issue } from './issue'; interface ForkTsCheckerWebpackPluginState { issuesPromise: Promise; dependenciesPromise: Promise; + abortController: AbortController | undefined; + aggregatedFilesChange: FilesChange | undefined; lastDependencies: FilesMatch | undefined; watching: boolean; initialized: boolean; @@ -17,6 +21,8 @@ function createPluginState(): ForkTsCheckerWebpackPluginState { return { issuesPromise: Promise.resolve(undefined), dependenciesPromise: Promise.resolve(undefined), + abortController: undefined, + aggregatedFilesChange: undefined, lastDependencies: undefined, watching: false, initialized: false, diff --git a/src/utils/async/abort-error.ts b/src/utils/async/abort-error.ts new file mode 100644 index 00000000..f65fe1c4 --- /dev/null +++ b/src/utils/async/abort-error.ts @@ -0,0 +1,16 @@ +import type { AbortSignal } from 'node-abort-controller'; + +class AbortError extends Error { + constructor(message = 'Task aborted.') { + super(message); + this.name = 'AbortError'; + } + + static throwIfAborted(signal: AbortSignal | undefined) { + if (signal?.aborted) { + throw new AbortError(); + } + } +} + +export { AbortError }; diff --git a/src/utils/async/pool.ts b/src/utils/async/pool.ts index 65627303..8578b3cd 100644 --- a/src/utils/async/pool.ts +++ b/src/utils/async/pool.ts @@ -1,7 +1,11 @@ -type Task = () => Promise; +import type { AbortSignal } from 'node-abort-controller'; + +import { AbortError } from './abort-error'; + +type Task = (signal?: AbortSignal) => Promise; interface Pool { - submit(task: Task): Promise; + submit(task: Task, signal?: AbortSignal): Promise; size: number; readonly pending: number; readonly drained: Promise; @@ -11,12 +15,15 @@ function createPool(size: number): Pool { let pendingPromises: Promise[] = []; const pool = { - async submit(task: Task): Promise { + async submit(task: Task, signal?: AbortSignal): Promise { while (pendingPromises.length >= pool.size) { + AbortError.throwIfAborted(signal); await Promise.race(pendingPromises).catch(() => undefined); } - const taskPromise = task().finally(() => { + AbortError.throwIfAborted(signal); + + const taskPromise = task(signal).finally(() => { pendingPromises = pendingPromises.filter( (pendingPromise) => pendingPromise !== taskPromise ); diff --git a/yarn.lock b/yarn.lock index 9ed93b82..24cf694b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5804,6 +5804,11 @@ nerf-dart@^1.0.0: resolved "https://registry.yarnpkg.com/nerf-dart/-/nerf-dart-1.0.0.tgz#e6dab7febf5ad816ea81cf5c629c5a0ebde72c1a" integrity sha1-5tq3/r9a2Bbqgc9cYpxaDr3nLBo= +node-abort-controller@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/node-abort-controller/-/node-abort-controller-3.0.1.tgz#f91fa50b1dee3f909afabb7e261b1e1d6b0cb74e" + integrity sha512-/ujIVxthRs+7q6hsdjHMaj8hRG9NuWmwrz+JdRwZ14jdFoKSkm+vDsCbF9PLpnSqjaWQJuTmVtcWHNLr+vrOFw== + node-emoji@^1.10.0: version "1.11.0" resolved "https://registry.yarnpkg.com/node-emoji/-/node-emoji-1.11.0.tgz#69a0150e6946e2f115e9d7ea4df7971e2628301c"