Skip to content

Commit

Permalink
perf(compiler-cli): introduce fast path for resource-only updates (#4…
Browse files Browse the repository at this point in the history
…0561)

This commit adds a new `IncrementalResourceCompilationTicket` which reuses
an existing `NgCompiler` instance and updates it to optimally process
template-only and style-only changes. Performing this update involves both
instructing `DecoratorHandler`s to react to the resource changes, as well as
invalidating `TemplateTypeChecker` state for the component(s) in question.
That way, querying the `TemplateTypeChecker` will trigger new TCB generation
for the changed template(s).

PR Close #40561
  • Loading branch information
alxhub authored and thePunderWoman committed Jan 27, 2021
1 parent 44150ec commit 156103c
Show file tree
Hide file tree
Showing 8 changed files with 198 additions and 2 deletions.
45 changes: 45 additions & 0 deletions packages/compiler-cli/src/ngtsc/annotations/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ export interface ComponentAnalysisData {

resources: ComponentResources;

/**
* The literal `styleUrls` extracted from the decorator, if present.
*/
styleUrls: string[]|null;

/**
* Inline stylesheets extracted from the decorator, if present.
*/
inlineStyles: string[]|null;

isPoisoned: boolean;
}

Expand Down Expand Up @@ -275,9 +285,11 @@ export class ComponentDecoratorHandler implements
}
}
}
let inlineStyles: string[]|null = null;
if (component.has('styles')) {
const litStyles = parseFieldArrayValue(component, 'styles', this.evaluator);
if (litStyles !== null) {
inlineStyles = [...litStyles];
if (styles === null) {
styles = litStyles;
} else {
Expand Down Expand Up @@ -333,6 +345,8 @@ export class ComponentDecoratorHandler implements
template,
providersRequiringFactory,
viewProvidersRequiringFactory,
inlineStyles,
styleUrls,
resources: {
styles: styleResources,
template: templateResource,
Expand Down Expand Up @@ -581,6 +595,37 @@ export class ComponentDecoratorHandler implements
return {data};
}

updateResources(node: ClassDeclaration, analysis: ComponentAnalysisData): void {
const containingFile = node.getSourceFile().fileName;

// If the template is external, re-parse it.
const templateDecl = analysis.template.declaration;
if (!templateDecl.isInline) {
analysis.template = this.extractTemplate(node, templateDecl);
}

// Update any external stylesheets and rebuild the combined 'styles' list.
// TODO(alxhub): write tests for styles when the primary compiler uses the updateResources path
let styles: string[] = [];
if (analysis.styleUrls !== null) {
for (const styleUrl of analysis.styleUrls) {
const resolvedStyleUrl = this.resourceLoader.resolve(styleUrl, containingFile);
const styleText = this.resourceLoader.load(resolvedStyleUrl);
styles.push(styleText);
}
}
if (analysis.inlineStyles !== null) {
for (const styleText of analysis.inlineStyles) {
styles.push(styleText);
}
}
for (const styleText of analysis.template.styles) {
styles.push(styleText);
}

analysis.meta.styles = styles;
}

compileFull(
node: ClassDeclaration, analysis: Readonly<ComponentAnalysisData>,
resolution: Readonly<ComponentResolutionData>, pool: ConstantPool): CompileResult[] {
Expand Down
53 changes: 52 additions & 1 deletion packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ interface LazyCompilationState {
export enum CompilationTicketKind {
Fresh,
IncrementalTypeScript,
IncrementalResource,
}

/**
Expand Down Expand Up @@ -93,14 +94,21 @@ export interface IncrementalTypeScriptCompilationTicket {
usePoisonedData: boolean;
}

export interface IncrementalResourceCompilationTicket {
kind: CompilationTicketKind.IncrementalResource;
compiler: NgCompiler;
modifiedResourceFiles: Set<string>;
}

/**
* A request to begin Angular compilation, either starting from scratch or from a known prior state.
*
* `CompilationTicket`s are used to initialize (or update) an `NgCompiler` instance, the core of the
* Angular compiler. They abstract the starting state of compilation and allow `NgCompiler` to be
* managed independently of any incremental compilation lifecycle.
*/
export type CompilationTicket = FreshCompilationTicket|IncrementalTypeScriptCompilationTicket;
export type CompilationTicket = FreshCompilationTicket|IncrementalTypeScriptCompilationTicket|
IncrementalResourceCompilationTicket;

/**
* Create a `CompilationTicket` for a brand new compilation, using no prior state.
Expand Down Expand Up @@ -180,6 +188,15 @@ export function incrementalFromDriverTicket(
};
}

export function resourceChangeTicket(compiler: NgCompiler, modifiedResourceFiles: Set<string>):
IncrementalResourceCompilationTicket {
return {
kind: CompilationTicketKind.IncrementalResource,
compiler,
modifiedResourceFiles,
};
}


/**
* The heart of the Angular Ivy compiler.
Expand Down Expand Up @@ -260,6 +277,10 @@ export class NgCompiler {
ticket.usePoisonedData,
perfRecorder,
);
case CompilationTicketKind.IncrementalResource:
const compiler = ticket.compiler;
compiler.updateWithChangedResources(ticket.modifiedResourceFiles);
return compiler;
}
}

Expand Down Expand Up @@ -306,6 +327,36 @@ export class NgCompiler {
this.ignoreForEmit = this.adapter.ignoreForEmit;
}

private updateWithChangedResources(changedResources: Set<string>): void {
if (this.compilation === null) {
// Analysis hasn't happened yet, so no update is necessary - any changes to resources will be
// captured by the inital analysis pass itself.
return;
}

this.resourceManager.invalidate();

const classesToUpdate = new Set<DeclarationNode>();
for (const resourceFile of changedResources) {
for (const templateClass of this.getComponentsWithTemplateFile(resourceFile)) {
classesToUpdate.add(templateClass);
}

for (const styleClass of this.getComponentsWithStyleFile(resourceFile)) {
classesToUpdate.add(styleClass);
}
}

for (const clazz of classesToUpdate) {
this.compilation.traitCompiler.updateResources(clazz);
if (!ts.isClassDeclaration(clazz)) {
continue;
}

this.compilation.templateTypeChecker.invalidateClass(clazz);
}
}

/**
* Get the resource dependencies of a file.
*
Expand Down
50 changes: 49 additions & 1 deletion packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {OptimizeFor, TypeCheckingProgramStrategy} from '../../typecheck/api';

import {NgCompilerOptions} from '../api';

import {freshCompilationTicket, NgCompiler} from '../src/compiler';
import {freshCompilationTicket, NgCompiler, resourceChangeTicket} from '../src/compiler';
import {NgCompilerHost} from '../src/host';

function makeFreshCompiler(
Expand Down Expand Up @@ -111,6 +111,7 @@ runInEachFileSystem(() => {
const program = ts.createProgram({host, options, rootNames: host.inputFiles});
const CmpA = getClass(getSourceFileOrError(program, cmpAFile), 'CmpA');
const CmpC = getClass(getSourceFileOrError(program, cmpCFile), 'CmpC');

const compiler = makeFreshCompiler(
host, options, program, new ReusedProgramStrategy(program, host, options, []),
new NoopIncrementalBuildStrategy(), /** enableTemplateTypeChecker */ false,
Expand Down Expand Up @@ -278,6 +279,53 @@ runInEachFileSystem(() => {
]));
});
});

