Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf(@angular-devkit/build-angular): avoid extra TypeScript emits with esbuild rebuilds #24114

Merged
merged 2 commits into from Oct 24, 2022
Merged

perf(@angular-devkit/build-angular): avoid extra TypeScript emits with esbuild rebuilds #24114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9f16d3
perf(@angular-devkit/build-angular): avoid extra TypeScript emits wit…
clydin Oct 21, 2022
dabbf1d
refactor(@angular-devkit/build-angular): show more cumulative metrics…
clydin Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
100 changes: 64 additions & 36 deletions packages/angular_devkit/build_angular/src/builders/browser-esbuild/compiler-plugin.ts
View file
Open in desktop
Expand Up @@ -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,
Expand All @@ -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';
Expand Down Expand Up @@ -133,11 +134,13 @@ const WINDOWS_SEP_REGEXP = new RegExp(`\\${path.win32.sep}`, 'g');
export class SourceFileCache extends Map<string, ts.SourceFile> {
readonly modifiedFiles = new Set<string>();
readonly babelFileCache = new Map<string, Uint8Array>();
readonly typeScriptFileCache = new Map<string, Uint8Array>();

invalidate(files: Iterable<string>): 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) {
Expand Down Expand Up @@ -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<ts.Diagnostic> {
// Collect program level diagnostics
Expand All @@ -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()) {
Expand Down Expand Up @@ -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 {
Expand Down
31 changes: 25 additions & 6 deletions packages/angular_devkit/build_angular/src/builders/browser-esbuild/profiling.ts
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@

import { debugPerformance } from '../../utils/environment-options';

let cumulativeDurations: Map<string, number> | undefined;
let cumulativeDurations: Map<string, number[]> | undefined;

export function resetCumulativeDurations(): void {
cumulativeDurations?.clear();
Expand All @@ -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<string, number>();
cumulativeDurations.set(name, (cumulativeDurations.get(name) ?? 0) + duration);
cumulativeDurations ??= new Map<string, number[]>();
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`);
}
}

Expand Down