Skip to content

Commit

Permalink
Do not attempt to evict currently compiling files from cache.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@mprobst @alexeagle
mprobst authored and alexeagle committed Jul 24, 2018
1 parent 1c40aeb commit f9f71cb
Show file tree
Hide file tree
Showing 10 changed files with 197 additions and 83 deletions.
9 changes: 6 additions & 3 deletions packages/concatjs/third_party/google3/rules_typescript/internal/tsc_wrapped/compiler_host.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ export interface BazelTsOptions extends ts.CompilerOptions {
}

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;
}

Expand Down
1 change: 0 additions & 1 deletion ...es/concatjs/third_party/google3/rules_typescript/internal/tsc_wrapped/diagnostics_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import 'jasmine';
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', () => {
Expand Down
105 changes: 69 additions & 36 deletions packages/concatjs/third_party/google3/rules_typescript/internal/tsc_wrapped/file_cache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -45,14 +42,21 @@ const DEFAULT_MAX_MEM_USAGE = 1024 * (1 << 20 /* 1 MB */);
* 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 = 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 lastDigests: {[filePath: string]: string} = {};
private cannotEvict = false;

cacheStats = {
hits: 0,
Expand Down Expand Up @@ -87,35 +91,44 @@ export class FileCache<CachedType> implements LRUCache<CachedType> {
* 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 {
Expand All @@ -131,8 +144,7 @@ export class FileCache<CachedType> implements LRUCache<CachedType> {
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');
Expand All @@ -143,7 +155,7 @@ export class FileCache<CachedType> implements LRUCache<CachedType> {
* 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 {
Expand Down Expand Up @@ -197,25 +209,47 @@ export class FileCache<CachedType> implements LRUCache<CachedType> {
/**
* 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 {
Expand All @@ -233,8 +267,7 @@ export class CachedFileLoader implements FileLoader {

// 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);
Expand Down
120 changes: 97 additions & 23 deletions ...ges/concatjs/third_party/google3/rules_typescript/internal/tsc_wrapped/file_cache_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

import 'jasmine';

import * as ts from 'typescript';

import {CachedFileLoader, FileCache} from './file_cache';
Expand All @@ -26,14 +27,33 @@ function fauxDebug(...args: any[]) {
}

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);

Expand All @@ -50,33 +70,87 @@ describe('FileCache', () => {
});

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);
const f3 = load('f3', fn3);
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);
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 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 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',
]);
});
});
2 changes: 1 addition & 1 deletion packages/concatjs/third_party/google3/rules_typescript/internal/tsc_wrapped/perf_trace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ declare interface Event {
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 {
Expand Down
Loading

0 comments on commit f9f71cb

Please sign in to comment.