From 9803fd08cd6150ff43f86b07426d67a31b9dd4ea Mon Sep 17 00:00:00 2001 From: JoostK Date: Mon, 14 Sep 2020 21:56:40 +0200 Subject: [PATCH] fixup! perf(ngcc): introduce cache for sharing data across entry-points --- .../ngcc/src/packages/transform_cache.ts | 97 +++++++++++++++---- .../test/packages/transform_cache_spec.ts | 72 +++++++++++++- 2 files changed, 150 insertions(+), 19 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/packages/transform_cache.ts b/packages/compiler-cli/ngcc/src/packages/transform_cache.ts index 1a80b64a01483f..662441781df90d 100644 --- a/packages/compiler-cli/ngcc/src/packages/transform_cache.ts +++ b/packages/compiler-cli/ngcc/src/packages/transform_cache.ts @@ -10,19 +10,30 @@ import {AbsoluteFsPath, FileSystem} from '../../../src/ngtsc/file_system'; /** * A cache that holds on to data that can be shared for processing all entry-points in a single - * invocation of ngcc. In particular, the default library files are cached as parsed `ts.SourceFile` - * as these files are used in each entry-point and some are expensive to parse, especially - * `lib.es5.d.ts` and `lib.dom.d.ts`. Additionally, a `ts.ModuleResolutionCache` is exposed for - * all module resolution operations to use, such that all entry-points can leverage a single module - * resolution cache. + * invocation of ngcc. In particular, the following aspects are shared across all entry-points + * through this cache: + * + * 1. Default library files such as `lib.dom.d.ts` and `lib.es5.d.ts`. These files don't change + * and some are very large, so parsing is expensive. Therefore, the parsed `ts.SourceFile`s for + * the default library files are cached. + * 2. The typings of @angular scoped packages. The typing files for @angular packages are typically + * used in the entry-points that ngcc processes, so benefit from a single source file cache. + * Especially `@angular/core/core.d.ts` is large and expensive to parse repeatedly. In contrast + * to default library files, we have to account for these files to be invalidated during a single + * invocation of ngcc, as ngcc will overwrite the .d.ts files during its processing. + * 3. A module resolution cache for TypeScript to use for module resolution. Module resolution is + * an expensive operation due to the large number of filesystem accesses. During a single + * invocation of ngcc it is assumed that the filesystem layout does not change, so a single + * module resolution cache is provided for use by all entry-points. * * The lifecycle of this cache corresponds with a single invocation of ngcc. Separate invocations, * e.g. the CLI's synchronous module resolution fallback will therefore all have their own cache. * This is because module resolution results cannot be assumed to be valid across invocations, as - * modifications of the file-system may have invalidated earlier results. + * modifications of the file-system may have invalidated earlier results. Additionally, it allows + * for the source file cache to be garbage collected once ngcc processing has completed. */ export class TransformCache { - private defaultLibCache = new Map(); + private sfCache = new Map(); readonly moduleResolutionCache: ts.ModuleResolutionCache; constructor(private fs: FileSystem) { @@ -35,21 +46,43 @@ export class TransformCache { const absPath = this.fs.resolve(fileName); if (isDefaultLibrary(absPath, this.fs)) { return this.getDefaultLibFileCached(absPath); + } else if (isAngularDts(absPath, this.fs)) { + return this.getAngularDtsCached(absPath); } else { return undefined; } } private getDefaultLibFileCached(absPath: AbsoluteFsPath): ts.SourceFile|undefined { - if (!this.defaultLibCache.has(absPath)) { + if (!this.sfCache.has(absPath)) { const content = readFile(absPath, this.fs); if (content === undefined) { return undefined; } const sf = ts.createSourceFile(absPath, content, ts.ScriptTarget.ES2015); - this.defaultLibCache.set(absPath, sf); + this.sfCache.set(absPath, sf); + } + return this.sfCache.get(absPath)!; + } + + /** + * The entry-point .d.ts files of @angular packages are also cached, as they are fairly large and + * commonly used in entry-points. Unlike the default library files, we must account for the + * possibility that the source file cache is out of date, as @angular packages are themselves + * processed by ngcc so their .d.ts files will be overwritten. Therefore, the file is always read + * from disk and compared with the cached source file's text; if the contents have changed the + * file is re-parsed and the cache entry is replaced. + */ + private getAngularDtsCached(absPath: AbsoluteFsPath): ts.SourceFile|undefined { + const content = readFile(absPath, this.fs); + if (content === undefined) { + return undefined; + } + if (!this.sfCache.has(absPath) || this.sfCache.get(absPath)!.text !== content) { + const sf = ts.createSourceFile(absPath, content, ts.ScriptTarget.ES2015); + this.sfCache.set(absPath, sf); } - return this.defaultLibCache.get(absPath)!; + return this.sfCache.get(absPath)!; } } @@ -61,15 +94,43 @@ export class TransformCache { * @param fs The filesystem to use for inspecting the path. */ export function isDefaultLibrary(absPath: AbsoluteFsPath, fs: FileSystem): boolean { - if (!/^lib\..+\.d\.ts$/.test(fs.basename(absPath))) { - return false; - } - let path = absPath; - for (const dirName of ['node_modules', 'typescript', 'lib'].reverse()) { - path = fs.dirname(path); - if (fs.basename(path) !== dirName) { - return false; + return isFile(absPath, ['node_modules', 'typescript', 'lib', /^lib\..+\.d\.ts$/], fs); +} + +/** + * Determines whether the provided path corresponds with a .d.ts file inside of an @angular + * scoped package. This logic only accounts for the .d.ts files in the root, which is sufficient + * to find the large, flattened entry-point files that benefit from caching. + * + * @param absPath The path for which to determine if it corresponds with an @angular .d.ts file. + * @param fs The filesystem to use for inspecting the path. + */ +export function isAngularDts(absPath: AbsoluteFsPath, fs: FileSystem): boolean { + return isFile(absPath, ['node_modules', '@angular', /./, /\.d\.ts$/], fs); +} + +/** + * Helper function to determine whether a file corresponds with a given pattern of segments. + * + * @param path The path for which to determine whether it represented to provided segments. + * @param segments Array of segments; the full path must have ending segments that match the + * patterns in this array. + * @param fs The filesystem to use for inspecting the path. + */ +function isFile( + path: AbsoluteFsPath, segments: ReadonlyArray, fs: FileSystem): boolean { + for (const pattern of segments.slice().reverse()) { + const segment = fs.basename(path); + if (typeof pattern === 'string') { + if (pattern !== segment) { + return false; + } + } else { + if (!pattern.test(segment)) { + return false; + } } + path = fs.dirname(path); } return true; } diff --git a/packages/compiler-cli/ngcc/test/packages/transform_cache_spec.ts b/packages/compiler-cli/ngcc/test/packages/transform_cache_spec.ts index 66ebf38a9344b9..9e679e8512b9c0 100644 --- a/packages/compiler-cli/ngcc/test/packages/transform_cache_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/transform_cache_spec.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {absoluteFrom, FileSystem, getFileSystem} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; -import {EntryPointCache, isDefaultLibrary, TransformCache} from '../../src/packages/transform_cache'; +import {EntryPointCache, isAngularDts, isDefaultLibrary, TransformCache} from '../../src/packages/transform_cache'; runInEachFileSystem(() => { describe('Transform caching', () => { @@ -28,6 +28,14 @@ runInEachFileSystem(() => { name: _('/node_modules/typescript/lib/lib.dom.d.ts'), contents: `export declare interface Window {}`, }, + { + name: _('/node_modules/@angular/core/core.d.ts'), + contents: `export declare interface Component {}`, + }, + { + name: _('/node_modules/@angular/common/common.d.ts'), + contents: `export declare interface NgIf {}`, + }, { name: _('/index.ts'), contents: `export const index = true;`, @@ -58,6 +66,49 @@ runInEachFileSystem(() => { expect(libDom_2).toBe(libDom); }); + it('should cache a parsed source file for @angular scoped packages', () => { + const cache = new TransformCache(fs); + + const core = cache.getCachedSourceFile('/node_modules/@angular/core/core.d.ts')!; + expect(core).not.toBeUndefined(); + expect(core.text).toContain('Component'); + + const common = cache.getCachedSourceFile('/node_modules/@angular/common/common.d.ts')!; + expect(common).not.toBeUndefined(); + expect(common.text).toContain('NgIf'); + + const core_2 = cache.getCachedSourceFile('/node_modules/@angular/core/core.d.ts')!; + expect(core_2).toBe(core); + + const common_2 = cache.getCachedSourceFile('/node_modules/@angular/common/common.d.ts')!; + expect(common_2).toBe(common); + }); + + it('should reparse @angular d.ts files when they change', () => { + const cache = new TransformCache(fs); + + const core = cache.getCachedSourceFile('/node_modules/@angular/core/core.d.ts')!; + expect(core).not.toBeUndefined(); + expect(core.text).toContain('Component'); + + const common = cache.getCachedSourceFile('/node_modules/@angular/common/common.d.ts')!; + expect(common).not.toBeUndefined(); + expect(common.text).toContain('NgIf'); + + fs.writeFile( + _('/node_modules/@angular/core/core.d.ts'), `export declare interface Directive {}`); + + const core_2 = cache.getCachedSourceFile('/node_modules/@angular/core/core.d.ts')!; + expect(core_2).not.toBe(core); + expect(core_2.text).toContain('Directive'); + + const core_3 = cache.getCachedSourceFile('/node_modules/@angular/core/core.d.ts')!; + expect(core_3).toBe(core_2); + + const common_2 = cache.getCachedSourceFile('/node_modules/@angular/common/common.d.ts')!; + expect(common_2).toBe(common); + }); + it('should not cache files that are not default library files inside of the typescript package', () => { const cache = new TransformCache(fs); @@ -90,6 +141,25 @@ runInEachFileSystem(() => { }); }); + describe('isAngularDts()', () => { + it('should accept .d.ts files inside of the @angular scope', () => { + expect(isAngularDts(_('/node_modules/@angular/core/core.d.ts'), fs)).toBe(true); + expect(isAngularDts(_('/node_modules/@angular/common/common.d.ts'), fs)).toBe(true); + }); + it('should reject non-.d.ts files inside @angular scoped packages', () => { + expect(isAngularDts(_('/node_modules/@angular/common/src/common.ts'), fs)).toBe(false); + }); + it('should reject .d.ts files nested deeply inside @angular scoped packages', () => { + expect(isAngularDts(_('/node_modules/@angular/common/src/common.d.ts'), fs)).toBe(false); + }); + it('should reject .d.ts files directly inside the @angular scope', () => { + expect(isAngularDts(_('/node_modules/@angular/common.d.ts'), fs)).toBe(false); + }); + it('should reject files that are not inside node_modules', () => { + expect(isAngularDts(_('/@angular/core/core.d.ts'), fs)).toBe(false); + }); + }); + describe('EntryPointCache', () => { let transformCache: TransformCache; beforeEach(() => {