Skip to content

Commit

Permalink
fix(language-service): Use module resolution cache (#32479)
Browse files Browse the repository at this point in the history
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 #32479
  • Loading branch information
kyliau authored and matsko committed Sep 10, 2019
1 parent ded5724 commit 6052b12
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 16 deletions.
37 changes: 22 additions & 15 deletions packages/language-service/src/reflector_host.ts
Expand Up @@ -48,11 +48,22 @@ class ReflectorModuleModuleResolutionHost implements ts.ModuleResolutionHost, Me
} }


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


constructor(getProgram: () => ts.Program, private readonly serviceHost: ts.LanguageServiceHost) { constructor(getProgram: () => ts.Program, private readonly tsLSHost: ts.LanguageServiceHost) {
this.hostAdapter = new ReflectorModuleModuleResolutionHost(serviceHost, getProgram); // 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 { getMetadataFor(modulePath: string): {[key: string]: any}[]|undefined {
Expand All @@ -64,23 +75,19 @@ export class ReflectorHost implements StaticSymbolResolverHost {
if (moduleName.startsWith('.')) { if (moduleName.startsWith('.')) {
throw new Error('Resolution of relative paths requires a containing file.'); throw new Error('Resolution of relative paths requires a containing file.');
} }
// serviceHost.getCurrentDirectory() returns the directory where tsconfig.json if (!this.fakeContainingPath) {
// 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 currentDirectory = this.serviceHost.getCurrentDirectory();
if (!currentDirectory) {
// If current directory is empty then the file must belong to an inferred // 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 // project (no tsconfig.json), in which case it's not possible to resolve
// the module without the caller explicitly providing a containing file. // the module without the caller explicitly providing a containing file.
throw new Error(`Could not resolve '${moduleName}' without a containing file.`); throw new Error(`Could not resolve '${moduleName}' without a containing file.`);
} }
// Any containing file gives the same result for absolute imports containingFile = this.fakeContainingPath;
containingFile = path.join(currentDirectory, 'index.ts');
} }
const compilerOptions = this.serviceHost.getCompilationSettings(); const compilerOptions = this.tsLSHost.getCompilationSettings();
const resolved = const resolved = ts.resolveModuleName(
ts.resolveModuleName(moduleName, containingFile, compilerOptions, this.hostAdapter) moduleName, containingFile, compilerOptions, this.hostAdapter,
.resolvedModule; this.moduleResolutionCache)
.resolvedModule;
return resolved ? resolved.resolvedFileName : null; return resolved ? resolved.resolvedFileName : null;
} }


Expand Down
47 changes: 46 additions & 1 deletion packages/language-service/test/reflector_host_spec.ts
Expand Up @@ -7,8 +7,11 @@
*/ */


import * as path from 'path'; import * as path from 'path';
import * as ts from 'typescript';


import {createLanguageService} from '../src/language_service';
import {ReflectorHost} from '../src/reflector_host'; import {ReflectorHost} from '../src/reflector_host';
import {TypeScriptServiceHost} from '../src/typescript_host';


import {toh} from './test_data'; import {toh} from './test_data';
import {MockTypescriptHost} from './test_utils'; import {MockTypescriptHost} from './test_utils';
Expand All @@ -19,7 +22,7 @@ describe('reflector_host_spec', () => {
it('should be able to find angular under windows', () => { it('should be able to find angular under windows', () => {
const originalJoin = path.join; const originalJoin = path.join;
const originalPosixJoin = path.posix.join; const originalPosixJoin = path.posix.join;
let mockHost = const mockHost =
new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh, 'node_modules', { new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh, 'node_modules', {
...path, ...path,
join: (...args: string[]) => originalJoin.apply(path, args), join: (...args: string[]) => originalJoin.apply(path, args),
Expand All @@ -37,4 +40,46 @@ describe('reflector_host_spec', () => {
const result = reflectorHost.moduleNameToFileName('@angular/core'); const result = reflectorHost.moduleNameToFileName('@angular/core');
expect(result).not.toBeNull('could not find @angular/core using path.win32'); expect(result).not.toBeNull('could not find @angular/core using path.win32');
}); });

it('should use module resolution cache', () => {
const mockHost = new MockTypescriptHost(['/app/main.ts'], toh);
// TypeScript relies on `ModuleResolutionHost.fileExists()` to perform
// module resolution, and ReflectorHost uses
// `LanguageServiceHost.getScriptSnapshot()` to implement `fileExists()`,
// so spy on this method to determine how many times it's called.
const spy = spyOn(mockHost, 'getScriptSnapshot').and.callThrough();

const tsLS = ts.createLanguageService(mockHost);

// First count is due to the instantiation of StaticReflector, which
// performs resolutions of core Angular symbols, like `NgModule`.
// TODO: Reduce this count to zero doing lazy instantiation.
const ngLSHost = new TypeScriptServiceHost(mockHost, tsLS);
const firstCount = spy.calls.count();
expect(firstCount).toBeGreaterThan(20);
expect(firstCount).toBeLessThan(50);
spy.calls.reset();

// Second count is due to resolution of the Tour of Heroes (toh) project.
// This resolves all Angular directives in the project.
ngLSHost.getAnalyzedModules();
const secondCount = spy.calls.count();
expect(secondCount).toBeGreaterThan(500);
expect(secondCount).toBeLessThan(600);
spy.calls.reset();

// Third count is due to recompution after the program changes.
mockHost.addCode(''); // this will mark project as dirty
ngLSHost.getAnalyzedModules();
const thirdCount = spy.calls.count();
expect(thirdCount).toBeGreaterThan(50);
expect(thirdCount).toBeLessThan(100);

// Summary
// | | First Count | Second Count | Third Count |
// |---------------|-------------|--------------|-------------|
// | Without Cache | 2581 | 6291 | 257 |
// | With Cache | 26 | 550 | 84 |
// | Improvement | ~100x | ~10x | ~3x |
});
}); });

0 comments on commit 6052b12

Please sign in to comment.