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.

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

perf(@ngtools/webpack): improve rebuild performance #4188

Merged
merged 1 commit into from
Jan 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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] = true;
}

fileExists(fileName: string): boolean {
Expand Down
66 changes: 66 additions & 0 deletions packages/@ngtools/webpack/src/lazy_routes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import {dirname, join} from 'path';
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.name) == '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.ResolvedModuleWithFailedLookupLocations = moduleName[0] == '.'
? { resolvedModule: { resolvedFileName: join(dirname(filePath), moduleName) + '.ts' },
failedLookupLocations: [] }
: ts.resolveModuleName(moduleName, filePath, program.getCompilerOptions(), host);
if (resolvedModuleName.resolvedModule
&& resolvedModuleName.resolvedModule.resolvedFileName
&& host.fileExists(resolvedModuleName.resolvedModule.resolvedFileName)) {
return [routePath, resolvedModuleName.resolvedModule.resolvedFileName];
} else {
return [routePath, null];
}
})
// Reduce to the LazyRouteMap map.
.reduce((acc: LazyRouteMap, [routePath, resolvedModuleName]: [string, string | null]) => {
acc[routePath] = resolvedModuleName;
return acc;
}, {});
}
77 changes: 64 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: LazyRouteMap = 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 = 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,31 @@ export class AotPlugin implements Tapable {
}
}

private _findLazyRoutesInAst(): LazyRouteMap {
const result: LazyRouteMap = Object.create(null);
const changedFilePaths = this._compilerHost.getChangedFilePaths();
for (const filePath of 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) {
this._compilation.warnings.push(
new Error(`Duplicated path in loadChildren detected during a rebuild. `
+ `We will take the latest version detected and override it to save rebuild time. `
+ `You should perform a full build to validate that your routes don't overlap.`)
);
}
} else {
result[routeKey] = route;
}
}
}
return result;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result will always be an empty object, it's not modified after it's created.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops.

}

// registration hook for webpack plugin
apply(compiler: any) {
this._compiler = compiler;
Expand All @@ -220,7 +249,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 @@ -314,18 +351,26 @@ export class AotPlugin implements Tapable {
this._compilerHost.populateWebpackResolver(this._compiler.resolvers.normal);
})
.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 (lazyRoute === null) {
return;
}

if (this.skipCodeGeneration) {
this._lazyRoutes[k] = lazyRoute;
} else {
Expand All @@ -334,7 +379,13 @@ export class AotPlugin implements Tapable {
}
});
})
.then(() => cb(), (err: any) => {
.then(() => {
this._compilerHost.resetChangedFileTracker();

// Only turn this off for the first successful run.
this._firstRun = false;
cb();
}, (err: any) => {
compilation.errors.push(err);
cb();
});
Expand Down
2 changes: 1 addition & 1 deletion packages/angular-cli/models/webpack-configs/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function getCommonConfig(wco: WebpackConfigOptions) {
extraPlugins.push(new webpack.optimize.CommonsChunkPlugin({
name: 'vendor',
chunks: ['main'],
minChunks: (module: any) => module.userRequest && module.userRequest.startsWith(nodeModules)
minChunks: (module: any) => module.resource && module.resource.startsWith(nodeModules)
}));
}

Expand Down
73 changes: 73 additions & 0 deletions tests/e2e/tests/build/rebuild.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import {
killAllProcesses,
exec,
waitForAnyProcessOutputToMatch,
silentExecAndWaitForOutputToMatch,
ng, execAndWaitForOutputToMatch,
} from '../../utils/process';
import {writeFile} from '../../utils/fs';
import {wait} from '../../utils/utils';


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

let oldNumberOfChunks = 0;
const chunkRegExp = /chunk\s+\{/g;

return execAndWaitForOutputToMatch('ng', ['serve'], /webpack: bundle is now VALID/)
// Should trigger a rebuild.
.then(() => exec('touch', 'src/main.ts'))
.then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now INVALID/, 1000))
.then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000))
// Count the bundles.
.then((stdout: string) => {
oldNumberOfChunks = stdout.split(chunkRegExp).length;
})
// Add a lazy module.
.then(() => ng('generate', 'module', 'lazy', '--routing'))
// Just wait for the rebuild, otherwise we might be validating this build.
.then(() => wait(1000))
.then(() => writeFile('src/app/app.module.ts', `
import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { FormsModule } from '@angular/forms';
import { HttpModule } from '@angular/http';

import { AppComponent } from './app.component';
import { RouterModule } from '@angular/router';

@NgModule({
declarations: [
AppComponent
],
imports: [
BrowserModule,
FormsModule,
HttpModule,
RouterModule.forRoot([
{ path: 'lazy', loadChildren: './lazy/lazy.module#LazyModule' }
])
],
providers: [],
bootstrap: [AppComponent]
})
export class AppModule { }
`))
// Should trigger a rebuild with a new bundle.
.then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now INVALID/, 1000))
.then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000))
// Count the bundles.
.then((stdout: string) => {
let newNumberOfChunks = stdout.split(chunkRegExp).length;
if (oldNumberOfChunks >= newNumberOfChunks) {
throw new Error('Expected webpack to create a new chunk, but did not.');
}
})
.then(() => killAllProcesses(), (err: any) => {
killAllProcesses();
throw err;
});
}
1 change: 1 addition & 0 deletions tests/e2e/tests/packages/webpack/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ export default function(skipCleaning: () => void) {
.then(() => replaceInFile('app/app.component.ts',
'./app.component.scss', 'app.component.scss'))
.then(() => exec(normalize('node_modules/.bin/webpack'), '-p'))
// test
.then(() => skipCleaning());
}
20 changes: 20 additions & 0 deletions tests/e2e/utils/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,26 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<strin
});
}

export function waitForAnyProcessOutputToMatch(match: RegExp, timeout = 30000): Promise<string> {
// Race between _all_ processes, and the timeout. First one to resolve/reject wins.
return Promise.race(_processes.map(childProcess => new Promise(resolve => {
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(new Error(`Waiting for ${match} timed out (timeout: ${timeout}msec)...`));
}, timeout);
})
]));
}

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