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
This commit is contained in:
		
							parent
							
								
									3961b3c360
								
							
						
					
					
						commit
						22bda2226b
					
				| @ -259,7 +259,7 @@ export class IncrementalCompilation implements IncrementalBuild<ClassRecord, Fil | |||||||
|       versions: this.versions, |       versions: this.versions, | ||||||
|       depGraph: this.depGraph, |       depGraph: this.depGraph, | ||||||
|       semanticDepGraph: newGraph, |       semanticDepGraph: newGraph, | ||||||
|       traitCompiler, |       priorAnalysis: traitCompiler.getAnalyzedRecords(), | ||||||
|       typeCheckResults: null, |       typeCheckResults: null, | ||||||
|       emitted, |       emitted, | ||||||
|     }; |     }; | ||||||
| @ -303,7 +303,11 @@ export class IncrementalCompilation implements IncrementalBuild<ClassRecord, Fil | |||||||
|       return null; |       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 { |   priorTypeCheckingResultsFor(sf: ts.SourceFile): FileTypeCheckingData|null { | ||||||
|  | |||||||
| @ -6,8 +6,10 @@ | |||||||
|  * found in the LICENSE file at https://angular.io/license
 |  * found in the LICENSE file at https://angular.io/license
 | ||||||
|  */ |  */ | ||||||
| 
 | 
 | ||||||
|  | import * as ts from 'typescript'; | ||||||
|  | 
 | ||||||
| import {AbsoluteFsPath} from '../../file_system'; | import {AbsoluteFsPath} from '../../file_system'; | ||||||
| import {TraitCompiler} from '../../transform'; | import {ClassRecord} from '../../transform'; | ||||||
| import {FileTypeCheckingData} from '../../typecheck/src/checker'; | import {FileTypeCheckingData} from '../../typecheck/src/checker'; | ||||||
| import {SemanticDepGraph} from '../semantic_graph'; | import {SemanticDepGraph} from '../semantic_graph'; | ||||||
| 
 | 
 | ||||||
| @ -51,9 +53,10 @@ export interface AnalyzedIncrementalState { | |||||||
|   semanticDepGraph: SemanticDepGraph; |   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 |    * All generated template type-checking files produced as part of this compilation, or `null` if | ||||||
|  | |||||||
| @ -16,6 +16,7 @@ ts_library( | |||||||
|         "//packages/compiler-cli/src/ngtsc/incremental", |         "//packages/compiler-cli/src/ngtsc/incremental", | ||||||
|         "//packages/compiler-cli/src/ngtsc/perf", |         "//packages/compiler-cli/src/ngtsc/perf", | ||||||
|         "//packages/compiler-cli/src/ngtsc/testing", |         "//packages/compiler-cli/src/ngtsc/testing", | ||||||
|  |         "//packages/compiler-cli/src/ngtsc/transform", | ||||||
|         "@npm//typescript", |         "@npm//typescript", | ||||||
|     ], |     ], | ||||||
| ) | ) | ||||||
|  | |||||||
| @ -10,6 +10,7 @@ import {absoluteFrom, getSourceFileOrError} from '../../file_system'; | |||||||
| import {runInEachFileSystem} from '../../file_system/testing'; | import {runInEachFileSystem} from '../../file_system/testing'; | ||||||
| import {NOOP_PERF_RECORDER} from '../../perf'; | import {NOOP_PERF_RECORDER} from '../../perf'; | ||||||
| import {makeProgram} from '../../testing'; | import {makeProgram} from '../../testing'; | ||||||
|  | import {TraitCompiler} from '../../transform'; | ||||||
| import {IncrementalCompilation} from '../src/incremental'; | import {IncrementalCompilation} from '../src/incremental'; | ||||||
| 
 | 
 | ||||||
| runInEachFileSystem(() => { | runInEachFileSystem(() => { | ||||||
| @ -20,13 +21,14 @@ runInEachFileSystem(() => { | |||||||
|         {name: FOO_PATH, contents: `export const FOO = true;`}, |         {name: FOO_PATH, contents: `export const FOO = true;`}, | ||||||
|       ]); |       ]); | ||||||
|       const fooSf = getSourceFileOrError(program, FOO_PATH); |       const fooSf = getSourceFileOrError(program, FOO_PATH); | ||||||
|  |       const traitCompiler = {getAnalyzedRecords: () => new Map()} as TraitCompiler; | ||||||
| 
 | 
 | ||||||
|       const versionMapFirst = new Map([[FOO_PATH, 'version.1']]); |       const versionMapFirst = new Map([[FOO_PATH, 'version.1']]); | ||||||
|       const firstCompilation = IncrementalCompilation.fresh( |       const firstCompilation = IncrementalCompilation.fresh( | ||||||
|           program, |           program, | ||||||
|           versionMapFirst, |           versionMapFirst, | ||||||
|       ); |       ); | ||||||
|       firstCompilation.recordSuccessfulAnalysis(null!); |       firstCompilation.recordSuccessfulAnalysis(traitCompiler); | ||||||
|       firstCompilation.recordSuccessfulEmit(fooSf); |       firstCompilation.recordSuccessfulEmit(fooSf); | ||||||
| 
 | 
 | ||||||
|       const versionMapSecond = new Map([[FOO_PATH, 'version.2']]); |       const versionMapSecond = new Map([[FOO_PATH, 'version.2']]); | ||||||
| @ -34,7 +36,7 @@ runInEachFileSystem(() => { | |||||||
|           program, versionMapSecond, program, firstCompilation.state, new Set(), |           program, versionMapSecond, program, firstCompilation.state, new Set(), | ||||||
|           NOOP_PERF_RECORDER); |           NOOP_PERF_RECORDER); | ||||||
| 
 | 
 | ||||||
|       secondCompilation.recordSuccessfulAnalysis(null!); |       secondCompilation.recordSuccessfulAnalysis(traitCompiler); | ||||||
|       expect(secondCompilation.safeToSkipEmit(fooSf)).toBeFalse(); |       expect(secondCompilation.safeToSkipEmit(fooSf)).toBeFalse(); | ||||||
|     }); |     }); | ||||||
|   }); |   }); | ||||||
|  | |||||||
| @ -166,6 +166,18 @@ export class TraitCompiler implements ProgramTypeCheckAdapter { | |||||||
|     return records; |     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. |    * Import a `ClassRecord` from a previous compilation. | ||||||
|    * |    * | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user