describe('resource-only changes', () => {
it('should reuse the full compilation state for a resource-only change', () => {
const COMPONENT = _('/cmp.ts');
const TEMPLATE = _('/template.html');
fs.writeFile(COMPONENT, `
import {Component} from '@angular/core';
@Component({
selector: 'test-cmp',
templateUrl: './template.html',
})
export class Cmp {}
`);
fs.writeFile(TEMPLATE, `<h1>Resource</h1>`);

const options: NgCompilerOptions = {
strictTemplates: true,
};
const baseHost = new NgtscCompilerHost(getFileSystem(), options);
const host = NgCompilerHost.wrap(baseHost, [COMPONENT], options, /* oldProgram */ null);
const program = ts.createProgram({host, options, rootNames: host.inputFiles});
const compilerA = makeFreshCompiler(
host, options, program, new ReusedProgramStrategy(program, host, options, []),
new NoopIncrementalBuildStrategy(), /** enableTemplateTypeChecker */ false,
/* usePoisonedData */ false);

const componentSf = getSourceFileOrError(program, COMPONENT);

// There should be no diagnostics for the component.
expect(compilerA.getDiagnosticsForFile(componentSf, OptimizeFor.WholeProgram).length)
.toBe(0);

// Change the resource file and introduce an error.
fs.writeFile(TEMPLATE, `<h1>Resource</h2>`);

// Perform a resource-only incremental step.
const resourceTicket = resourceChangeTicket(compilerA, new Set([TEMPLATE]));
const compilerB = NgCompiler.fromTicket(resourceTicket, host);

// A resource-only update should reuse the same compiler instance.
expect(compilerB).toBe(compilerA);

// The new template error should be reported in component diagnostics.
expect(compilerB.getDiagnosticsForFile(componentSf, OptimizeFor.WholeProgram).length)
.toBe(1);
});
});
});
});

