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

fix(language-service): improve performance of updateModuleAnalysis() #15543

Merged
merged 1 commit into from Mar 28, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 18 additions & 11 deletions packages/compiler-cli/src/compiler_host.ts
Expand Up @@ -33,6 +33,7 @@ export class CompilerHost implements AotCompilerHost {
private resolverCache = new Map<string, ModuleMetadata[]>();
private bundleIndexCache = new Map<string, boolean>();
private bundleIndexNames = new Set<string>();
private moduleFileNames = new Map<string, string>();
protected resolveModuleNameHost: CompilerHostContext;

constructor(
Expand Down Expand Up @@ -69,19 +70,25 @@ export class CompilerHost implements AotCompilerHost {
getCanonicalFileName(fileName: string): string { return fileName; }

moduleNameToFileName(m: string, containingFile: string): string|null {
if (!containingFile || !containingFile.length) {
if (m.indexOf('.') === 0) {
throw new Error('Resolution of relative paths requires a containing file.');
const key = m + ':' + (containingFile || '');
let result = this.moduleFileNames.get(key);
if (!result) {
if (!containingFile || !containingFile.length) {
if (m.indexOf('.') === 0) {
throw new Error('Resolution of relative paths requires a containing file.');
}
// Any containing file gives the same result for absolute imports
containingFile = this.getCanonicalFileName(path.join(this.basePath, 'index.ts'));
}
// Any containing file gives the same result for absolute imports
containingFile = this.getCanonicalFileName(path.join(this.basePath, 'index.ts'));
m = m.replace(EXT, '');
const resolved =
ts.resolveModuleName(
m, containingFile.replace(/\\/g, '/'), this.options, this.resolveModuleNameHost)
.resolvedModule;
result = resolved ? this.getCanonicalFileName(resolved.resolvedFileName) : null;
this.moduleFileNames.set(key, result);
}
m = m.replace(EXT, '');
const resolved =
ts.resolveModuleName(
m, containingFile.replace(/\\/g, '/'), this.options, this.resolveModuleNameHost)
.resolvedModule;
return resolved ? this.getCanonicalFileName(resolved.resolvedFileName) : null;
return result;
};

/**
Expand Down
21 changes: 21 additions & 0 deletions packages/compiler/src/aot/static_symbol_resolver.ts
Expand Up @@ -59,6 +59,7 @@ export class StaticSymbolResolver {
// Note: this will only contain StaticSymbols without members!
private importAs = new Map<StaticSymbol, StaticSymbol>();
private symbolResourcePaths = new Map<StaticSymbol, string>();
private symbolFromFile = new Map<string, StaticSymbol[]>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a field summaryFilePath: string in ResolvedStaticSymbol instead of the map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is much quicker to have the hash table return the array directly instead of scanning all the resolved symbols for the file. Since this is a perf optimization I believe the hash table is justified.


constructor(
private host: StaticSymbolResolverHost, private staticSymbolCache: StaticSymbolCache,
Expand Down Expand Up @@ -143,6 +144,25 @@ export class StaticSymbolResolver {
this.importAs.set(sourceSymbol, targetSymbol);
}

/**
* Invalidate all information derived from the given file.
*
* @param fileName the file to invalidate
*/
invalidateFile(fileName: string) {
this.metadataCache.delete(fileName);
this.resolvedFilePaths.delete(fileName);
const symbols = this.symbolFromFile.get(fileName);
if (symbols) {
this.symbolFromFile.delete(fileName);
for (const symbol of symbols) {
this.resolvedSymbols.delete(symbol);
this.importAs.delete(symbol);
this.symbolResourcePaths.delete(symbol);
}
}
}

private _resolveSymbolMembers(staticSymbol: StaticSymbol): ResolvedStaticSymbol {
const members = staticSymbol.members;
const baseResolvedSymbol =
Expand Down Expand Up @@ -281,6 +301,7 @@ export class StaticSymbolResolver {
}
resolvedSymbols.forEach(
(resolvedSymbol) => this.resolvedSymbols.set(resolvedSymbol.symbol, resolvedSymbol));
this.symbolFromFile.set(filePath, resolvedSymbols.map(resolvedSymbol => resolvedSymbol.symbol));
}

private createResolvedSymbol(
Expand Down
24 changes: 22 additions & 2 deletions packages/language-service/src/typescript_host.ts
Expand Up @@ -87,6 +87,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
private fileToComponent: Map<string, StaticSymbol>;
private templateReferences: string[];
private collectedErrors: Map<string, any[]>;
private fileVersions = new Map<string, string>();

constructor(private host: ts.LanguageServiceHost, private tsService: ts.LanguageService) {}

Expand Down Expand Up @@ -222,7 +223,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
if (this.modulesOutOfDate) {
this.analyzedModules = null;
this._reflector = null;
this._staticSymbolResolver = null;
this.templateReferences = null;
this.fileToComponent = null;
this.ensureAnalyzedModules();
Expand All @@ -242,8 +242,28 @@ export class TypeScriptServiceHost implements LanguageServiceHost {

private validate() {
const program = this.program;
if (this.lastProgram != program) {
if (this._staticSymbolResolver && this.lastProgram != program) {
// Invalidate file that have changed in the static symbol resolver
const invalidateFile = (fileName: string) =>
this._staticSymbolResolver.invalidateFile(fileName);
this.clearCaches();
const seen = new Set<string>();
for (let sourceFile of this.program.getSourceFiles()) {
const fileName = sourceFile.fileName;
seen.add(fileName);
const version = this.host.getScriptVersion(fileName);
const lastVersion = this.fileVersions.get(fileName);
if (version != lastVersion) {
this.fileVersions.set(fileName, version);
invalidateFile(fileName);
}
}

// Remove file versions that are no longer in the file and invalidate them.
const missing = Array.from(this.fileVersions.keys()).filter(f => !seen.has(f));
missing.forEach(f => this.fileVersions.delete(f));
missing.forEach(invalidateFile);

this.lastProgram = program;
}
}
Expand Down