Skip to content

Commit

Permalink
perf: abort stale type-checks on fast recompilations (#764)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
piotr-oles authored Jul 18, 2022
1 parent 6155272 commit ab5b12d
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 29 deletions.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
9 changes: 6 additions & 3 deletions src/hooks/tap-done-to-async-get-issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 5 additions & 0 deletions src/hooks/tap-error-to-log-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
83 changes: 63 additions & 20 deletions src/hooks/tap-start-to-run-workers.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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
});
}

Expand Down
6 changes: 6 additions & 0 deletions src/plugin-state.ts
Original file line number Diff line number Diff line change
@@ -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<Issue[] | undefined>;
dependenciesPromise: Promise<FilesMatch | undefined>;
abortController: AbortController | undefined;
aggregatedFilesChange: FilesChange | undefined;
lastDependencies: FilesMatch | undefined;
watching: boolean;
initialized: boolean;
Expand All @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions src/utils/async/abort-error.ts
Original file line number Diff line number Diff line change
@@ -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 };
15 changes: 11 additions & 4 deletions src/utils/async/pool.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
type Task<T> = () => Promise<T>;
import type { AbortSignal } from 'node-abort-controller';

import { AbortError } from './abort-error';

type Task<T> = (signal?: AbortSignal) => Promise<T>;

interface Pool {
submit<T>(task: Task<T>): Promise<T>;
submit<T>(task: Task<T>, signal?: AbortSignal): Promise<T>;
size: number;
readonly pending: number;
readonly drained: Promise<void>;
Expand All @@ -11,12 +15,15 @@ function createPool(size: number): Pool {
let pendingPromises: Promise<unknown>[] = [];

const pool = {
async submit<T>(task: Task<T>): Promise<T> {
async submit<T>(task: Task<T>, signal?: AbortSignal): Promise<T> {
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
);
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit ab5b12d

Please sign in to comment.