Skip to content
Permalink
Browse files

fix(language-service): Use module resolution cache (#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 #32479
  • Loading branch information...
kyliau authored and matsko committed Sep 4, 2019
1 parent ded5724 commit 6052b12fb3b4bd90a14d935a30ebafd2553d8641
@@ -48,11 +48,22 @@ 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, private readonly serviceHost: ts.LanguageServiceHost) {
this.hostAdapter = new ReflectorModuleModuleResolutionHost(serviceHost, getProgram);
constructor(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 {
@@ -64,23 +75,19 @@ export class ReflectorHost implements StaticSymbolResolverHost {
if (moduleName.startsWith('.')) {
throw new Error('Resolution of relative paths requires a containing file.');
}
// serviceHost.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 currentDirectory = this.serviceHost.getCurrentDirectory();
if (!currentDirectory) {
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.`);
}
// Any containing file gives the same result for absolute imports
containingFile = path.join(currentDirectory, 'index.ts');
containingFile = this.fakeContainingPath;
}
const compilerOptions = this.serviceHost.getCompilationSettings();
const resolved =
ts.resolveModuleName(moduleName, containingFile, compilerOptions, 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;
}

@@ -7,8 +7,11 @@
*/

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

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

import {toh} from './test_data';
import {MockTypescriptHost} from './test_utils';
@@ -19,7 +22,7 @@ describe('reflector_host_spec', () => {
it('should be able to find angular under windows', () => {
const originalJoin = path.join;
const originalPosixJoin = path.posix.join;
let mockHost =
const mockHost =
new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh, 'node_modules', {
...path,
join: (...args: string[]) => originalJoin.apply(path, args),
@@ -37,4 +40,46 @@ describe('reflector_host_spec', () => {
const result = reflectorHost.moduleNameToFileName('@angular/core');
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.
You can’t perform that action at this time.