Skip to content

Commit

Permalink
fix(language-service): cache module resolution
Browse files Browse the repository at this point in the history
This is a patch PR for angular#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).
  • Loading branch information
Keen Yee Liau committed Sep 10, 2019
1 parent 2b7116a commit cb7b0e2
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 cb7b0e2

Please sign in to comment.