Skip to content

Commit

Permalink
fix(language-service): cache module resolution (#32483)
Browse files Browse the repository at this point in the history
This is a patch PR for #32479

This PR fixes a critical performance issue where the language
service makes a MASSIVE number of filesystem calls when performing
module resolution.
This is because there is no caching. To make matters worse, module
resolution is performed for every program change (which means every
few keystrokes trigger a massive number of fs calls).

PR Close #32483
  • Loading branch information
kyliau authored and matsko committed Sep 12, 2019
1 parent 38fe473 commit 1c5b157
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 14 deletions.
41 changes: 29 additions & 12 deletions packages/language-service/src/reflector_host.ts
Expand Up @@ -7,7 +7,7 @@
*/

import {StaticSymbolResolverHost} from '@angular/compiler';
import {CompilerOptions, MetadataCollector, MetadataReaderHost, createMetadataReaderCache, readMetadata} from '@angular/compiler-cli/src/language_services';
import {MetadataCollector, MetadataReaderHost, createMetadataReaderCache, readMetadata} from '@angular/compiler-cli/src/language_services';
import * as path from 'path';
import * as ts from 'typescript';

Expand Down Expand Up @@ -48,13 +48,23 @@ class ReflectorModuleModuleResolutionHost implements ts.ModuleResolutionHost, Me
}

export class ReflectorHost implements StaticSymbolResolverHost {
private hostAdapter: ReflectorModuleModuleResolutionHost;
private metadataReaderCache = createMetadataReaderCache();
private readonly hostAdapter: ReflectorModuleModuleResolutionHost;
private readonly metadataReaderCache = createMetadataReaderCache();
private readonly moduleResolutionCache: ts.ModuleResolutionCache;
private readonly fakeContainingPath: string;

constructor(
getProgram: () => ts.Program, serviceHost: ts.LanguageServiceHost,
private options: CompilerOptions) {
this.hostAdapter = new ReflectorModuleModuleResolutionHost(serviceHost, getProgram);
getProgram: () => ts.Program, private readonly tsLSHost: ts.LanguageServiceHost, _: {}) {
// tsLSHost.getCurrentDirectory() returns the directory where tsconfig.json
// is located. This is not the same as process.cwd() because the language
// service host sets the "project root path" as its current directory.
const currentDir = tsLSHost.getCurrentDirectory();
this.fakeContainingPath = currentDir ? path.join(currentDir, 'fakeContainingFile.ts') : '';
this.hostAdapter = new ReflectorModuleModuleResolutionHost(tsLSHost, getProgram);
this.moduleResolutionCache = ts.createModuleResolutionCache(
currentDir,
s => s, // getCanonicalFileName
tsLSHost.getCompilationSettings());
}

getMetadataFor(modulePath: string): {[key: string]: any}[]|undefined {
Expand All @@ -63,15 +73,22 @@ export class ReflectorHost implements StaticSymbolResolverHost {

moduleNameToFileName(moduleName: string, containingFile?: string): string|null {
if (!containingFile) {
if (moduleName.indexOf('.') === 0) {
if (moduleName.startsWith('.')) {
throw new Error('Resolution of relative paths requires a containing file.');
}
// Any containing file gives the same result for absolute imports
containingFile = path.join(this.options.basePath !, 'index.ts').replace(/\\/g, '/');
if (!this.fakeContainingPath) {
// If current directory is empty then the file must belong to an inferred
// project (no tsconfig.json), in which case it's not possible to resolve
// the module without the caller explicitly providing a containing file.
throw new Error(`Could not resolve '${moduleName}' without a containing file.`);
}
containingFile = this.fakeContainingPath;
}
const resolved =
ts.resolveModuleName(moduleName, containingFile !, this.options, this.hostAdapter)
.resolvedModule;
const compilerOptions = this.tsLSHost.getCompilationSettings();
const resolved = ts.resolveModuleName(
moduleName, containingFile, compilerOptions, this.hostAdapter,
this.moduleResolutionCache)
.resolvedModule;
return resolved ? resolved.resolvedFileName : null;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/language-service/test/reflector_host_spec.ts
Expand Up @@ -20,7 +20,7 @@ describe('reflector_host_spec', () => {
const originalJoin = path.join;
const originalPosixJoin = path.posix.join;
let mockHost =
new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh, 'app/node_modules', {
new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh, 'node_modules', {
...path,
join: (...args: string[]) => originalJoin.apply(path, args),
posix:
Expand All @@ -37,4 +37,4 @@ describe('reflector_host_spec', () => {
const result = reflectorHost.moduleNameToFileName('@angular/core');
expect(result).not.toBeNull('could not find @angular/core using path.win32');
});
});
});

0 comments on commit 1c5b157

Please sign in to comment.