From b627f7f02e714f68a2f6ab0724597ad6107db3ce Mon Sep 17 00:00:00 2001 From: JoostK Date: Sun, 13 Sep 2020 20:31:33 +0200 Subject: [PATCH] test(compiler-cli): improve test performance using shared source file cache (#38909) Some compiler tests take a long time to run, even using multiple executors. A profiling session revealed that most time is spent in parsing source files, especially the default libraries are expensive to parse. The default library files are constant across all tests, so this commit introduces a shared cache of parsed source files of the default libraries. This achieves a significant improvement for several targets on my machine: //packages/compiler-cli/test/compliance: from 23s to 5s. //packages/compiler-cli/test/ngtsc: from 115s to 11s. Note that the number of shards for the compliance tests has been halved, as the extra shards no longer provide any speedup. PR Close #38909 --- .../compiler-cli/test/compliance/BUILD.bazel | 2 +- packages/compiler-cli/test/helpers/index.ts | 1 + .../test/helpers/src/cached_source_files.ts | 44 +++++++++++++++++++ packages/compiler-cli/test/ngtsc/env.ts | 13 +++++- packages/compiler/test/BUILD.bazel | 1 + packages/compiler/test/aot/test_util.ts | 15 +++++-- 6 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 packages/compiler-cli/test/helpers/src/cached_source_files.ts diff --git a/packages/compiler-cli/test/compliance/BUILD.bazel b/packages/compiler-cli/test/compliance/BUILD.bazel index d238d225a8..b37fe372c1 100644 --- a/packages/compiler-cli/test/compliance/BUILD.bazel +++ b/packages/compiler-cli/test/compliance/BUILD.bazel @@ -22,7 +22,7 @@ jasmine_node_test( data = [ "//packages/compiler-cli/test/ngtsc/fake_core:npm_package", ], - shard_count = 4, + shard_count = 2, tags = [ "ivy-only", ], diff --git a/packages/compiler-cli/test/helpers/index.ts b/packages/compiler-cli/test/helpers/index.ts index 2c8ece941d..5c504e39cf 100644 --- a/packages/compiler-cli/test/helpers/index.ts +++ b/packages/compiler-cli/test/helpers/index.ts @@ -7,3 +7,4 @@ */ export {getAngularPackagesFromRunfiles, resolveNpmTreeArtifact} from './src/runfile_helpers'; export * from './src/mock_file_loading'; +export * from './src/cached_source_files'; diff --git a/packages/compiler-cli/test/helpers/src/cached_source_files.ts b/packages/compiler-cli/test/helpers/src/cached_source_files.ts new file mode 100644 index 0000000000..64dbd95529 --- /dev/null +++ b/packages/compiler-cli/test/helpers/src/cached_source_files.ts @@ -0,0 +1,44 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as ts from 'typescript'; +import {basename} from '../../../src/ngtsc/file_system'; + +// A cache of source files that are typically used across tests and are expensive to parse. +let sourceFileCache = new Map(); + +/** + * If the `fileName` is determined to benefit from caching across tests, a parsed `ts.SourceFile` + * is returned from a shared cache. If caching is not applicable for the requested `fileName`, then + * `null` is returned. + * + * Even if a `ts.SourceFile` already exists for the given `fileName` will the contents be loaded + * from disk, such that it can be verified whether the cached `ts.SourceFile` is identical to the + * disk contents. If there is a difference, a new `ts.SourceFile` is parsed from the loaded contents + * which replaces the prior cache entry. + * + * @param fileName the path of the file to request a source file for. + * @param load a callback to load the contents of the file; this is even called when a cache entry + * is available to verify that the cached `ts.SourceFile` corresponds with the contents on disk. + */ +export function getCachedSourceFile( + fileName: string, load: () => string | undefined): ts.SourceFile|null { + if (!/^lib\..+\.d\.ts$/.test(basename(fileName))) { + return null; + } + + const content = load(); + if (content === undefined) { + return null; + } + if (!sourceFileCache.has(fileName) || sourceFileCache.get(fileName)!.text !== content) { + const sf = ts.createSourceFile(fileName, content, ts.ScriptTarget.ES2015); + sourceFileCache.set(fileName, sf); + } + return sourceFileCache.get(fileName)!; +} diff --git a/packages/compiler-cli/test/ngtsc/env.ts b/packages/compiler-cli/test/ngtsc/env.ts index dcf3a3f25d..869a22e1fc 100644 --- a/packages/compiler-cli/test/ngtsc/env.ts +++ b/packages/compiler-cli/test/ngtsc/env.ts @@ -18,6 +18,7 @@ import {IndexedComponent} from '../../src/ngtsc/indexer'; import {NgtscProgram} from '../../src/ngtsc/program'; import {LazyRoute} from '../../src/ngtsc/routing'; import {setWrapHostForTest} from '../../src/transformers/compiler_host'; +import {getCachedSourceFile} from '../helpers'; /** @@ -266,7 +267,17 @@ export class NgtscTestEnvironment { } } -class AugmentedCompilerHost extends NgtscCompilerHost { +class NgtscTestCompilerHost extends NgtscCompilerHost { + getSourceFile(fileName: string, languageVersion: ts.ScriptTarget): ts.SourceFile|undefined { + const cachedSf = getCachedSourceFile(fileName, () => this.readFile(fileName)); + if (cachedSf !== null) { + return cachedSf; + } + return super.getSourceFile(fileName, languageVersion); + } +} + +class AugmentedCompilerHost extends NgtscTestCompilerHost { delegate!: ts.CompilerHost; } diff --git a/packages/compiler/test/BUILD.bazel b/packages/compiler/test/BUILD.bazel index ac43c2b336..53444e2738 100644 --- a/packages/compiler/test/BUILD.bazel +++ b/packages/compiler/test/BUILD.bazel @@ -29,6 +29,7 @@ ts_library( "//packages:types", "//packages/compiler", "//packages/compiler-cli", + "//packages/compiler-cli/test/helpers", "@npm//typescript", ], ) diff --git a/packages/compiler/test/aot/test_util.ts b/packages/compiler/test/aot/test_util.ts index 1ff49efb4a..006678439a 100644 --- a/packages/compiler/test/aot/test_util.ts +++ b/packages/compiler/test/aot/test_util.ts @@ -10,6 +10,7 @@ import {AotCompilerHost, AotCompilerOptions, createAotCompiler, GeneratedFile, t import {MetadataBundlerHost} from '@angular/compiler-cli/src/metadata/bundler'; import {MetadataCollector} from '@angular/compiler-cli/src/metadata/collector'; import {ModuleMetadata} from '@angular/compiler-cli/src/metadata/index'; +import {getCachedSourceFile} from '@angular/compiler-cli/test/helpers'; import {newArray} from '@angular/compiler/src/util'; import * as fs from 'fs'; import * as path from 'path'; @@ -182,6 +183,10 @@ export class EmittingCompilerHost implements ts.CompilerHost { onError?: (message: string) => void): ts.SourceFile { const content = this.readFile(fileName); if (content) { + const cachedSf = getCachedSourceFile(fileName, () => content); + if (cachedSf !== null) { + return cachedSf; + } return ts.createSourceFile(fileName, content, languageVersion, /* setParentNodes */ true); } throw new Error(`File not found '${fileName}'.`); @@ -316,16 +321,20 @@ export class MockCompilerHost implements ts.CompilerHost { // ts.CompilerHost getSourceFile( fileName: string, languageVersion: ts.ScriptTarget, - onError?: (message: string) => void): ts.SourceFile { + onError?: (message: string) => void): ts.SourceFile|undefined { let result = this.sourceFiles.get(fileName); if (!result) { const content = this.getFileContent(fileName); + const cachedSf = getCachedSourceFile(fileName, () => content); + if (cachedSf !== null) { + return cachedSf; + } if (content) { result = ts.createSourceFile(fileName, content, languageVersion); this.sourceFiles.set(fileName, result); } } - return result!; + return result; } getDefaultLibFileName(options: ts.CompilerOptions): string { @@ -424,7 +433,7 @@ export class MockAotCompilerHost implements AotCompilerHost { } } else { const sf = this.tsHost.getSourceFile(modulePath, ts.ScriptTarget.Latest); - const metadata = this.metadataProvider.getMetadata(sf); + const metadata = sf && this.metadataProvider.getMetadata(sf); return metadata ? [metadata] : []; } return undefined;