From 300c2fec9ced559dce4f3c815bb6c61dd5b61871 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Thu, 28 May 2020 16:08:52 -0700 Subject: [PATCH] refactor(compiler-cli): make IncrementalBuild strategy configurable (#37339) Commit 24b2f1da2b introduced an `NgCompiler` which operates on a `ts.Program` independently of the `NgtscProgram`. The NgCompiler got its `IncrementalDriver` (for incremental reuse of Angular compilation results) by looking at a monkey-patched property on the `ts.Program`. This monkey-patching operation causes problems with the Angular indexer (specifically, it seems to cause the indexer to retain too much of prior programs, resulting in OOM issues). To work around this, `IncrementalDriver` reuse is now handled by a dedicated `IncrementalBuildStrategy`. One implementation of this interface is used by the `NgtscProgram` to perform the old-style reuse, relying on the previous instance of `NgtscProgram` instead of monkey-patching. Only for `NgTscPlugin` is the monkey-patching strategy used, as the plugin sits behind an interface which only provides access to the `ts.Program`, not a prior instance of the plugin. PR Close #37339 --- packages/compiler-cli/BUILD.bazel | 1 + .../src/ngtsc/core/src/compiler.ts | 49 ++-------- .../src/ngtsc/core/test/BUILD.bazel | 1 + .../src/ngtsc/core/test/compiler_test.ts | 4 +- .../src/ngtsc/incremental/index.ts | 1 + .../src/ngtsc/incremental/src/strategy.ts | 95 +++++++++++++++++++ packages/compiler-cli/src/ngtsc/program.ts | 9 +- packages/compiler-cli/src/ngtsc/tsc_plugin.ts | 4 +- .../language-service/ivy/compiler/BUILD.bazel | 1 + .../language-service/ivy/compiler/compiler.ts | 10 +- 10 files changed, 126 insertions(+), 49 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/incremental/src/strategy.ts diff --git a/packages/compiler-cli/BUILD.bazel b/packages/compiler-cli/BUILD.bazel index 2ad66dfa6a..773af85c62 100644 --- a/packages/compiler-cli/BUILD.bazel +++ b/packages/compiler-cli/BUILD.bazel @@ -27,6 +27,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/core:api", "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", + "//packages/compiler-cli/src/ngtsc/incremental", "//packages/compiler-cli/src/ngtsc/indexer", "//packages/compiler-cli/src/ngtsc/perf", "//packages/compiler-cli/src/ngtsc/reflection", diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index a52b17c34d..bcb9b6a470 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -15,7 +15,7 @@ import {ErrorCode, ngErrorCode} from '../../diagnostics'; import {checkForPrivateExports, ReferenceGraph} from '../../entry_point'; import {getSourceFileOrError, LogicalFileSystem} from '../../file_system'; import {AbsoluteModuleStrategy, AliasingHost, AliasStrategy, DefaultImportTracker, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, PrivateExportAliasingHost, R3SymbolsImportRewriter, Reference, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesAliasingHost, UnifiedModulesStrategy} from '../../imports'; -import {IncrementalDriver} from '../../incremental'; +import {IncrementalBuildStrategy, IncrementalDriver} from '../../incremental'; import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer'; import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, MetadataReader} from '../../metadata'; import {ModuleWithProvidersScanner} from '../../modulewithproviders'; @@ -100,7 +100,8 @@ export class NgCompiler { private adapter: NgCompilerAdapter, private options: NgCompilerOptions, private tsProgram: ts.Program, private typeCheckingProgramStrategy: TypeCheckingProgramStrategy, - oldProgram: ts.Program|null = null, private perfRecorder: PerfRecorder = NOOP_PERF_RECORDER) { + private incrementalStrategy: IncrementalBuildStrategy, oldProgram: ts.Program|null = null, + private perfRecorder: PerfRecorder = NOOP_PERF_RECORDER) { this.constructionDiagnostics.push(...this.adapter.constructionDiagnostics); const incompatibleTypeCheckOptionsDiagnostic = verifyCompatibleTypeCheckOptions(this.options); if (incompatibleTypeCheckOptionsDiagnostic !== null) { @@ -129,7 +130,7 @@ export class NgCompiler { if (oldProgram === null) { this.incrementalDriver = IncrementalDriver.fresh(tsProgram); } else { - const oldDriver = getIncrementalDriver(oldProgram); + const oldDriver = this.incrementalStrategy.getIncrementalDriver(oldProgram); if (oldDriver !== null) { this.incrementalDriver = IncrementalDriver.reconcile(oldProgram, oldDriver, tsProgram, modifiedResourceFiles); @@ -140,7 +141,7 @@ export class NgCompiler { this.incrementalDriver = IncrementalDriver.fresh(tsProgram); } } - setIncrementalDriver(tsProgram, this.incrementalDriver); + this.incrementalStrategy.setIncrementalDriver(this.incrementalDriver, tsProgram); this.ignoreForDiagnostics = new Set(tsProgram.getSourceFiles().filter(sf => this.adapter.isShim(sf))); @@ -506,7 +507,7 @@ export class NgCompiler { const program = this.typeCheckingProgramStrategy.getProgram(); this.perfRecorder.stop(typeCheckSpan); - setIncrementalDriver(program, this.incrementalDriver); + this.incrementalStrategy.setIncrementalDriver(this.incrementalDriver, program); this.nextProgram = program; return diagnostics; @@ -793,44 +794,6 @@ function getR3SymbolsFile(program: ts.Program): ts.SourceFile|null { return program.getSourceFiles().find(file => file.fileName.indexOf('r3_symbols.ts') >= 0) || null; } -/** - * Symbol under which the `IncrementalDriver` is stored on a `ts.Program`. - * - * The TS model of incremental compilation is based around reuse of a previous `ts.Program` in the - * construction of a new one. The `NgCompiler` follows this abstraction - passing in a previous - * `ts.Program` is sufficient to trigger incremental compilation. This previous `ts.Program` need - * not be from an Angular compilation (that is, it need not have been created from `NgCompiler`). - * - * If it is, though, Angular can benefit from reusing previous analysis work. This reuse is managed - * by the `IncrementalDriver`, which is inherited from the old program to the new program. To - * support this behind the API of passing an old `ts.Program`, the `IncrementalDriver` is stored on - * the `ts.Program` under this symbol. - */ -const SYM_INCREMENTAL_DRIVER = Symbol('NgIncrementalDriver'); - -/** - * Get an `IncrementalDriver` from the given `ts.Program` if one is present. - * - * See `SYM_INCREMENTAL_DRIVER` for more details. - */ -function getIncrementalDriver(program: ts.Program): IncrementalDriver|null { - const driver = (program as any)[SYM_INCREMENTAL_DRIVER]; - if (driver === undefined || !(driver instanceof IncrementalDriver)) { - return null; - } - return driver; -} - -/** - * Save the given `IncrementalDriver` onto the given `ts.Program`, for retrieval in a subsequent - * incremental compilation. - * - * See `SYM_INCREMENTAL_DRIVER` for more details. - */ -function setIncrementalDriver(program: ts.Program, driver: IncrementalDriver): void { - (program as any)[SYM_INCREMENTAL_DRIVER] = driver; -} - /** * Since "strictTemplates" is a true superset of type checking capabilities compared to * "strictTemplateTypeCheck", it is required that the latter is not explicitly disabled if the diff --git a/packages/compiler-cli/src/ngtsc/core/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/core/test/BUILD.bazel index aeb41e51d0..ec34344c91 100644 --- a/packages/compiler-cli/src/ngtsc/core/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/core/test/BUILD.bazel @@ -14,6 +14,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/core:api", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", + "//packages/compiler-cli/src/ngtsc/incremental", "//packages/compiler-cli/src/ngtsc/typecheck", "@npm//typescript", ], diff --git a/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts b/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts index 1679f0f01f..cd2234e4fa 100644 --- a/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts +++ b/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts @@ -10,6 +10,7 @@ import * as ts from 'typescript'; import {absoluteFrom as _, FileSystem, getFileSystem, getSourceFileOrError, NgtscCompilerHost, setFileSystem} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; +import {NoopIncrementalBuildStrategy} from '../../incremental'; import {ReusedProgramStrategy} from '../../typecheck/src/augmented_program'; import {NgCompilerOptions} from '../api'; import {NgCompiler} from '../src/compiler'; @@ -47,7 +48,8 @@ runInEachFileSystem(() => { const host = NgCompilerHost.wrap(baseHost, [COMPONENT], options, /* oldProgram */ null); const program = ts.createProgram({host, options, rootNames: host.inputFiles}); const compiler = new NgCompiler( - host, options, program, new ReusedProgramStrategy(program, host, options, [])); + host, options, program, new ReusedProgramStrategy(program, host, options, []), + new NoopIncrementalBuildStrategy()); const diags = compiler.getDiagnostics(getSourceFileOrError(program, COMPONENT)); expect(diags.length).toBe(1); diff --git a/packages/compiler-cli/src/ngtsc/incremental/index.ts b/packages/compiler-cli/src/ngtsc/incremental/index.ts index 3f476367dd..887ea6ede7 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/index.ts +++ b/packages/compiler-cli/src/ngtsc/incremental/index.ts @@ -8,3 +8,4 @@ export {NOOP_INCREMENTAL_BUILD} from './src/noop'; export {IncrementalDriver} from './src/state'; +export * from './src/strategy'; diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/strategy.ts b/packages/compiler-cli/src/ngtsc/incremental/src/strategy.ts new file mode 100644 index 0000000000..8e8b3379f6 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/incremental/src/strategy.ts @@ -0,0 +1,95 @@ +/** + * @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 {IncrementalDriver} from './state'; + +/** + * Strategy used to manage the association between a `ts.Program` and the `IncrementalDriver` which + * represents the reusable Angular part of its compilation. + */ +export interface IncrementalBuildStrategy { + /** + * Determine the Angular `IncrementalDriver` for the given `ts.Program`, if one is available. + */ + getIncrementalDriver(program: ts.Program): IncrementalDriver|null; + + /** + * Associate the given `IncrementalDriver` with the given `ts.Program` and make it available to + * future compilations. + */ + setIncrementalDriver(driver: IncrementalDriver, program: ts.Program): void; +} + +/** + * A noop implementation of `IncrementalBuildStrategy` which neither returns nor tracks any + * incremental data. + */ +export class NoopIncrementalBuildStrategy implements IncrementalBuildStrategy { + getIncrementalDriver(): null { + return null; + } + + setIncrementalDriver(): void {} +} + +/** + * Tracks an `IncrementalDriver` within the strategy itself. + */ +export class TrackedIncrementalBuildStrategy implements IncrementalBuildStrategy { + private previous: IncrementalDriver|null = null; + private next: IncrementalDriver|null = null; + + getIncrementalDriver(): IncrementalDriver|null { + return this.next !== null ? this.next : this.previous; + } + + setIncrementalDriver(driver: IncrementalDriver): void { + this.next = driver; + } + + toNextBuildStrategy(): TrackedIncrementalBuildStrategy { + const strategy = new TrackedIncrementalBuildStrategy(); + strategy.previous = this.next; + return strategy; + } +} + +/** + * Manages the `IncrementalDriver` associated with a `ts.Program` by monkey-patching it onto the + * program under `SYM_INCREMENTAL_DRIVER`. + */ +export class PatchedProgramIncrementalBuildStrategy implements IncrementalBuildStrategy { + getIncrementalDriver(program: ts.Program): IncrementalDriver|null { + const driver = (program as any)[SYM_INCREMENTAL_DRIVER]; + if (driver === undefined || !(driver instanceof IncrementalDriver)) { + return null; + } + return driver; + } + + setIncrementalDriver(driver: IncrementalDriver, program: ts.Program): void { + (program as any)[SYM_INCREMENTAL_DRIVER] = driver; + } +} + + +/** + * Symbol under which the `IncrementalDriver` is stored on a `ts.Program`. + * + * The TS model of incremental compilation is based around reuse of a previous `ts.Program` in the + * construction of a new one. The `NgCompiler` follows this abstraction - passing in a previous + * `ts.Program` is sufficient to trigger incremental compilation. This previous `ts.Program` need + * not be from an Angular compilation (that is, it need not have been created from `NgCompiler`). + * + * If it is, though, Angular can benefit from reusing previous analysis work. This reuse is managed + * by the `IncrementalDriver`, which is inherited from the old program to the new program. To + * support this behind the API of passing an old `ts.Program`, the `IncrementalDriver` is stored on + * the `ts.Program` under this symbol. + */ +const SYM_INCREMENTAL_DRIVER = Symbol('NgIncrementalDriver'); diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 1f6e009bba..d1e3d33885 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -14,6 +14,7 @@ import {verifySupportedTypeScriptVersion} from '../typescript_support'; import {NgCompiler, NgCompilerHost} from './core'; import {NgCompilerOptions} from './core/api'; +import {TrackedIncrementalBuildStrategy} from './incremental'; import {IndexedComponent} from './indexer'; import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf'; import {ReusedProgramStrategy} from './typecheck'; @@ -51,6 +52,7 @@ export class NgtscProgram implements api.Program { private host: NgCompilerHost; private perfRecorder: PerfRecorder = NOOP_PERF_RECORDER; private perfTracker: PerfTracker|null = null; + private incrementalStrategy: TrackedIncrementalBuildStrategy; constructor( rootNames: ReadonlyArray, private options: NgCompilerOptions, @@ -77,9 +79,14 @@ export class NgtscProgram implements api.Program { const reusedProgramStrategy = new ReusedProgramStrategy( this.tsProgram, this.host, this.options, this.host.shimExtensionPrefixes); + this.incrementalStrategy = oldProgram !== undefined ? + oldProgram.incrementalStrategy.toNextBuildStrategy() : + new TrackedIncrementalBuildStrategy(); + // Create the NgCompiler which will drive the rest of the compilation. this.compiler = new NgCompiler( - this.host, options, this.tsProgram, reusedProgramStrategy, reuseProgram, this.perfRecorder); + this.host, options, this.tsProgram, reusedProgramStrategy, this.incrementalStrategy, + reuseProgram, this.perfRecorder); } getTsProgram(): ts.Program { diff --git a/packages/compiler-cli/src/ngtsc/tsc_plugin.ts b/packages/compiler-cli/src/ngtsc/tsc_plugin.ts index 00efdd11ca..0afc0a168b 100644 --- a/packages/compiler-cli/src/ngtsc/tsc_plugin.ts +++ b/packages/compiler-cli/src/ngtsc/tsc_plugin.ts @@ -11,6 +11,7 @@ import * as ts from 'typescript'; import {NgCompiler, NgCompilerHost} from './core'; import {NgCompilerOptions, UnifiedModulesHost} from './core/api'; import {NodeJSFileSystem, setFileSystem} from './file_system'; +import {PatchedProgramIncrementalBuildStrategy} from './incremental'; import {NOOP_PERF_RECORDER} from './perf'; import {ReusedProgramStrategy} from './typecheck/src/augmented_program'; @@ -94,7 +95,8 @@ export class NgTscPlugin implements TscPlugin { const typeCheckStrategy = new ReusedProgramStrategy( program, this.host, this.options, this.host.shimExtensionPrefixes); this._compiler = new NgCompiler( - this.host, this.options, program, typeCheckStrategy, oldProgram, NOOP_PERF_RECORDER); + this.host, this.options, program, typeCheckStrategy, + new PatchedProgramIncrementalBuildStrategy(), oldProgram, NOOP_PERF_RECORDER); return { ignoreForDiagnostics: this._compiler.ignoreForDiagnostics, ignoreForEmit: this._compiler.ignoreForEmit, diff --git a/packages/language-service/ivy/compiler/BUILD.bazel b/packages/language-service/ivy/compiler/BUILD.bazel index 6cf1cdde6b..bd4910a42e 100644 --- a/packages/language-service/ivy/compiler/BUILD.bazel +++ b/packages/language-service/ivy/compiler/BUILD.bazel @@ -9,6 +9,7 @@ ts_library( "//packages/compiler-cli", "//packages/compiler-cli/src/ngtsc/core", "//packages/compiler-cli/src/ngtsc/file_system", + "//packages/compiler-cli/src/ngtsc/incremental", "//packages/compiler-cli/src/ngtsc/typecheck", "@npm//typescript", ], diff --git a/packages/language-service/ivy/compiler/compiler.ts b/packages/language-service/ivy/compiler/compiler.ts index dec9bccae4..0daa13355d 100644 --- a/packages/language-service/ivy/compiler/compiler.ts +++ b/packages/language-service/ivy/compiler/compiler.ts @@ -10,6 +10,7 @@ import {CompilerOptions} from '@angular/compiler-cli'; import {NgCompiler, NgCompilerHost} from '@angular/compiler-cli/src/ngtsc/core'; import {AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system'; +import {PatchedProgramIncrementalBuildStrategy} from '@angular/compiler-cli/src/ngtsc/incremental'; import {TypeCheckingProgramStrategy, UpdateMode} from '@angular/compiler-cli/src/ngtsc/typecheck'; import * as ts from 'typescript/lib/tsserverlibrary'; @@ -31,7 +32,9 @@ export class Compiler { ); this.strategy = createTypeCheckingProgramStrategy(project); this.lastKnownProgram = this.strategy.getProgram(); - this.compiler = new NgCompiler(ngCompilerHost, options, this.lastKnownProgram, this.strategy); + this.compiler = new NgCompiler( + ngCompilerHost, options, this.lastKnownProgram, this.strategy, + new PatchedProgramIncrementalBuildStrategy()); } setCompilerOptions(options: CompilerOptions) { @@ -43,8 +46,9 @@ export class Compiler { const ngCompilerHost = NgCompilerHost.wrap(this.tsCompilerHost, inputFiles, this.options, this.lastKnownProgram); const program = this.strategy.getProgram(); - this.compiler = - new NgCompiler(ngCompilerHost, this.options, program, this.strategy, this.lastKnownProgram); + this.compiler = new NgCompiler( + ngCompilerHost, this.options, program, this.strategy, + new PatchedProgramIncrementalBuildStrategy(), this.lastKnownProgram); try { // This is the only way to force the compiler to update the typecheck file // in the program. We have to do try-catch because the compiler immediately