Do not attempt to evict currently compiling files from cache.
TypeScript's compiler holds hard references to source files for the
entire duration of a compilation. These files cannot be garbage
collected, so evicting them from the current cache does not actually
free memory.
This caused particularly bad caching behaviour where if the sources for
one compilation unit were larger than the cache, tsc_wrapped would empty
out its entire cache trying to free up memory with no effect, rendering
the cache ineffectual.
Instead, this change pins source files that are part of the current
compilation unit (as tracked by the list of current digests).
This also undoes the generalisation of FileCache. Apparently its reuse
in the linter never materialized. Given the specific knowledge about
file digests, this also seems unlikely in the future.
While at it, this also fixes some warnings across the files, uses an ES6
Map for the digests, and improves performance of the strict deps test by
caching lib.d.ts ASTs.
PiperOrigin-RevId: 205792937
diff --git a/internal/tsc_wrapped/compiler_host.ts b/internal/tsc_wrapped/compiler_host.ts
index 068110b..9e95fd6 100644
--- a/internal/tsc_wrapped/compiler_host.ts
+++ b/internal/tsc_wrapped/compiler_host.ts
@@ -24,12 +24,15 @@
}
export function narrowTsOptions(options: ts.CompilerOptions): BazelTsOptions {
- if (!options.rootDirs)
+ if (!options.rootDirs) {
throw new Error(`compilerOptions.rootDirs should be set by tsconfig.bzl`);
- if (!options.rootDir)
+ }
+ if (!options.rootDir) {
throw new Error(`compilerOptions.rootDirs should be set by tsconfig.bzl`);
- if (!options.outDir)
+ }
+ if (!options.outDir) {
throw new Error(`compilerOptions.rootDirs should be set by tsconfig.bzl`);
+ }
return options as BazelTsOptions;
}
diff --git a/internal/tsc_wrapped/diagnostics_test.ts b/internal/tsc_wrapped/diagnostics_test.ts
index 6fa2eb8..fceece4 100644
--- a/internal/tsc_wrapped/diagnostics_test.ts
+++ b/internal/tsc_wrapped/diagnostics_test.ts
@@ -2,7 +2,6 @@
import * as ts from 'typescript';
import * as diagnostics from './diagnostics';
-import {TS_ERR_CANNOT_FIND_MODULE} from './strict_deps';
import {BazelOptions} from './tsconfig';
describe('diagnostics', () => {
diff --git a/internal/tsc_wrapped/file_cache.ts b/internal/tsc_wrapped/file_cache.ts
index 87613bc..b5371a9 100644
--- a/internal/tsc_wrapped/file_cache.ts
+++ b/internal/tsc_wrapped/file_cache.ts
@@ -16,19 +16,16 @@
*/
import * as fs from 'fs';
-import * as path from 'path';
import * as ts from 'typescript';
import * as perfTrace from './perf_trace';
-export interface CacheEntry<CachedType> {
+/**
+ * An entry in FileCache consists of blaze's digest of the file and the parsed
+ * ts.SourceFile AST.
+ */
+export interface CacheEntry {
digest: string;
- value: CachedType;
-}
-
-export interface LRUCache<CachedType> {
- getCache(key: string): CachedType|undefined;
- putCache(key: string, value: CacheEntry<CachedType>): void;
- inCache(key: string): boolean;
+ value: ts.SourceFile;
}
/**
@@ -45,14 +42,21 @@
* reaching the cache size limit, it deletes the oldest (first) entries. Used
* cache entries are moved to the end of the list by deleting and re-inserting.
*/
-export class FileCache<CachedType> implements LRUCache<CachedType> {
- private fileCache = new Map<string, CacheEntry<CachedType>>();
+// TODO(martinprobst): Drop the <T> parameter, it's no longer used.
+export class FileCache<T = {}> {
+ private fileCache = new Map<string, CacheEntry>();
/**
* FileCache does not know how to construct bazel's opaque digests. This
- * field caches the last compile run's digests, so that code below knows what
- * digest to assign to a newly loaded file.
+ * field caches the last (or current) compile run's digests, so that code
+ * below knows what digest to assign to a newly loaded file.
*/
- private lastDigests: {[filePath: string]: string} = {};
+ private lastDigests = new Map<string, string>();
+ /**
+ * FileCache can enter a degenerate state, where all cache entries are pinned
+ * by lastDigests, but the system is still out of memory. In that case, do not
+ * attempt to free memory until lastDigests has changed.
+ */
+ private cannotEvict = false;
cacheStats = {
hits: 0,
@@ -87,35 +91,44 @@
* updateCache must be called before loading files - only files that were
* updated (with a digest) previously can be loaded.
*/
- updateCache(digests: {[filePath: string]: string}) {
+ updateCache(digests: {[k: string]: string}): void;
+ updateCache(digests: Map<string, string>): void;
+ updateCache(digests: Map<string, string>|{[k: string]: string}) {
+ // TODO(martinprobst): drop the Object based version, it's just here for
+ // backwards compatibility.
+ if (!(digests instanceof Map)) {
+ digests = new Map(Object.keys(digests).map(
+ (k): [string, string] => [k, (digests as {[k: string]: string})[k]]));
+ }
this.debug('updating digests:', digests);
this.lastDigests = digests;
- for (const fp of Object.keys(digests)) {
- const entry = this.fileCache.get(fp);
- if (entry && entry.digest !== digests[fp]) {
+ this.cannotEvict = false;
+ for (const [filePath, newDigest] of digests.entries()) {
+ const entry = this.fileCache.get(filePath);
+ if (entry && entry.digest !== newDigest) {
this.debug(
- 'dropping file cache entry for', fp, 'digests', entry.digest,
- digests[fp]);
- this.fileCache.delete(fp);
+ 'dropping file cache entry for', filePath, 'digests', entry.digest,
+ newDigest);
+ this.fileCache.delete(filePath);
}
}
}
getLastDigest(filePath: string): string {
- const digest = this.lastDigests[filePath];
+ const digest = this.lastDigests.get(filePath);
if (!digest) {
throw new Error(
`missing input digest for ${filePath}.` +
- `(only have ${Object.keys(this.lastDigests)})`);
+ `(only have ${Array.from(this.lastDigests.keys())})`);
}
return digest;
}
- getCache(filePath: string): CachedType|undefined {
+ getCache(filePath: string): ts.SourceFile|undefined {
this.cacheStats.reads++;
const entry = this.fileCache.get(filePath);
- let value: CachedType|undefined;
+ let value: ts.SourceFile|undefined;
if (!entry) {
this.debug('Cache miss:', filePath);
} else {
@@ -131,8 +144,7 @@
return value;
}
- putCache(filePath: string, entry: CacheEntry<CachedType>):
- void {
+ putCache(filePath: string, entry: CacheEntry): void {
const dropped = this.maybeFreeMemory();
this.fileCache.set(filePath, entry);
this.debug('Loaded', filePath, 'dropped', dropped, 'cache entries');
@@ -143,7 +155,7 @@
* has a known cache digest. FileCache can only cache known files.
*/
isKnownInput(filePath: string): boolean {
- return !!this.lastDigests[filePath];
+ return this.lastDigests.has(filePath);
}
inCache(filePath: string): boolean {
@@ -197,25 +209,47 @@
/**
* Frees memory if required. Returns the number of dropped entries.
*/
- private maybeFreeMemory() {
- if (!this.shouldFreeMemory()) {
+ maybeFreeMemory() {
+ if (!this.shouldFreeMemory() || this.cannotEvict) {
return 0;
}
// Drop half the cache, the least recently used entry == the first entry.
- this.debug('Evicting from the cache');
- let numberKeysToDrop = this.fileCache.size / 2;
- let keysDropped = numberKeysToDrop;
+ this.debug('Evicting from the cache...');
+ const originalSize = this.fileCache.size;
+ let numberKeysToDrop = originalSize / 2;
+ if (numberKeysToDrop === 0) {
+ this.debug('Out of memory with an empty cache.');
+ return 0;
+ }
// Map keys are iterated in insertion order, since we reinsert on access
// this is indeed a LRU strategy.
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/keys
for (const key of this.fileCache.keys()) {
- if (numberKeysToDrop == 0) break;
+ if (numberKeysToDrop === 0) break;
+ // Do not attempt to drop files that are part of the current compilation
+ // unit. They are hard-retained by TypeScript compiler anyway, so they
+ // cannot be freed in either case.
+ if (this.lastDigests.has(key)) continue;
this.fileCache.delete(key);
numberKeysToDrop--;
}
+ const keysDropped = originalSize - this.fileCache.size;
this.cacheStats.evictions += keysDropped;
+ this.debug('Evicted', keysDropped, 'cache entries');
+ if (keysDropped === 0) {
+ // Freeing memory did not drop any cache entries, because all are pinned.
+ // Stop evicting until the pinned list changes again. This prevents
+ // degenerating into an O(n^2) situation where each file load iterates
+ // through the list of all files, trying to evict cache keys in vain
+ // because all are pinned.
+ this.cannotEvict = true;
+ }
return keysDropped;
}
+
+ getCacheKeysForTest() {
+ return Array.from(this.fileCache.keys());
+ }
}
export interface FileLoader {
@@ -233,8 +267,7 @@
// TODO(alexeagle): remove unused param after usages updated:
// angular:packages/bazel/src/ngc-wrapped/index.ts
- constructor(
- private readonly cache: FileCache<ts.SourceFile>, unused?: boolean) {}
+ constructor(private readonly cache: FileCache, unused?: boolean) {}
fileExists(filePath: string) {
return this.cache.isKnownInput(filePath);
diff --git a/internal/tsc_wrapped/file_cache_test.ts b/internal/tsc_wrapped/file_cache_test.ts
index 9b1785f..082291b 100644
--- a/internal/tsc_wrapped/file_cache_test.ts
+++ b/internal/tsc_wrapped/file_cache_test.ts
@@ -16,6 +16,7 @@
*/
import 'jasmine';
+
import * as ts from 'typescript';
import {CachedFileLoader, FileCache} from './file_cache';
@@ -26,14 +27,33 @@
}
describe('FileCache', () => {
- it('can be constructed', () => {
- const fileCache = new FileCache(fauxDebug);
- expect(fileCache).toBeTruthy();
+ let fileCache: FileCache;
+ let fileLoader: CachedFileLoader;
+
+ beforeEach(() => {
+ fileCache = new FileCache(fauxDebug);
+ fileLoader = new CachedFileLoader(fileCache);
});
+ function load(name: string, fn: string) {
+ return fileLoader.loadFile(name, fn, ts.ScriptTarget.ES5);
+ }
+
+ function setCurrentFiles(...fileName: string[]) {
+ // Give all files the same digest, so that the file cache allows reading
+ // them, but does not evict them.
+ const digests =
+ new Map(fileName.map((fn): [string, string] => [fn, 'digest']));
+ fileCache.updateCache(digests);
+ }
+
+ function expectCacheKeys() {
+ // Strip the long /tmp paths created by writeTempFile.
+ return expect(
+ fileCache.getCacheKeysForTest().map(fn => fn.replace(/.*\./, '')));
+ }
+
it('caches files', () => {
- const fileCache = new FileCache<ts.SourceFile>(fauxDebug);
- const fileLoader = new CachedFileLoader(fileCache);
const fn = writeTempFile('file_cache_test', 'let x: number = 12;\n');
invalidateFileCache(fileCache, fn);
@@ -50,33 +70,87 @@
});
it('caches in LRU order', () => {
- const fileCache = new FileCache<ts.SourceFile>(fauxDebug);
let free = false;
fileCache.shouldFreeMemory = () => free;
- const fileLoader = new CachedFileLoader(fileCache);
-
- function load(name: string, fn: string) {
- return fileLoader.loadFile(name, fn, ts.ScriptTarget.ES5);
- }
-
- const fn1 = writeTempFile('file_cache_test1', 'let x: number = 12;\n');
- const fn2 = writeTempFile('file_cache_test2', 'let x: number = 13;\n');
- const fn3 = writeTempFile('file_cache_test3', 'let x: number = 14;\n');
- invalidateFileCache(fileCache, fn1, fn2, fn3);
+ const fn1 = writeTempFile('file_cache_test1', 'let x: number = 1;\n');
+ const fn2 = writeTempFile('file_cache_test2', 'let x: number = 2;\n');
+ const fn3 = writeTempFile('file_cache_test3', 'let x: number = 3;\n');
+ const fn4 = writeTempFile('file_cache_test4', 'let x: number = 4;\n');
+ const fn5 = writeTempFile('file_cache_test5', 'let x: number = 5;\n');
+ setCurrentFiles(fn1, fn2, fn3, fn4, fn5);
// Populate the cache.
const f1 = load('f1', fn1);
const f2 = load('f2', fn2);
- // Load f1 from cache again. Now f1 is the most recently used file.
- expect(load('f1', fn1)).toBe(f1);
-
- free = true;
const f3 = load('f3', fn3);
- free = false;
- // f1 is still in cache, it was the MRU file.
- expect(load('f1', fn1)).toBe(f1);
- // f2 however was evicted.
- expect(load('f1', fn2)).not.toBe(f2);
+ const f4 = load('f4', fn4);
+
+ expectCacheKeys().toEqual([
+ 'file_cache_test1',
+ 'file_cache_test2',
+ 'file_cache_test3',
+ 'file_cache_test4',
+ ]);
+
+ // Load f1 from cache again. Now f1 is the most recently used file.
+ expect(load('f1', fn1)).toBe(f1, 'f1 should be cached');
+ expectCacheKeys().toEqual([
+ 'file_cache_test2',
+ 'file_cache_test3',
+ 'file_cache_test4',
+ 'file_cache_test1',
+ ]);
+
+ // Now load f5 and pretend memory must be freed.
+ // length / 2 == 2 files must be cleared.
+ // f2 + f5 are pinned because they are part of the compilation unit.
+ // f1, f3, f4 are eligible for eviction, and f1 is more recently used than
+ // the others, so f1 shoud be retained, f3 + f4 dropped.
+
+ setCurrentFiles(fn2, fn5);
+ free = true;
+ const f5 = load('f5', fn5);
+ expectCacheKeys().toEqual([
+ 'file_cache_test2',
+ 'file_cache_test1',
+ 'file_cache_test5',
+ ]);
+ setCurrentFiles(fn1, fn2, fn3, fn4, fn5);
+ expect(load('f1', fn1))
+ .toBe(f1, 'f1 should not be dropped, it was recently used');
+ expect(load('f2', fn2)).toBe(f2, 'f2 should be pinned');
+ expect(load('f3', fn3)).not.toBe(f3, 'f3 should have been dropped');
+ expect(load('f4', fn4)).not.toBe(f4, 'f4 should have been dropped');
+ expect(load('f5', fn5)).toBe(f5, 'f5 should be pinned');
+ });
+
+ it('degenerates to cannotEvict mode', () => {
+ // Pretend to always be out of memory.
+ fileCache.shouldFreeMemory = () => true;
+
+ const fn1 = writeTempFile('file_cache_test1', 'let x: number = 1;\n');
+ const fn2 = writeTempFile('file_cache_test2', 'let x: number = 2;\n');
+ const fn3 = writeTempFile('file_cache_test3', 'let x: number = 3;\n');
+ const fn4 = writeTempFile('file_cache_test4', 'let x: number = 4;\n');
+ setCurrentFiles(fn1, fn2, fn3);
+
+ load('fn1', fn1),
+ load('fn2', fn2);
+ load('fn3', fn3);
+
+ expect(fileCache['cannotEvict']).toBe(true, 'all files are pinned');
+ expectCacheKeys().toEqual([
+ 'file_cache_test1',
+ 'file_cache_test2',
+ 'file_cache_test3',
+ ]);
+ setCurrentFiles(fn1, fn4);
+ expect(fileCache['cannotEvict']).toBe(false, 'pinned files reset');
+ load('fn4', fn4);
+ expectCacheKeys().toEqual([
+ 'file_cache_test1',
+ 'file_cache_test4',
+ ]);
});
});
diff --git a/internal/tsc_wrapped/perf_trace.ts b/internal/tsc_wrapped/perf_trace.ts
index 6d604c0..3e0399f 100644
--- a/internal/tsc_wrapped/perf_trace.ts
+++ b/internal/tsc_wrapped/perf_trace.ts
@@ -45,7 +45,7 @@
args?: any;
}
-let events: Event[] = [];
+const events: Event[] = [];
/** wrap wraps enter()/leave() calls around a block of code. */
export function wrap<T>(name: string, f: () => T): T {
diff --git a/internal/tsc_wrapped/strict_deps_test.ts b/internal/tsc_wrapped/strict_deps_test.ts
index 941424c..6937d0d 100644
--- a/internal/tsc_wrapped/strict_deps_test.ts
+++ b/internal/tsc_wrapped/strict_deps_test.ts
@@ -21,6 +21,10 @@
import {checkModuleDeps} from './strict_deps';
describe('strict deps', () => {
+ // Cache ASTs that are not part of the test to avoid parsing lib.d.ts over and
+ // over again.
+ const astCache = new Map<string, ts.SourceFile>();
+
function createProgram(files: ts.MapLike<string>) {
const options: ts.CompilerOptions = {
noResolve: true,
@@ -28,13 +32,15 @@
rootDirs: ['/src', '/src/blaze-bin'],
paths: {'*': ['*', 'blaze-bin/*']},
};
-
// Fake compiler host relying on `files` above.
const host = ts.createCompilerHost(options);
const originalGetSourceFile = host.getSourceFile.bind(host);
- host.getSourceFile = function(fileName: string) {
+ host.getSourceFile = (fileName: string) => {
if (!files[fileName]) {
- return originalGetSourceFile(fileName, ts.ScriptTarget.Latest);
+ if (astCache.has(fileName)) return astCache.get(fileName);
+ const file = originalGetSourceFile(fileName, ts.ScriptTarget.Latest);
+ astCache.set(fileName, file);
+ return file;
}
return ts.createSourceFile(
fileName, files[fileName], ts.ScriptTarget.Latest);
diff --git a/internal/tsc_wrapped/test_support.ts b/internal/tsc_wrapped/test_support.ts
index 3e341ac..2465cfd 100644
--- a/internal/tsc_wrapped/test_support.ts
+++ b/internal/tsc_wrapped/test_support.ts
@@ -16,6 +16,7 @@
*/
/** @fileoverview Helper functions for tests. */
+
import * as fs from 'fs';
import {FileCache} from './file_cache';
@@ -30,11 +31,11 @@
let digestNumber = 0;
-export function invalidateFileCache(fc: FileCache<{}>, ...fileNames: string[]) {
- const digests: {[filePath: string]: string} = {};
+export function invalidateFileCache(fc: FileCache, ...fileNames: string[]) {
+ const digests = new Map<string, string>();
for (const fp of fileNames) {
digestNumber++;
- digests[fp] = fp + digestNumber;
+ digests.set(fp, `${fp}${digestNumber}`);
}
fc.updateCache(digests);
}
diff --git a/internal/tsc_wrapped/tsc_wrapped.ts b/internal/tsc_wrapped/tsc_wrapped.ts
index 286e6f8..161a11a 100644
--- a/internal/tsc_wrapped/tsc_wrapped.ts
+++ b/internal/tsc_wrapped/tsc_wrapped.ts
@@ -1,4 +1,3 @@
-import * as fs from 'fs';
import * as path from 'path';
import * as ts from 'typescript';
@@ -30,7 +29,7 @@
}
// The one FileCache instance used in this process.
-const fileCache = new FileCache<ts.SourceFile>(debug);
+const fileCache = new FileCache(debug);
/**
* Runs a single build, returning false on failure. This is potentially called
@@ -39,6 +38,10 @@
*/
function runOneBuild(
args: string[], inputs?: {[path: string]: string}): boolean {
+ if (args.length !== 1) {
+ console.error('Expected one argument: path to tsconfig.json');
+ return false;
+ }
// Strip leading at-signs, used in build_defs.bzl to indicate a params file
const tsconfigFile = args[0].replace(/^@+/, '');
@@ -69,25 +72,21 @@
if (inputs) {
fileLoader = new CachedFileLoader(fileCache);
// Resolve the inputs to absolute paths to match TypeScript internals
- const resolvedInputs: {[path: string]: string} = {};
+ const resolvedInputs = new Map<string, string>();
for (const key of Object.keys(inputs)) {
- resolvedInputs[resolveNormalizedPath(key)] = inputs[key];
+ resolvedInputs.set(resolveNormalizedPath(key), inputs[key]);
}
fileCache.updateCache(resolvedInputs);
} else {
fileLoader = new UncachedFileLoader();
}
- if (args.length !== 1) {
- console.error('Expected one argument: path to tsconfig.json');
- return false;
- }
-
const compilerHostDelegate =
ts.createCompilerHost({target: ts.ScriptTarget.ES5});
const compilerHost = new CompilerHost(
- files, options, bazelOpts, compilerHostDelegate, fileLoader, allowActionInputReads);
+ files, options, bazelOpts, compilerHostDelegate, fileLoader,
+ allowActionInputReads);
let program = ts.createProgram(files, options, compilerHost);
@@ -101,8 +100,7 @@
if (!bazelOpts.disableStrictDeps) {
const ignoredFilesPrefixes = [bazelOpts.nodeModulesPrefix];
if (options.rootDir) {
- ignoredFilesPrefixes.push(
- path.resolve(options.rootDir, 'node_modules'));
+ ignoredFilesPrefixes.push(path.resolve(options.rootDir, 'node_modules'));
}
program = strictDepsPlugin.wrap(program, {
...bazelOpts,
diff --git a/internal/tsc_wrapped/tsconfig_test.ts b/internal/tsc_wrapped/tsconfig_test.ts
index e4d27bd..63d3e7b 100644
--- a/internal/tsc_wrapped/tsconfig_test.ts
+++ b/internal/tsc_wrapped/tsconfig_test.ts
@@ -15,7 +15,6 @@
* limitations under the License.
*/
-import * as path from 'path';
import * as ts from 'typescript';
import {parseTsconfig, resolveNormalizedPath} from './tsconfig';
diff --git a/internal/tsc_wrapped/worker.ts b/internal/tsc_wrapped/worker.ts
index da71d31..aacb7b8 100644
--- a/internal/tsc_wrapped/worker.ts
+++ b/internal/tsc_wrapped/worker.ts
@@ -1,6 +1,7 @@
import * as path from 'path';
/* tslint:disable:no-require-imports */
const protobufjs = require('protobufjs');
+// tslint:disable-next-line:variable-name: ByteBuffer is instantiatable.
const ByteBuffer = require('bytebuffer');
protobufjs.convertFieldsToCamelCase = true;