Expand Down
7 changes: 7 additions & 0 deletions packages/compiler-cli/src/ngtsc/resource/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ export class AdapterResourceLoader implements ResourceLoader {
return result;
}

/**
* Invalidate the entire resource cache.
*/
invalidate(): void {
this.cache.clear();
}

/**
* Attempt to resolve `url` in the context of `fromFile`, while respecting the rootDirs
* option from the tsconfig. First, normalize the file name.
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler-cli/src/ngtsc/transform/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ export interface DecoratorHandler<D, A, R> {
analyze(node: ClassDeclaration, metadata: Readonly<D>, handlerFlags?: HandlerFlags):
AnalysisOutput<A>;

/**
* React to a change in a resource file by updating the `analysis` or `resolution`, under the
* assumption that nothing in the TypeScript code has changed.
*/
updateResources?(node: ClassDeclaration, analysis: A, resolution: R): void;

/**
* Post-process the analysis of a decorator/class combination and record any necessary information
* in the larger compilation.
Expand Down
14 changes: 14 additions & 0 deletions packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,20 @@ export class TraitCompiler implements ProgramTypeCheckAdapter {
}
}

updateResources(clazz: DeclarationNode): void {
if (!this.reflector.isClass(clazz) || !this.classes.has(clazz)) {
return;
}
const record = this.classes.get(clazz)!;
for (const trait of record.traits) {
if (trait.state !== TraitState.Resolved || trait.handler.updateResources === undefined) {
continue;
}

trait.handler.updateResources(clazz, trait.analysis, trait.resolution);
}
}

compile(clazz: DeclarationNode, constantPool: ConstantPool): CompileResult[]|null {
const original = ts.getOriginalNode(clazz) as typeof clazz;
if (!this.reflector.isClass(clazz) || !this.reflector.isClass(original) ||
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ export interface TemplateTypeChecker {
* Retrieve the type checking engine's metadata for the given directive class, if available.
*/
getDirectiveMetadata(dir: ts.ClassDeclaration): TypeCheckableDirectiveMeta|null;

/**
* Reset the `TemplateTypeChecker`'s state for the given class, so that it will be recomputed on
* the next request.
*/
invalidateClass(clazz: ts.ClassDeclaration): void;
}

/**
Expand Down
19 changes: 19 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,25 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
return engine.getExpressionCompletionLocation(ast);
}

invalidateClass(clazz: ts.ClassDeclaration): void {
this.completionCache.delete(clazz);
this.symbolBuilderCache.delete(clazz);
this.scopeCache.delete(clazz);
this.elementTagCache.delete(clazz);

const sf = clazz.getSourceFile();
const sfPath = absoluteFromSourceFile(sf);
const shimPath = this.typeCheckingStrategy.shimPathForComponent(clazz);
const fileData = this.getFileData(sfPath);
const templateId = fileData.sourceManager.getTemplateId(clazz);

fileData.shimData.delete(shimPath);
fileData.isComplete = false;
fileData.templateOverrides?.delete(templateId);

this.isComplete = false;
}

private getOrCreateCompletionEngine(component: ts.ClassDeclaration): CompletionEngine|null {
if (this.completionCache.has(component)) {
return this.completionCache.get(component)!;
Expand Down

0 comments on commit 156103c

Please sign in to comment.