From b9f16d3f899c177d458ce42dd20e7a811b5cd5b0 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 21 Oct 2022 16:01:59 -0400 Subject: [PATCH 1/2] perf(@angular-devkit/build-angular): avoid extra TypeScript emits with esbuild rebuilds To further improve incremental rebuild performance of the experimental esbuild-based browser application builder, the output of the TypeScript file loader within the Angular compiler plugin are now cached in memory by the input file name and invalidated via the file watching events. This allows an additional TypeScript emit including the associated transformations per input file to be avoided if the file has not changed or has not been affected by other files within the TypeScript program. --- .../browser-esbuild/compiler-plugin.ts | 100 +++++++++++------- 1 file changed, 64 insertions(+), 36 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/compiler-plugin.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/compiler-plugin.ts index 1ffc346a62bc..0f70fd42e142 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/compiler-plugin.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/compiler-plugin.ts @@ -8,7 +8,6 @@ import type { CompilerHost, NgtscProgram } from '@angular/compiler-cli'; import { transformAsync } from '@babel/core'; -import * as assert from 'assert'; import type { OnStartResult, OutputFile, @@ -17,9 +16,11 @@ import type { Plugin, PluginBuild, } from 'esbuild'; -import { promises as fs } from 'fs'; -import { platform } from 'os'; -import * as path from 'path'; +import * as assert from 'node:assert'; +import * as fs from 'node:fs/promises'; +import { platform } from 'node:os'; +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; import ts from 'typescript'; import angularApplicationPreset from '../../babel/presets/application'; import { requiresLinking } from '../../babel/webpack-loader'; @@ -133,11 +134,13 @@ const WINDOWS_SEP_REGEXP = new RegExp(`\\${path.win32.sep}`, 'g'); export class SourceFileCache extends Map { readonly modifiedFiles = new Set(); readonly babelFileCache = new Map(); + readonly typeScriptFileCache = new Map(); invalidate(files: Iterable): void { this.modifiedFiles.clear(); for (let file of files) { this.babelFileCache.delete(file); + this.typeScriptFileCache.delete(pathToFileURL(file).href); // Normalize separators to allow matching TypeScript Host paths if (USING_WINDOWS) { @@ -355,6 +358,17 @@ export function createCompilerPlugin( previousBuilder = builder; await profileAsync('NG_ANALYZE_PROGRAM', () => angularCompiler.analyzeAsync()); + const affectedFiles = profileSync('NG_FIND_AFFECTED', () => + findAffectedFiles(builder, angularCompiler), + ); + + if (pluginOptions.sourceFileCache) { + for (const affected of affectedFiles) { + pluginOptions.sourceFileCache.typeScriptFileCache.delete( + pathToFileURL(affected.fileName).href, + ); + } + } function* collectDiagnostics(): Iterable { // Collect program level diagnostics @@ -364,7 +378,6 @@ export function createCompilerPlugin( yield* builder.getGlobalDiagnostics(); // Collect source file specific diagnostics - const affectedFiles = findAffectedFiles(builder, angularCompiler); const optimizeFor = affectedFiles.size > 1 ? OptimizeFor.WholeProgram : OptimizeFor.SingleFile; for (const sourceFile of builder.getSourceFiles()) { @@ -434,41 +447,56 @@ export function createCompilerPlugin( async () => { assert.ok(fileEmitter, 'Invalid plugin execution order'); - const typescriptResult = await fileEmitter( - pluginOptions.fileReplacements?.[args.path] ?? args.path, + // The filename is currently used as a cache key. Since the cache is memory only, + // the options cannot change and do not need to be represented in the key. If the + // cache is later stored to disk, then the options that affect transform output + // would need to be added to the key as well as a check for any change of content. + let contents = pluginOptions.sourceFileCache?.typeScriptFileCache.get( + pathToFileURL(args.path).href, ); - if (!typescriptResult) { - // No TS result indicates the file is not part of the TypeScript program. - // If allowJs is enabled and the file is JS then defer to the next load hook. - if (compilerOptions.allowJs && /\.[cm]?js$/.test(args.path)) { - return undefined; + + if (contents === undefined) { + const typescriptResult = await fileEmitter( + pluginOptions.fileReplacements?.[args.path] ?? args.path, + ); + if (!typescriptResult) { + // No TS result indicates the file is not part of the TypeScript program. + // If allowJs is enabled and the file is JS then defer to the next load hook. + if (compilerOptions.allowJs && /\.[cm]?js$/.test(args.path)) { + return undefined; + } + + // Otherwise return an error + return { + errors: [ + { + text: `File '${args.path}' is missing from the TypeScript compilation.`, + notes: [ + { + text: `Ensure the file is part of the TypeScript program via the 'files' or 'include' property.`, + }, + ], + }, + ], + }; } - // Otherwise return an error - return { - errors: [ - { - text: `File '${args.path}' is missing from the TypeScript compilation.`, - notes: [ - { - text: `Ensure the file is part of the TypeScript program via the 'files' or 'include' property.`, - }, - ], - }, - ], - }; - } + const data = typescriptResult.content ?? ''; + // The pre-transformed data is used as a cache key. Since the cache is memory only, + // the options cannot change and do not need to be represented in the key. If the + // cache is later stored to disk, then the options that affect transform output + // would need to be added to the key as well. + contents = babelDataCache.get(data); + if (contents === undefined) { + const transformedData = await transformWithBabel(args.path, data, pluginOptions); + contents = Buffer.from(transformedData, 'utf-8'); + babelDataCache.set(data, contents); + } - const data = typescriptResult.content ?? ''; - // The pre-transformed data is used as a cache key. Since the cache is memory only, - // the options cannot change and do not need to be represented in the key. If the - // cache is later stored to disk, then the options that affect transform output - // would need to be added to the key as well. - let contents = babelDataCache.get(data); - if (contents === undefined) { - const transformedData = await transformWithBabel(args.path, data, pluginOptions); - contents = Buffer.from(transformedData, 'utf-8'); - babelDataCache.set(data, contents); + pluginOptions.sourceFileCache?.typeScriptFileCache.set( + pathToFileURL(args.path).href, + contents, + ); } return { From dabbf1d174847f5018856a617930efa206c0497c Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 21 Oct 2022 16:08:21 -0400 Subject: [PATCH 2/2] refactor(@angular-devkit/build-angular): show more cumulative metrics for esbuild perf debug output When the `NG_BUILD_DEBUG_PERF` environment variable is used to debug performance of the experimental esbuild-based browser application builder, additional information will be logged for cumulative profiling actions. This includes the count, minimum, maximum, and average. --- .../src/builders/browser-esbuild/profiling.ts | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/profiling.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/profiling.ts index c7a0cceff4b5..82b852e997ee 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/profiling.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/profiling.ts @@ -8,7 +8,7 @@ import { debugPerformance } from '../../utils/environment-options'; -let cumulativeDurations: Map | undefined; +let cumulativeDurations: Map | undefined; export function resetCumulativeDurations(): void { cumulativeDurations?.clear(); @@ -19,20 +19,39 @@ export function logCumulativeDurations(): void { return; } - for (const [name, duration] of cumulativeDurations) { + for (const [name, durations] of cumulativeDurations) { + let total = 0; + let min; + let max; + for (const duration of durations) { + total += duration; + if (min === undefined || duration < min) { + min = duration; + } + if (max === undefined || duration > max) { + max = duration; + } + } + const average = total / durations.length; // eslint-disable-next-line no-console - console.log(`DURATION[${name}]: ${duration.toFixed(9)} seconds`); + console.log( + `DURATION[${name}]: ${total.toFixed(9)}s [count: ${durations.length}; avg: ${average.toFixed( + 9, + )}s; min: ${min?.toFixed(9)}s; max: ${max?.toFixed(9)}s]`, + ); } } function recordDuration(name: string, startTime: bigint, cumulative?: boolean): void { const duration = Number(process.hrtime.bigint() - startTime) / 10 ** 9; if (cumulative) { - cumulativeDurations ??= new Map(); - cumulativeDurations.set(name, (cumulativeDurations.get(name) ?? 0) + duration); + cumulativeDurations ??= new Map(); + const durations = cumulativeDurations.get(name) ?? []; + durations.push(duration); + cumulativeDurations.set(name, durations); } else { // eslint-disable-next-line no-console - console.log(`DURATION[${name}]: ${duration.toFixed(9)} seconds`); + console.log(`DURATION[${name}]: ${duration.toFixed(9)}s`); } }