perf(language-service): update NgCompiler via resource-only path when able (#40585)

This commit changes the Language Service's "compiler factory" mechanism to
leverage the new resource-only update path for `NgCompiler`. When an
incoming change only affects a resource file like a component template or
stylesheet, going through the new API allows the Language Service to avoid
unnecessary incremental steps of the `NgCompiler` and return answers more
efficiently.

PR Close #40585
This commit is contained in:
Alex Rickabaugh 2021-01-21 15:54:40 -08:00 committed by Misko Hevery
parent 11ca2f04f9
commit e3bd23c915
10 changed files with 98 additions and 141 deletions

View File

@ -25,7 +25,13 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
readonly factoryTracker = null; // no .ngfactory shims
readonly unifiedModulesHost = null; // only used in Bazel
readonly rootDirs: AbsoluteFsPath[];
private readonly templateVersion = new Map<string, string>();
/**
* Map of resource filenames to the version of the file last read via `readResource`.
*
* Used to implement `getModifiedResourceFiles`.
*/
private readonly lastReadResourceVersion = new Map<string, string>();
constructor(private readonly project: ts.server.Project) {
this.rootDirs = getRootDirs(this, project.getCompilationSettings());
@ -81,14 +87,18 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
throw new Error(`Failed to get script snapshot while trying to read ${fileName}`);
}
const version = this.project.getScriptVersion(fileName);
this.templateVersion.set(fileName, version);
this.lastReadResourceVersion.set(fileName, version);
return snapshot.getText(0, snapshot.getLength());
}
isTemplateDirty(fileName: string): boolean {
const lastVersion = this.templateVersion.get(fileName);
const latestVersion = this.project.getScriptVersion(fileName);
return lastVersion !== latestVersion;
getModifiedResourceFiles(): Set<string>|undefined {
const modifiedFiles = new Set<string>();
for (const [fileName, oldVersion] of this.lastReadResourceVersion) {
if (this.project.getScriptVersion(fileName) !== oldVersion) {
modifiedFiles.add(fileName);
}
}
return modifiedFiles.size > 0 ? modifiedFiles : undefined;
}
}

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {CompilationTicket, freshCompilationTicket, incrementalFromCompilerTicket, NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
import {CompilationTicket, freshCompilationTicket, incrementalFromCompilerTicket, NgCompiler, resourceChangeTicket} from '@angular/compiler-cli/src/ngtsc/core';
import {NgCompilerOptions} from '@angular/compiler-cli/src/ngtsc/core/api';
import {TrackedIncrementalBuildStrategy} from '@angular/compiler-cli/src/ngtsc/incremental';
import {TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
@ -37,53 +37,32 @@ export class CompilerFactory {
getOrCreate(): NgCompiler {
const program = this.programStrategy.getProgram();
if (this.compiler === null || program !== this.lastKnownProgram) {
const modifiedResourceFiles = this.adapter.getModifiedResourceFiles() ?? new Set();
if (this.compiler !== null && program === this.lastKnownProgram) {
if (modifiedResourceFiles.size > 0) {
// Only resource files have changed since the last NgCompiler was created.
const ticket = resourceChangeTicket(this.compiler, modifiedResourceFiles);
this.compiler = NgCompiler.fromTicket(ticket, this.adapter);
}
return this.compiler;
}
let ticket: CompilationTicket;
if (this.compiler === null || this.lastKnownProgram === null) {
ticket = freshCompilationTicket(
program, this.options, this.incrementalStrategy, this.programStrategy, true, true);
} else {
ticket = incrementalFromCompilerTicket(
this.compiler, program, this.incrementalStrategy, this.programStrategy, new Set());
this.compiler, program, this.incrementalStrategy, this.programStrategy,
modifiedResourceFiles);
}
this.compiler = NgCompiler.fromTicket(ticket, this.adapter);
this.lastKnownProgram = program;
}
return this.compiler;
}
/**
* Create a new instance of the Ivy compiler if the program has changed since
* the last time the compiler was instantiated. If the program has not changed,
* return the existing instance.
* @param fileName override the template if this is an external template file
* @param options angular compiler options
*/
getOrCreateWithChangedFile(fileName: string): NgCompiler {
const compiler = this.getOrCreate();
if (isExternalTemplate(fileName)) {
this.overrideTemplate(fileName, compiler);
}
return compiler;
}
private overrideTemplate(fileName: string, compiler: NgCompiler) {
if (!this.adapter.isTemplateDirty(fileName)) {
return;
}
// 1. Get the latest snapshot
const latestTemplate = this.adapter.readResource(fileName);
// 2. Find all components that use the template
const ttc = compiler.getTemplateTypeChecker();
const components = compiler.getComponentsWithTemplateFile(fileName);
// 3. Update component template
for (const component of components) {
if (ts.isClassDeclaration(component)) {
ttc.overrideComponentTemplate(component, latestTemplate);
}
}
}
registerLastKnownProgram() {
this.lastKnownProgram = this.programStrategy.getProgram();
}

View File

@ -67,7 +67,7 @@ export class LanguageService {
}
getSemanticDiagnostics(fileName: string): ts.Diagnostic[] {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const ttc = compiler.getTemplateTypeChecker();
const diagnostics: ts.Diagnostic[] = [];
if (isTypeScriptFile(fileName)) {
@ -90,7 +90,7 @@ export class LanguageService {
getDefinitionAndBoundSpan(fileName: string, position: number): ts.DefinitionInfoAndBoundSpan
|undefined {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const results =
new DefinitionBuilder(this.tsLS, compiler).getDefinitionAndBoundSpan(fileName, position);
this.compilerFactory.registerLastKnownProgram();
@ -99,7 +99,7 @@ export class LanguageService {
getTypeDefinitionAtPosition(fileName: string, position: number):
readonly ts.DefinitionInfo[]|undefined {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const results =
new DefinitionBuilder(this.tsLS, compiler).getTypeDefinitionsAtPosition(fileName, position);
this.compilerFactory.registerLastKnownProgram();
@ -107,7 +107,7 @@ export class LanguageService {
}
getQuickInfoAtPosition(fileName: string, position: number): ts.QuickInfo|undefined {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const templateInfo = getTemplateInfoAtPosition(fileName, position, compiler);
if (templateInfo === undefined) {
return undefined;
@ -129,7 +129,7 @@ export class LanguageService {
}
getReferencesAtPosition(fileName: string, position: number): ts.ReferenceEntry[]|undefined {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const results = new ReferencesAndRenameBuilder(this.strategy, this.tsLS, compiler)
.getReferencesAtPosition(fileName, position);
this.compilerFactory.registerLastKnownProgram();
@ -137,7 +137,7 @@ export class LanguageService {
}
getRenameInfo(fileName: string, position: number): ts.RenameInfo {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const renameInfo = new ReferencesAndRenameBuilder(this.strategy, this.tsLS, compiler)
.getRenameInfo(absoluteFrom(fileName), position);
if (!renameInfo.canRename) {
@ -152,7 +152,7 @@ export class LanguageService {
}
findRenameLocations(fileName: string, position: number): readonly ts.RenameLocation[]|undefined {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const results = new ReferencesAndRenameBuilder(this.strategy, this.tsLS, compiler)
.findRenameLocations(fileName, position);
this.compilerFactory.registerLastKnownProgram();
@ -161,7 +161,7 @@ export class LanguageService {
private getCompletionBuilder(fileName: string, position: number):
CompletionBuilder<TmplAstNode|AST>|null {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
const compiler = this.compilerFactory.getOrCreate();
const templateInfo = getTemplateInfoAtPosition(fileName, position, compiler);
if (templateInfo === undefined) {
return null;
@ -219,7 +219,7 @@ export class LanguageService {
}
getTcb(fileName: string, position: number): GetTcbResponse {
return this.withCompiler<GetTcbResponse>(fileName, compiler => {
return this.withCompiler<GetTcbResponse>(compiler => {
const templateInfo = getTemplateInfoAtPosition(fileName, position, compiler);
if (templateInfo === undefined) {
return undefined;
@ -263,8 +263,8 @@ export class LanguageService {
});
}
private withCompiler<T>(fileName: string, p: (compiler: NgCompiler) => T): T {
const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
private withCompiler<T>(p: (compiler: NgCompiler) => T): T {
const compiler = this.compilerFactory.getOrCreate();
const result = p(compiler);
this.compilerFactory.registerLastKnownProgram();
return result;

View File

@ -17,6 +17,38 @@ describe('language-service/compiler integration', () => {
initMockFileSystem('Native');
});
it('should react to a change in an external template', () => {
const cmpFile = absoluteFrom('/test.ts');
const tmplFile = absoluteFrom('/test.html');
const env = LanguageServiceTestEnvironment.setup([
{
name: cmpFile,
contents: `
import {Component} from '@angular/core';
@Component({
selector: 'test-cmp',
templateUrl: './test.html',
})
export class TestCmp {}
`,
isRoot: true,
},
{
name: tmplFile,
contents: '<other-cmp>Test</other-cmp>',
},
]);
const diags = env.ngLS.getSemanticDiagnostics(cmpFile);
expect(diags.length).toBeGreaterThan(0);
env.updateFile(tmplFile, '<div>Test</div>');
const afterDiags = env.ngLS.getSemanticDiagnostics(cmpFile);
expect(afterDiags.length).toBe(0);
});
it('should not produce errors from inline test declarations mixing with those of the app', () => {
const appCmpFile = absoluteFrom('/test.cmp.ts');
const appModuleFile = absoluteFrom('/test.mod.ts');

View File

@ -6,14 +6,13 @@
* found in the LICENSE file at https://angular.io/license
*/
import {TmplAstNode} from '@angular/compiler';
import {absoluteFrom, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
import * as ts from 'typescript';
import {DisplayInfoKind, unsafeCastDisplayInfoKindToScriptElementKind} from '../display_parts';
import {LanguageService} from '../language_service';
import {LanguageServiceTestEnvironment} from './env';
import {extractCursorInfo, LanguageServiceTestEnvironment} from './env';
const DIR_WITH_INPUT = {
'Dir': `
@ -640,7 +639,6 @@ function setup(
AppCmp: ts.ClassDeclaration,
ngLS: LanguageService,
cursor: number,
nodes: TmplAstNode[],
text: string,
} {
const codePath = absoluteFrom('/test.ts');
@ -650,6 +648,7 @@ function setup(
const otherDirectiveClassDecls = Object.values(otherDeclarations).join('\n\n');
const {cursor, text: templateWithoutCursor} = extractCursorInfo(templateWithCursor);
const env = LanguageServiceTestEnvironment.setup([
{
name: codePath,
@ -675,18 +674,15 @@ function setup(
},
{
name: templatePath,
contents: 'Placeholder template',
contents: templateWithoutCursor,
}
]);
const {nodes, cursor, text} =
env.overrideTemplateWithCursor(codePath, 'AppCmp', templateWithCursor);
return {
env,
fileName: templatePath,
AppCmp: env.getClass(codePath, 'AppCmp'),
ngLS: env.ngLS,
nodes,
text,
text: templateWithoutCursor,
cursor,
};
}

View File

@ -27,8 +27,8 @@ describe('definitions', () => {
};
const env = createModuleWithDeclarations([appFile], [templateFile]);
const {cursor} = env.overrideTemplateWithCursor(
absoluteFrom('/app.ts'), 'AppCmp', '<input #myInput /> {{myIn¦put.value}}');
const {cursor} = env.updateFileWithCursor(
absoluteFrom('/app.html'), '<input #myInput /> {{myIn¦put.value}}');
env.expectNoSourceDiagnostics();
const {definitions} = env.ngLS.getDefinitionAndBoundSpan(absoluteFrom('/app.html'), cursor)!;
expect(definitions![0].name).toEqual('myInput');

View File

@ -46,13 +46,6 @@ function writeTsconfig(
export type TestableOptions = StrictTemplateOptions;
export interface TemplateOverwriteResult {
cursor: number;
nodes: TmplAstNode[];
component: ts.ClassDeclaration;
text: string;
}
export class LanguageServiceTestEnvironment {
private constructor(
readonly tsLS: ts.LanguageService, readonly ngLS: LanguageService,
@ -118,26 +111,16 @@ export class LanguageServiceTestEnvironment {
return getClassOrError(sf, className);
}
overrideTemplateWithCursor(fileName: AbsoluteFsPath, className: string, contents: string):
TemplateOverwriteResult {
const program = this.tsLS.getProgram();
if (program === undefined) {
throw new Error(`Expected to get a ts.Program`);
}
const sf = getSourceFileOrError(program, fileName);
const component = getClassOrError(sf, className);
const ngCompiler = this.ngLS.compilerFactory.getOrCreate();
const templateTypeChecker = ngCompiler.getTemplateTypeChecker();
updateFileWithCursor(fileName: AbsoluteFsPath, contents: string): {cursor: number, text: string} {
const {cursor, text} = extractCursorInfo(contents);
const {nodes} = templateTypeChecker.overrideComponentTemplate(component, text);
return {cursor, nodes, component, text};
this.updateFile(fileName, text);
return {cursor, text};
}
updateFile(fileName: AbsoluteFsPath, contents: string): void {
const scriptInfo = this.projectService.getScriptInfo(fileName);
const normalFileName = ts.server.toNormalizedPath(fileName);
const scriptInfo =
this.projectService.getOrCreateScriptInfoForNormalizedPath(normalFileName, true, '');
if (scriptInfo === undefined) {
throw new Error(`Could not find a file named ${fileName}`);
}

View File

@ -1,41 +0,0 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import * as ts from 'typescript/lib/tsserverlibrary';
import {LanguageServiceAdapter} from '../../adapters';
import {MockService, setup, TEST_TEMPLATE} from './mock_host';
describe('Language service adapter', () => {
let project: ts.server.Project;
let service: MockService;
beforeAll(() => {
const {project: _project, service: _service} = setup();
project = _project;
service = _service;
});
it('should mark template dirty if it has not seen the template before', () => {
const adapter = new LanguageServiceAdapter(project);
expect(adapter.isTemplateDirty(TEST_TEMPLATE)).toBeTrue();
});
it('should not mark template dirty if template has not changed', () => {
const adapter = new LanguageServiceAdapter(project);
adapter.readResource(TEST_TEMPLATE);
expect(adapter.isTemplateDirty(TEST_TEMPLATE)).toBeFalse();
});
it('should mark template dirty if template has changed', () => {
const adapter = new LanguageServiceAdapter(project);
service.overwrite(TEST_TEMPLATE, '<p>Hello World</p>');
expect(adapter.isTemplateDirty(TEST_TEMPLATE)).toBeTrue();
});
});

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
import {absoluteFrom, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
import * as ts from 'typescript/lib/tsserverlibrary';
@ -476,8 +476,8 @@ describe('quick info', () => {
});
it('should provide documentation', () => {
const {cursor} = env.overrideTemplateWithCursor(
absoluteFrom('/app.ts'), 'AppCmp', `<div>{{¦title}}</div>`);
const {cursor} =
env.updateFileWithCursor(absoluteFrom('/app.html'), `<div>{{¦title}}</div>`);
const quickInfo = env.ngLS.getQuickInfoAtPosition(absoluteFrom('/app.html'), cursor);
const documentation = toText(quickInfo!.documentation);
expect(documentation).toBe('This is the title of the `AppCmp` Component.');
@ -522,8 +522,7 @@ describe('quick info', () => {
{templateOverride, expectedSpanText, expectedDisplayString}:
{templateOverride: string, expectedSpanText: string, expectedDisplayString: string}):
ts.QuickInfo {
const {cursor, text} =
env.overrideTemplateWithCursor(absoluteFrom('/app.ts'), 'AppCmp', templateOverride);
const {cursor, text} = env.updateFileWithCursor(absoluteFrom('/app.html'), templateOverride);
env.expectNoSourceDiagnostics();
env.expectNoTemplateDiagnostics(absoluteFrom('/app.ts'), 'AppCmp');
const quickInfo = env.ngLS.getQuickInfoAtPosition(absoluteFrom('/app.html'), cursor);

View File

@ -49,8 +49,7 @@ describe('type definitions', () => {
});
function getTypeDefinitionsAndAssertBoundSpan({templateOverride}: {templateOverride: string}) {
const {cursor, text} =
env.overrideTemplateWithCursor(absoluteFrom('/app.ts'), 'AppCmp', templateOverride);
const {cursor, text} = env.updateFileWithCursor(absoluteFrom('/app.html'), templateOverride);
env.expectNoSourceDiagnostics();
env.expectNoTemplateDiagnostics(absoluteFrom('/app.ts'), 'AppCmp');
const defs = env.ngLS.getTypeDefinitionAtPosition(absoluteFrom('/app.html'), cursor);