perf(compiler-cli): fix regressions in incremental program reuse (#37690)
Commit4213e8d5
introduced shim reference tagging into the compiler, and changed how the `TypeCheckProgramHost` worked under the hood during the creation of a template type-checking program. This work enabled a more incremental flow for template type-checking, but unintentionally introduced several regressions in performance, caused by poor incrementality during `ts.Program` creation. 1. The `TypeCheckProgramHost` was made to rely on the `ts.CompilerHost` to retrieve instances of `ts.SourceFile`s from the original program. If the host does not return the original instance of such files, but instead creates new instances, this has two negative effects: it incurs additional parsing time, and it interferes with TypeScript's ability to reuse information about such files. 2. During the incremental creation of a `ts.Program`, TypeScript compares the `referencedFiles` of `ts.SourceFile` instances from the old program with those in the new program. If these arrays differ, TypeScript cannot fully reuse the old program. The implementation of reference tagging introduced in4213e8d5
restores the original `referencedFiles` array after a `ts.Program` is created, which means that future incremental operations involving that program will always fail this comparison, effectively limiting the incrementality TypeScript can achieve. Problem 1 exacerbates problem 2: if a new `ts.SourceFile` is created by the host after shim generation has been disabled, it will have an untagged `referencedFiles` array even if the original file's `referencedFiles` was not restored, triggering problem 2 when creating the template type-checking program. To fix these issues, `referencedFiles` arrays are now restored on the old `ts.Program` prior to the creation of a new incremental program. This allows TypeScript to get the most out of reusing the old program's data. Additionally, the `TypeCheckProgramHost` now uses the original `ts.Program` to retrieve original instances of `ts.SourceFile`s where possible, preventing issues when a host would otherwise return fresh instances. Together, these fixes ensure that program reuse is as incremental as possible, and tests have been added to verify this for certain scenarios. An optimization was further added to prevent the creation of a type-checking `ts.Program` in the first place if no type-checking is necessary. PR Close #37690
This commit is contained in:

committed by
Andrew Kushnir

parent
2cbe53a9ba
commit
96b96fba0f
@ -12,6 +12,7 @@ ts_library(
|
||||
"//packages/compiler-cli/src/ngtsc/file_system/testing",
|
||||
"//packages/compiler-cli/src/ngtsc/indexer",
|
||||
"//packages/compiler-cli/src/ngtsc/routing",
|
||||
"//packages/compiler-cli/src/ngtsc/testing",
|
||||
"//packages/compiler-cli/src/ngtsc/util",
|
||||
"//packages/compiler-cli/test:test_utils",
|
||||
"//packages/compiler-cli/test/helpers",
|
||||
|
@ -128,6 +128,13 @@ export class NgtscTestEnvironment {
|
||||
return this.oldProgram.getTsProgram();
|
||||
}
|
||||
|
||||
getReuseTsProgram(): ts.Program {
|
||||
if (this.oldProgram === null) {
|
||||
throw new Error('No ts.Program has been created yet.');
|
||||
}
|
||||
return (this.oldProgram as NgtscProgram).getReuseTsProgram();
|
||||
}
|
||||
|
||||
/**
|
||||
* Older versions of the CLI do not provide the `CompilerHost.getModifiedResourceFiles()` method.
|
||||
* This results in the `changedResources` set being `null`.
|
||||
|
@ -9,8 +9,9 @@
|
||||
import * as ts from 'typescript';
|
||||
|
||||
import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics';
|
||||
import {absoluteFrom as _, getFileSystem} from '../../src/ngtsc/file_system';
|
||||
import {absoluteFrom as _, getFileSystem, getSourceFileOrError} from '../../src/ngtsc/file_system';
|
||||
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
|
||||
import {expectCompleteReuse} from '../../src/ngtsc/testing';
|
||||
import {loadStandardTestFiles} from '../helpers/src/mock_file_loading';
|
||||
|
||||
import {NgtscTestEnvironment} from './env';
|
||||
@ -1862,18 +1863,26 @@ export declare class AnimationEvent {
|
||||
expect(env.driveDiagnostics()).toEqual([]);
|
||||
});
|
||||
|
||||
it('should not leave references to shims after execution', () => {
|
||||
// This test verifies that proper cleanup is performed for the technique being used to
|
||||
// include shim files in the ts.Program, and that none are left in the referencedFiles of
|
||||
// any ts.SourceFile after compilation.
|
||||
it('should not leave referencedFiles in a tagged state', () => {
|
||||
env.enableMultipleCompilations();
|
||||
|
||||
env.driveMain();
|
||||
for (const sf of env.getTsProgram().getSourceFiles()) {
|
||||
for (const ref of sf.referencedFiles) {
|
||||
expect(ref.fileName).not.toContain('.ngtypecheck.ts');
|
||||
}
|
||||
}
|
||||
const sf = getSourceFileOrError(env.getTsProgram(), _('/test.ts'));
|
||||
expect(sf.referencedFiles.map(ref => ref.fileName)).toEqual([]);
|
||||
});
|
||||
|
||||
it('should allow for complete program reuse during incremental compilations', () => {
|
||||
env.enableMultipleCompilations();
|
||||
|
||||
env.write('other.ts', `export const VERSION = 1;`);
|
||||
|
||||
env.driveMain();
|
||||
const firstProgram = env.getReuseTsProgram();
|
||||
|
||||
env.write('other.ts', `export const VERSION = 2;`);
|
||||
env.driveMain();
|
||||
|
||||
expectCompleteReuse(firstProgram);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
Reference in New Issue
Block a user