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

@@ -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",

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

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",
     ],

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

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

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

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<string>, 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 {

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,

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",
     ],

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 =
+    this.compiler = new NgCompiler(
-        new NgCompiler(ngCompilerHost, this.options, program, this.strategy, this.lastKnownProgram);
+        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