Skip to content

Commit

Permalink
perf(@ngtools/webpack): improve rebuild performance
Browse files Browse the repository at this point in the history
We need to run full static analysis on the first build to discover routes
in node_modules, but in rebuilding the app we just need to look for the
changed files.

This introduces a subtle bug though; if the module structure changes, we
might be missing lazy routes if those are in modules imported but
weren't in the original structure.

This is, for now, considered "okay" as it's a relatively rare case. We
should probably output a warning though.
  • Loading branch information
hansl committed Jan 24, 2017
1 parent 43c6861 commit c09100e
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 25 deletions.
31 changes: 25 additions & 6 deletions packages/@ngtools/webpack/src/compiler_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ export class WebpackCompilerHost implements ts.CompilerHost {
private _delegate: ts.CompilerHost;
private _files: {[path: string]: VirtualFileStats} = Object.create(null);
private _directories: {[path: string]: VirtualDirStats} = Object.create(null);
private _changed = false;

private _changedFiles: {[path: string]: boolean} = Object.create(null);
private _changedDirs: {[path: string]: boolean} = Object.create(null);

private _basePath: string;
private _setParentNodes: boolean;
Expand Down Expand Up @@ -129,10 +131,15 @@ export class WebpackCompilerHost implements ts.CompilerHost {
let p = dirname(fileName);
while (p && !this._directories[p]) {
this._directories[p] = new VirtualDirStats(p);
this._changedDirs[p] = true;
p = dirname(p);
}

this._changed = true;
this._changedFiles[fileName] = true;
}

get dirty() {
return Object.keys(this._changedFiles).length > 0;
}

enableCaching() {
Expand All @@ -141,21 +148,26 @@ export class WebpackCompilerHost implements ts.CompilerHost {

populateWebpackResolver(resolver: any) {
const fs = resolver.fileSystem;
if (!this._changed) {
if (!this.dirty) {
return;
}

const isWindows = process.platform.startsWith('win');
for (const fileName of Object.keys(this._files)) {
for (const fileName of this.getChangedFilePaths()) {
const stats = this._files[fileName];
if (stats) {
// If we're on windows, we need to populate with the proper path separator.
const path = isWindows ? fileName.replace(/\//g, '\\') : fileName;
fs._statStorage.data[path] = [null, stats];
fs._readFileStorage.data[path] = [null, stats.content];
} else {
// Support removing files as well.
const path = isWindows ? fileName.replace(/\//g, '\\') : fileName;
fs._statStorage.data[path] = [new Error(), null];
fs._readFileStorage.data[path] = [new Error(), null];
}
}
for (const dirName of Object.keys(this._directories)) {
for (const dirName of Object.keys(this._changedDirs)) {
const stats = this._directories[dirName];
const dirs = this.getDirectories(dirName);
const files = this.getFiles(dirName);
Expand All @@ -164,12 +176,19 @@ export class WebpackCompilerHost implements ts.CompilerHost {
fs._statStorage.data[path] = [null, stats];
fs._readdirStorage.data[path] = [null, files.concat(dirs)];
}
}

this._changed = false;
resetChangedFileTracker() {
this._changedFiles = Object.create(null);
this._changedDirs = Object.create(null);
}
getChangedFilePaths(): string[] {
return Object.keys(this._changedFiles);
}

invalidate(fileName: string): void {
this._files[fileName] = null;
this._changedFiles[fileName] = false;
}

fileExists(fileName: string): boolean {
Expand Down
63 changes: 63 additions & 0 deletions packages/@ngtools/webpack/src/lazy_routes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import * as ts from 'typescript';

import {TypeScriptFileRefactor} from './refactor';


function _getContentOfKeyLiteral(source: ts.SourceFile, node: ts.Node): string {
if (node.kind == ts.SyntaxKind.Identifier) {
return (node as ts.Identifier).text;
} else if (node.kind == ts.SyntaxKind.StringLiteral) {
return (node as ts.StringLiteral).text;
} else {
return null;
}
}


export interface LazyRouteMap {
[path: string]: string;
}


export function findLazyRoutes(filePath: string,
program: ts.Program,
host: ts.CompilerHost): LazyRouteMap {
const refactor = new TypeScriptFileRefactor(filePath, host, program);

return refactor
// Find all object literals in the file.
.findAstNodes(null, ts.SyntaxKind.ObjectLiteralExpression, true)
// Get all their property assignments.
.map((node: ts.ObjectLiteralExpression) => {
return refactor.findAstNodes(node, ts.SyntaxKind.PropertyAssignment, false);
})
// Take all `loadChildren` elements.
.reduce((acc: ts.PropertyAssignment[], props: ts.PropertyAssignment[]) => {
return acc.concat(props.filter(literal => {
return _getContentOfKeyLiteral(refactor.sourceFile, literal) == 'loadChildren';
}));
}, [])
// Get only string values.
.filter((node: ts.PropertyAssignment) => node.initializer.kind == ts.SyntaxKind.StringLiteral)
// Get the string value.
.map((node: ts.PropertyAssignment) => (node.initializer as ts.StringLiteral).text)
// Map those to either [path, absoluteModulePath], or [path, null] if the module pointing to
// does not exist.
.map((routePath: string) => {
const moduleName = routePath.split('#')[0];
const resolvedModuleName = ts.resolveModuleName(
moduleName, filePath, program.getCompilerOptions(), host);
if (resolvedModuleName.resolvedModule
&& resolvedModuleName.resolvedModule.resolvedFileName
&& host.fileExists(resolvedModuleName.resolvedModule.resolvedFileName)) {
return [routePath, resolvedModuleName];
} else {
return [routePath, null];
}
})
// Reduce to the LazyRouteMap map.
.reduce((acc: LazyRouteMap, [routePath, resolvedModuleName]: [string, string | null]) => {
acc[routePath] = resolvedModuleName;
return acc;
}, {});
}
69 changes: 56 additions & 13 deletions packages/@ngtools/webpack/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {WebpackCompilerHost} from './compiler_host';
import {resolveEntryModuleFromMain} from './entry_resolver';
import {Tapable} from './webpack';
import {PathsPlugin} from './paths-plugin';
import {findLazyRoutes, LazyRouteMap} from './lazy_routes';


/**
Expand Down Expand Up @@ -39,7 +40,7 @@ export class AotPlugin implements Tapable {
private _rootFilePath: string[];
private _compilerHost: WebpackCompilerHost;
private _resourceLoader: WebpackResourceLoader;
private _lazyRoutes: { [route: string]: string };
private _lazyRoutes: { [route: string]: string } = Object.create(null);
private _tsConfigPath: string;
private _entryModule: string;

Expand All @@ -56,6 +57,8 @@ export class AotPlugin implements Tapable {
private _i18nFormat: string;
private _locale: string;

private _firstRun: boolean = true;

constructor(options: AotPluginOptions) {
this._setupOptions(options);
}
Expand All @@ -78,6 +81,7 @@ export class AotPlugin implements Tapable {
get i18nFile() { return this._i18nFile; }
get i18nFormat() { return this._i18nFormat; }
get locale() { return this._locale; }
get firstRun() { return this._firstRun; }

private _setupOptions(options: AotPluginOptions) {
// Fill in the missing options.
Expand Down Expand Up @@ -194,6 +198,28 @@ export class AotPlugin implements Tapable {
}
}

private _findLazyRoutesInAst(): LazyRouteMap {
const result: LazyRouteMap = Object.create(null);
const changedFilePaths = this._compilerHost.getChangedFilePaths();
for (const filePath in changedFilePaths) {
const fileLazyRoutes = findLazyRoutes(filePath, this._program, this._compilerHost);
for (const routeKey of Object.keys(fileLazyRoutes)) {
const route = fileLazyRoutes[routeKey];
if (routeKey in this._lazyRoutes) {
if (route === null) {
this._lazyRoutes[routeKey] = null;
} else if (this._lazyRoutes[routeKey] !== route) {
throw new Error(`Duplicated path in loadChildren detected: "${routeKey}" is used in 2 `
+ `loadChildren, but they point to different modules `
+ `("${this._lazyRoutes[routeKey]}" and "${route}"). Webpack cannot `
+ `distinguish on context and would fail to load the proper one.`);
}
}
}
}
return result;
}

// registration hook for webpack plugin
apply(compiler: any) {
this._compiler = compiler;
Expand All @@ -220,7 +246,15 @@ export class AotPlugin implements Tapable {
result.dependencies.forEach((d: any) => d.critical = false);
result.resolveDependencies = (p1: any, p2: any, p3: any, p4: RegExp, cb: any ) => {
const dependencies = Object.keys(this._lazyRoutes)
.map((key) => new ContextElementDependency(this._lazyRoutes[key], key));
.map((key) => {
const value = this._lazyRoutes[key];
if (value !== null) {
return new ContextElementDependency(value, key);
} else {
return null;
}
})
.filter(x => !!x);
cb(null, dependencies);
};
return callback(null, result);
Expand Down Expand Up @@ -312,19 +346,24 @@ export class AotPlugin implements Tapable {
.then(() => {
// Populate the file system cache with the virtual module.
this._compilerHost.populateWebpackResolver(this._compiler.resolvers.normal);
this._compilerHost.resetChangedFileTracker();
})
.then(() => {
// Process the lazy routes
this._lazyRoutes = {};
const allLazyRoutes = __NGTOOLS_PRIVATE_API_2.listLazyRoutes({
program: this._program,
host: this._compilerHost,
angularCompilerOptions: this._angularCompilerOptions,
entryModule: this._entryModule
});
Object.keys(allLazyRoutes)
// We need to run the `listLazyRoutes` the first time because it also navigates libraries
// and other things that we might miss using the findLazyRoutesInAst.
let discoveredLazyRoutes: LazyRouteMap = this.firstRun
? __NGTOOLS_PRIVATE_API_2.listLazyRoutes({
program: this._program,
host: this._compilerHost,
angularCompilerOptions: this._angularCompilerOptions,
entryModule: this._entryModule
})
: this._findLazyRoutesInAst();

// Process the lazy routes discovered.
Object.keys(discoveredLazyRoutes)
.forEach(k => {
const lazyRoute = allLazyRoutes[k];
const lazyRoute = discoveredLazyRoutes[k];
k = k.split('#')[0];
if (this.skipCodeGeneration) {
this._lazyRoutes[k] = lazyRoute;
Expand All @@ -334,7 +373,11 @@ export class AotPlugin implements Tapable {
}
});
})
.then(() => cb(), (err: any) => {
.then(() => {
this._firstRun = false;
cb();
}, (err: any) => {
this._firstRun = false;
compilation.errors.push(err);
cb();
});
Expand Down
18 changes: 12 additions & 6 deletions packages/angular-cli/models/webpack-build-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function getWebpackCommonConfig(
) {

const appRoot = path.resolve(projectRoot, appConfig.root);
const nodeModules = path.resolve(projectRoot, 'node_modules');
const nodeModules = path.join(projectRoot, 'node_modules');

let extraPlugins: any[] = [];
let extraRules: any[] = [];
Expand All @@ -56,6 +56,8 @@ export function getWebpackCommonConfig(
entryPoints['polyfills'] = [path.resolve(appRoot, appConfig.polyfills)];
}

entryPoints['angular'] = [path.resolve(appRoot, '../node_modules/@angular/core/index.js')];

// determine hashing format
const hashFormat = getOutputHashFormat(outputHashing);

Expand All @@ -72,11 +74,15 @@ export function getWebpackCommonConfig(
}

if (vendorChunk) {
extraPlugins.push(new webpack.optimize.CommonsChunkPlugin({
name: 'vendor',
chunks: ['main'],
minChunks: (module: any) => module.userRequest && module.userRequest.startsWith(nodeModules)
}));
extraPlugins.push(
new webpack.optimize.CommonsChunkPlugin({
name: 'vendor',
chunks: ['main'],
minChunks: (module: any) => {
return module.resource && module.resource.startsWith(nodeModules);
}
}),
);
}

// process environment file replacement
Expand Down
19 changes: 19 additions & 0 deletions tests/e2e/tests/build/rebuild.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {killAllProcesses, exec, waitForAnyProcessOutputToMatch} from '../../utils/process';
import {ngServe} from '../../utils/project';
import {request} from '../../utils/http';


export default function() {
if (process.platform.startsWith('win')) {
return Promise.resolve();
}

return ngServe()
.then(() => exec('touch', 'src/main.ts'))
.then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 10000))
.then(() => request('http://localhost:4200/'))
.then(() => killAllProcesses(), (err: any) => {
killAllProcesses();
throw err;
});
}
18 changes: 18 additions & 0 deletions tests/e2e/utils/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,24 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<strin
});
}

export function waitForAnyProcessOutputToMatch(match: RegExp, timeout = 30000) {
// Race between _all_ processes, and the timeout. First one to resolve/reject wins.
return Promise.race(_processes.map(childProcess => new Promise((resolve, reject) => {
let stdout = '';
childProcess.stdout.on('data', (data: Buffer) => {
stdout += data.toString();
if (data.toString().match(match)) {
resolve(stdout);
}
});
})).concat([
new Promise((resolve, reject) => {
// Wait for 30 seconds and timeout.
setTimeout(reject, timeout);
})
]));
}

export function killAllProcesses(signal = 'SIGTERM') {
_processes.forEach(process => treeKill(process.pid, signal));
_processes = [];
Expand Down

0 comments on commit c09100e

Please sign in to comment.