From 22bda2226bd42d081b7314c1b11144ad79ad1c44 Mon Sep 17 00:00:00 2001
From: JoostK <joost.koehoorn@gmail.com>
Date: Wed, 9 Jun 2021 21:25:43 +0200
Subject: [PATCH] fix(compiler-cli): prevent prior compilations from being
 retained in watch builds (#42537)

In watch builds, the compiler attempts to reuse as much information from
a prior compilation as possible. To accomplish this, it keeps a
reference to the most recently succeeded `TraitCompiler`, which contains
all analysis data for the program. However, `TraitCompiler` has an
internal reference to an `IncrementalBuild`, which is itself built on
top of its prior state. Consequently, all prior compilations continued
to be referenced, preventing garbage collection from cleaning up these
instances.

This commit changes the `AnalyzedIncrementalState` to no longer retain
a `TraitCompiler` instance, but only the analysis data it contains. This
breaks the retainer path to the prior incremental state, allowing it to
be garbage collected.

PR Close #42537
---
 .../src/ngtsc/incremental/src/incremental.ts         |  8 ++++++--
 .../compiler-cli/src/ngtsc/incremental/src/state.ts  |  9 ++++++---
 .../src/ngtsc/incremental/test/BUILD.bazel           |  1 +
 .../src/ngtsc/incremental/test/incremental_spec.ts   |  6 ++++--
 .../src/ngtsc/transform/src/compilation.ts           | 12 ++++++++++++
 5 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts b/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts
index e0ef06c463750..1f466dc968c01 100644
--- a/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts
+++ b/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts
@@ -259,7 +259,7 @@ export class IncrementalCompilation implements IncrementalBuild<ClassRecord, Fil
       versions: this.versions,
       depGraph: this.depGraph,
       semanticDepGraph: newGraph,
-      traitCompiler,
+      priorAnalysis: traitCompiler.getAnalyzedRecords(),
       typeCheckResults: null,
       emitted,
     };
@@ -303,7 +303,11 @@ export class IncrementalCompilation implements IncrementalBuild<ClassRecord, Fil
       return null;
     }
 
-    return this.step.priorState.traitCompiler.recordsFor(sf);
+    const priorAnalysis = this.step.priorState.priorAnalysis;
+    if (!priorAnalysis.has(sf)) {
+      return null;
+    }
+    return priorAnalysis.get(sf)!;
   }
 
   priorTypeCheckingResultsFor(sf: ts.SourceFile): FileTypeCheckingData|null {
diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts
index bda9a28527a09..5526affe5c055 100644
--- a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts
+++ b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts
@@ -6,8 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import * as ts from 'typescript';
+
 import {AbsoluteFsPath} from '../../file_system';
-import {TraitCompiler} from '../../transform';
+import {ClassRecord} from '../../transform';
 import {FileTypeCheckingData} from '../../typecheck/src/checker';
 import {SemanticDepGraph} from '../semantic_graph';
 
@@ -51,9 +53,10 @@ export interface AnalyzedIncrementalState {
   semanticDepGraph: SemanticDepGraph;
 
   /**
-   * `TraitCompiler` which contains records of all analyzed classes within the build.
+   * The analysis data from a prior compilation. This stores the trait information for all source
+   * files that was present in a prior compilation.
    */
-  traitCompiler: TraitCompiler;
+  priorAnalysis: Map<ts.SourceFile, ClassRecord[]>;
 
   /**
    * All generated template type-checking files produced as part of this compilation, or `null` if
diff --git a/packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel
index 333aff0605111..77fb8e15f5ed0 100644
--- a/packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel
+++ b/packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel
@@ -16,6 +16,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/incremental",
         "//packages/compiler-cli/src/ngtsc/perf",
         "//packages/compiler-cli/src/ngtsc/testing",
+        "//packages/compiler-cli/src/ngtsc/transform",
         "@npm//typescript",
     ],
 )
diff --git a/packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts b/packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts
index 5eae81e0a1fb1..1dd3edca02318 100644
--- a/packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts
+++ b/packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts
@@ -10,6 +10,7 @@ import {absoluteFrom, getSourceFileOrError} from '../../file_system';
 import {runInEachFileSystem} from '../../file_system/testing';
 import {NOOP_PERF_RECORDER} from '../../perf';
 import {makeProgram} from '../../testing';
+import {TraitCompiler} from '../../transform';
 import {IncrementalCompilation} from '../src/incremental';
 
 runInEachFileSystem(() => {
@@ -20,13 +21,14 @@ runInEachFileSystem(() => {
         {name: FOO_PATH, contents: `export const FOO = true;`},
       ]);
       const fooSf = getSourceFileOrError(program, FOO_PATH);
+      const traitCompiler = {getAnalyzedRecords: () => new Map()} as TraitCompiler;
 
       const versionMapFirst = new Map([[FOO_PATH, 'version.1']]);
       const firstCompilation = IncrementalCompilation.fresh(
           program,
           versionMapFirst,
       );
-      firstCompilation.recordSuccessfulAnalysis(null!);
+      firstCompilation.recordSuccessfulAnalysis(traitCompiler);
       firstCompilation.recordSuccessfulEmit(fooSf);
 
       const versionMapSecond = new Map([[FOO_PATH, 'version.2']]);
@@ -34,7 +36,7 @@ runInEachFileSystem(() => {
           program, versionMapSecond, program, firstCompilation.state, new Set(),
           NOOP_PERF_RECORDER);
 
-      secondCompilation.recordSuccessfulAnalysis(null!);
+      secondCompilation.recordSuccessfulAnalysis(traitCompiler);
       expect(secondCompilation.safeToSkipEmit(fooSf)).toBeFalse();
     });
   });
diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
index 6c8ba0da9ccfc..a189f5d4a93b5 100644
--- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
+++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
@@ -166,6 +166,18 @@ export class TraitCompiler implements ProgramTypeCheckAdapter {
     return records;
   }
 
+  getAnalyzedRecords(): Map<ts.SourceFile, ClassRecord[]> {
+    const result = new Map<ts.SourceFile, ClassRecord[]>();
+    for (const [sf, classes] of this.fileToClasses) {
+      const records: ClassRecord[] = [];
+      for (const clazz of classes) {
+        records.push(this.classes.get(clazz)!);
+      }
+      result.set(sf, records);
+    }
+    return result;
+  }
+
   /**
    * Import a `ClassRecord` from a previous compilation.
    *