Repository navigation
refactor(compiler-cli): parse template diagNodes lazily and skip unused devmode TCB binding - #71904
Conversation
angular-robot
Bot
added
area: performance
618fbbd to
e1e0870
Compare
atscott
changed the title
atscott
changed the title
e1e0870 to
49ea51f
Compare
atscott
changed the title
49ea51f to
703676a
Compare
| }; | ||
|
|
||
| let cachedDiagNodes: TmplAstNode[] | undefined; | ||
| return { |
There was a problem hiding this comment.
Would it make sense to have a class for this instead of the inline object literal?
There was a problem hiding this comment.
✨ This mirrors ngtsc's parseExtractedTemplate, where the return type is the ParsedComponentTemplate interface (export interface ParsedComponentTemplate extends ParsedTemplate) and the return value spreads {...parsedTemplate, diagNodes}. Since parseTemplate returns a plain ParsedTemplate object, the object literal lets us spread ...parseWithOptions(parseOptions) without manually copying each ParsedTemplate field in a class constructor.
| let cachedDiagNodes: TmplAstNode[] | undefined; | ||
| return { | ||
| ...parseWithOptions(parseOptions), | ||
| get diagNodes(): TmplAstNode[] { |
There was a problem hiding this comment.
I was under the impression that we needed these ahead of time so that the spans in the TCB are correct.
There was a problem hiding this comment.
✨ Like ngtsc's parseExtractedTemplate, we still do a second parse with the span-preserving options (preserveWhitespaces: true, preserveLineEndings: true, preserveSignificantWhitespace: true, leadingTriviaChars: []) for TCBs, diagnostics, and indexing — ngtsc just runs that second parse eagerly inside parseExtractedTemplate even though diagNodes is only ever read later during typeCheck() / index().
When we generate a TCB here, buildTcbTargets still reads parsed.diagNodes, which runs parseWithOptions(diagParseOptions) with the same content, contentSpan, and span-preserving options, so the TCB gets the exact same AST and spans. Making it a lazy getter just avoids paying for that second parse in paths that never read diagNodes (devmode, emitDeclarationOnly, and cross-file DependencyTemplates lookups that only inspect ngContentSelectors).
703676a to
d40330f
Compare
…ed devmode TCB binding parseComponentTemplate previously ran the template parser twice on every component template: once with parseOptions for runtime code generation and eagerly a second time with diagParseOptions for TCB generation, diagnostics, and indexing. In devmode, emitDeclarationOnly, and cross-file DependencyTemplates lookups (which only inspect ngContentSelectors), diagNodes is never read, so the second parse was wasted work. This change makes diagNodes a lazy memoized getter on ParsedComponentTemplate so the second parse only runs when diagNodes is actually accessed. It also skips ensureBoundWithMetadata in HybridCompiler.prepareChunk and prepareChunkSync for single-file devmode chunks and emitDeclarationOnly builds (populating preparedTcbData on demand in the indexer and language service when needed), skips re-running adaptTcbInput in prepareTcbTargets Pass 2 for targets that were already adapted with requiresInline in Pass 1, and reuses cached source text from compiler.fileCache in ngp.ts.
d40330f to
42b4191
Compare
|
This PR was merged into the repository. The changes were merged into the following branches:
|
… parser closures Follows up on angular#71904 with two targeted hardening fixes: 1. In `HybridCompiler.invalidateStaleFileAnalysis`, drop the `existing.boundFingerprint !== undefined` guard when comparing `existing.boundFingerprint !== nextFingerprint` so un-fingerprinted cache entries (for example from a direct `processFileSync` call) clear stale `parsedTemplates`, `preparedTcbData`, and `tcb` before stamping a newly computed fingerprint. Also clear `fileAnalysis.tcb` in `populateBoundData` whenever `preparedTcbData` is recomputed so subsequent `getTcbForFile` lookups do not return stale TCB code. 2. In `parseComponentTemplate`, isolate the lazy `diagNodes` parser closures into `createInlineDiagParser` and `createExternalDiagParser` so external/indirect templates do not capture the containing TypeScript file's `content` or `ComponentMetadata`, and clear the parser closure reference inside `defineLazyDiagNodes` once `diagNodes` is evaluated.
parseComponentTemplatepreviously ran the template parser twice on every component template: once withparseOptionsfor runtime code generation and eagerly a second time withdiagParseOptionsfor TCB generation, diagnostics, and indexing. In devmode,emitDeclarationOnly, and cross-fileDependencyTemplateslookups (which only inspectngContentSelectors),diagNodesis never read, so the second parse was wasted work.This change makes
diagNodesa lazy memoized getter onParsedComponentTemplateso the second parse only runs whendiagNodesis actually accessed. It also skipsensureBoundWithMetadatainHybridCompiler.prepareChunkandprepareChunkSyncduring devmode andemitDeclarationOnlybuilds (binding only runtimeparsed.nodeswhen a multi-file devmode chunk haslocalCompilationExtraImports, and populatingpreparedTcbDataon demand in the indexer and language service when needed), skips re-runningadaptTcbInputinprepareTcbTargetsPass 2 for targets that were already adapted withrequiresInlinein Pass 1, and reuses cached source text fromcompiler.fileCacheinngp.ts.