Skip to content

Commit

Permalink
perf(@angular-devkit/build-angular): minimize Angular diagnostics inc…
Browse files Browse the repository at this point in the history 
…remental analysis in esbuild-based builder

When using the experimental esbuild-based browser application builder, the Angular diagnostic analysis performed
per rebuild is now reduced to only the affected files for that rebuild. A rebuild will now query the TypeScript
compiler and the Angular compiler to determine the list of potentially affected files. The Angular compiler will
then only be queried for diagnostics for this set of affected files instead of the entirety of the program.
  • Loading branch information
@clydin @alan-agius4
clydin authored and alan-agius4 committed Oct 12, 2022
1 parent 1518133 commit 52db3c0
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 19 deletions.
95 changes: 78 additions & 17 deletions packages/angular_devkit/build_angular/src/builders/browser-esbuild/compiler-plugin.ts
View file
Expand Up @@ -173,9 +173,8 @@ export function createCompilerPlugin(

// This uses a wrapped dynamic import to load `@angular/compiler-cli` which is ESM.
// Once TypeScript provides support for retaining dynamic imports this workaround can be dropped.
const compilerCli = await loadEsmModule<typeof import('@angular/compiler-cli')>(
'@angular/compiler-cli',
);
const { GLOBAL_DEFS_FOR_TERSER_WITH_AOT, NgtscProgram, OptimizeFor, readConfiguration } =
await loadEsmModule<typeof import('@angular/compiler-cli')>('@angular/compiler-cli');

// Temporary deep import for transformer support
const {
Expand All @@ -185,7 +184,7 @@ export function createCompilerPlugin(

// Setup defines based on the values provided by the Angular compiler-cli
build.initialOptions.define ??= {};
for (const [key, value] of Object.entries(compilerCli.GLOBAL_DEFS_FOR_TERSER_WITH_AOT)) {
for (const [key, value] of Object.entries(GLOBAL_DEFS_FOR_TERSER_WITH_AOT)) {
if (key in build.initialOptions.define) {
// Skip keys that have been manually provided
continue;
Expand All @@ -202,7 +201,7 @@ export function createCompilerPlugin(
rootNames,
errors: configurationDiagnostics,
} = profileSync('NG_READ_CONFIG', () =>
compilerCli.readConfiguration(pluginOptions.tsconfig, {
readConfiguration(pluginOptions.tsconfig, {
noEmitOnError: false,
suppressOutputPathCheck: true,
outDir: undefined,
Expand Down Expand Up @@ -249,6 +248,7 @@ export function createCompilerPlugin(
let previousBuilder: ts.EmitAndSemanticDiagnosticsBuilderProgram | undefined;
let previousAngularProgram: NgtscProgram | undefined;
const babelDataCache = new Map<string, string>();
const diagnosticCache = new WeakMap<ts.SourceFile, ts.Diagnostic[]>();

build.onStart(async () => {
const result: OnStartResult = {
Expand Down Expand Up @@ -339,12 +339,10 @@ export function createCompilerPlugin(
// Create the Angular specific program that contains the Angular compiler
const angularProgram = profileSync(
'NG_CREATE_PROGRAM',
() =>
new compilerCli.NgtscProgram(rootNames, compilerOptions, host, previousAngularProgram),
() => new NgtscProgram(rootNames, compilerOptions, host, previousAngularProgram),
);
previousAngularProgram = angularProgram;
const angularCompiler = angularProgram.compiler;
const { ignoreForDiagnostics } = angularCompiler;
const typeScriptProgram = angularProgram.getTsProgram();
augmentProgramWithVersioning(typeScriptProgram);

Expand All @@ -366,12 +364,16 @@ export function createCompilerPlugin(
yield* builder.getGlobalDiagnostics();

// Collect source file specific diagnostics
const OptimizeFor = compilerCli.OptimizeFor;
const affectedFiles = findAffectedFiles(builder, angularCompiler);
const optimizeFor =
affectedFiles.size > 1 ? OptimizeFor.WholeProgram : OptimizeFor.SingleFile;
for (const sourceFile of builder.getSourceFiles()) {
if (ignoreForDiagnostics.has(sourceFile)) {
if (angularCompiler.ignoreForDiagnostics.has(sourceFile)) {
continue;
}

// TypeScript will use cached diagnostics for files that have not been
// changed or affected for this build when using incremental building.
yield* profileSync(
'NG_DIAGNOSTICS_SYNTACTIC',
() => builder.getSyntacticDiagnostics(sourceFile),
Expand All @@ -383,12 +385,22 @@ export function createCompilerPlugin(
true,
);

const angularDiagnostics = profileSync(
'NG_DIAGNOSTICS_TEMPLATE',
() => angularCompiler.getDiagnosticsForFile(sourceFile, OptimizeFor.WholeProgram),
true,
);
yield* angularDiagnostics;
// Only request Angular template diagnostics for affected files to avoid
// overhead of template diagnostics for unchanged files.
if (affectedFiles.has(sourceFile)) {
const angularDiagnostics = profileSync(
'NG_DIAGNOSTICS_TEMPLATE',
() => angularCompiler.getDiagnosticsForFile(sourceFile, optimizeFor),
true,
);
diagnosticCache.set(sourceFile, angularDiagnostics);
yield* angularDiagnostics;
} else {
const angularDiagnostics = diagnosticCache.get(sourceFile);
if (angularDiagnostics) {
yield* angularDiagnostics;
}
}
}
}

Expand All @@ -408,7 +420,7 @@ export function createCompilerPlugin(
mergeTransformers(angularCompiler.prepareEmit().transformers, {
before: [replaceBootstrap(() => builder.getProgram().getTypeChecker())],
}),
(sourceFile) => angularCompiler.incrementalDriver.recordSuccessfulEmit(sourceFile),
(sourceFile) => angularCompiler.incrementalCompilation.recordSuccessfulEmit(sourceFile),
);

return result;
Expand Down Expand Up @@ -590,3 +602,52 @@ async function transformWithBabel(

return result?.code ?? data;
}

function findAffectedFiles(
builder: ts.EmitAndSemanticDiagnosticsBuilderProgram,
{ ignoreForDiagnostics, ignoreForEmit, incrementalCompilation }: NgtscProgram['compiler'],
): Set<ts.SourceFile> {
const affectedFiles = new Set<ts.SourceFile>();

// eslint-disable-next-line no-constant-condition
while (true) {
const result = builder.getSemanticDiagnosticsOfNextAffectedFile(undefined, (sourceFile) => {
// If the affected file is a TTC shim, add the shim's original source file.
// This ensures that changes that affect TTC are typechecked even when the changes
// are otherwise unrelated from a TS perspective and do not result in Ivy codegen changes.
// For example, changing @Input property types of a directive used in another component's
// template.
// A TTC shim is a file that has been ignored for diagnostics and has a filename ending in `.ngtypecheck.ts`.
if (ignoreForDiagnostics.has(sourceFile) && sourceFile.fileName.endsWith('.ngtypecheck.ts')) {
// This file name conversion relies on internal compiler logic and should be converted
// to an official method when available. 15 is length of `.ngtypecheck.ts`
const originalFilename = sourceFile.fileName.slice(0, -15) + '.ts';
const originalSourceFile = builder.getSourceFile(originalFilename);
if (originalSourceFile) {
affectedFiles.add(originalSourceFile);
}

return true;
}

return false;
});

if (!result) {
break;
}

affectedFiles.add(result.affected as ts.SourceFile);
}

// A file is also affected if the Angular compiler requires it to be emitted
for (const sourceFile of builder.getSourceFiles()) {
if (ignoreForEmit.has(sourceFile) || incrementalCompilation.safeToSkipEmit(sourceFile)) {
continue;
}

affectedFiles.add(sourceFile);
}

return affectedFiles;
}
4 changes: 2 additions & 2 deletions packages/angular_devkit/build_angular/src/builders/browser-esbuild/profiling.ts
View file
Expand Up @@ -21,7 +21,7 @@ export function logCumulativeDurations(): void {

for (const [name, duration] of cumulativeDurations) {
// eslint-disable-next-line no-console
console.log(`DURATION[${name}]: ${duration} seconds`);
console.log(`DURATION[${name}]: ${duration.toFixed(9)} seconds`);
}
}

Expand All @@ -32,7 +32,7 @@ function recordDuration(name: string, startTime: bigint, cumulative?: boolean):
cumulativeDurations.set(name, (cumulativeDurations.get(name) ?? 0) + duration);
} else {
// eslint-disable-next-line no-console
console.log(`DURATION[${name}]: ${duration} seconds`);
console.log(`DURATION[${name}]: ${duration.toFixed(9)} seconds`);
}
}

Expand Down

0 comments on commit 52db3c0

Please sign in to comment.