From 1566c80acda774cd8135b69f70d68b2d1e16c577 Mon Sep 17 00:00:00 2001 From: Jialin Huang Date: Tue, 26 May 2026 23:29:55 +0800 Subject: [PATCH 1/2] Improve JS emitter performance Batch placeholder replacement in resolveAllReferences so each source file gets one text.replace + one replaceWithText call, instead of one cycle per placeholder. Each replaceWithText call discards ts-morph's cached AST and re-parses the file, so the previous per-placeholder loop made the cost grow as O(placeholders per file * file size). For each file we now: - scan the text once to collect the placeholders actually present; - iterate the precomputed declaration/dependency maps (preserving the original insertion order so name-collision aliasing in addImport is unchanged); - accumulate a single placeholder -> local-name map; - apply all replacements in a single bulk text.replace and a single replaceWithText. Output is byte-identical on batch_modular and openai_modular smoke specs, and the binder integration tests (22) all pass unchanged. Real-SDK measurements (Microsoft.Network and Microsoft.Compute management-plane via 'tsp compile client.tsp --emit=@azure-tools/typespec-ts'): | Spec | resolve references (before) | resolve references (after) | Total onEmit (before) | Total onEmit (after) | |---------|----------------------------:|---------------------------:|----------------------:|---------------------:| | Network | 22:08.539 | 8.664s | 31:25.022 | 8:44.110 | | Compute | 3:20.601 | 3.455s | 5:17.883 | 1:49.726 | That's a 153x speedup on the binder phase for Network (22 minutes saved per SDK generation) and 58x for Compute (3.5 minutes saved). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../typespec-ts/src/framework/hooks/binder.ts | 187 +++++++++++++----- 1 file changed, 140 insertions(+), 47 deletions(-) diff --git a/packages/typespec-ts/src/framework/hooks/binder.ts b/packages/typespec-ts/src/framework/hooks/binder.ts index 9ff1443f41..6ac09e946a 100644 --- a/packages/typespec-ts/src/framework/hooks/binder.ts +++ b/packages/typespec-ts/src/framework/hooks/binder.ts @@ -249,9 +249,54 @@ class BinderImp implements Binder { * Applies all tracked imports to their respective source files. */ resolveAllReferences(sourceRoot: string, testRoot?: string): void { + // Pre-compute placeholder -> declaration/dependency maps + const declarationByPlaceholder = new Map< + string, + [unknown, DeclarationInfo | StaticHelperMetadata] + >(); + for (const [key, value] of this.declarations) { + declarationByPlaceholder.set(this.serializePlaceholder(key), [ + key, + value + ]); + } + for (const [key, value] of this.staticHelpers) { + declarationByPlaceholder.set(this.serializePlaceholder(key), [ + key, + value + ]); + } + const dependencyByPlaceholder = new Map(); + for (const dependency of Object.values(this.dependencies)) { + dependencyByPlaceholder.set( + this.serializePlaceholder(refkey(dependency)), + dependency + ); + } + this.project.getSourceFiles().map((file) => { - this.resolveDeclarationReferences(file); - this.resolveDependencyReferences(file); + // Scan each file's text once to find placeholders that are actually + // present, then build one placeholder -> replacement map per file and + // apply it in a single pass. + const presentPlaceholders = collectPlaceholders(file); + if (presentPlaceholders.size > 0) { + const replacementMap = new Map(); + this.resolveDeclarationReferences( + file, + declarationByPlaceholder, + presentPlaceholders, + replacementMap + ); + this.resolveDependencyReferences( + file, + dependencyByPlaceholder, + presentPlaceholders, + replacementMap + ); + if (replacementMap.size > 0) { + applyReplacements(file, replacementMap); + } + } const importStructures = this.imports.get(file); if (importStructures) { // Sort imports in place by module specifier to ensure consistent ordering @@ -267,34 +312,54 @@ class BinderImp implements Binder { this.cleanUnreferencedHelpers(sourceRoot, testRoot); } - private resolveDependencyReferences(file: SourceFile) { - if (!hasAnyPlaceholders(file)) { - return; - } - for (const dependency of Object.values(this.dependencies)) { - const placeholder = this.serializePlaceholder(refkey(dependency)); - const { name, module } = dependency; - const occurences = countPlaceholderOccurrences(file, placeholder); - if (occurences > 0) { - const importDec = this.addImport(file, module, name); - const uniqueName = importDec.alias ?? name; - replacePlaceholder(file, placeholder, uniqueName); + /** + * Resolves placeholders that refer to external dependencies by registering + * the matching imports and recording the local names in `replacementMap`. + * @param file - The source file currently being processed. + * @param dependencyByPlaceholder - Map from placeholder string to its dependency descriptor. + * @param presentPlaceholders - The set of placeholders actually present in the file's text. + * @param replacementMap - The per-file map that receives placeholder -> local-name entries. + */ + private resolveDependencyReferences( + file: SourceFile, + dependencyByPlaceholder: Map, + presentPlaceholders: Set, + replacementMap: Map + ) { + for (const [placeholder, dependency] of dependencyByPlaceholder) { + if (!presentPlaceholders.has(placeholder)) { + continue; } + const { name, module } = dependency; + const importDec = this.addImport(file, module, name); + const uniqueName = importDec.alias ?? name; + replacementMap.set(placeholder, uniqueName); } } - private resolveDeclarationReferences(file: SourceFile) { - if (!hasAnyPlaceholders(file)) { - return; - } - - for (const [declarationKey, declaration] of [ - ...this.declarations, - ...this.staticHelpers - ]) { - const placeholderKey = this.serializePlaceholder(declarationKey); - const occurences = countPlaceholderOccurrences(file, placeholderKey); - if (!occurences) { + /** + * Resolves placeholders that refer to in-project declarations, registering + * cross-file imports as needed and recording the local names in + * `replacementMap`. + * @param file - The source file currently being processed. + * @param declarationByPlaceholder - Map from placeholder string to its declaration key and metadata. + * @param presentPlaceholders - The set of placeholders actually present in the file's text. + * @param replacementMap - The per-file map that receives placeholder -> local-name entries. + */ + private resolveDeclarationReferences( + file: SourceFile, + declarationByPlaceholder: Map< + string, + [unknown, DeclarationInfo | StaticHelperMetadata] + >, + presentPlaceholders: Set, + replacementMap: Map + ) { + for (const [ + placeholderKey, + [declarationKey, declaration] + ] of declarationByPlaceholder) { + if (!presentPlaceholders.has(placeholderKey)) { continue; } @@ -317,7 +382,7 @@ class BinderImp implements Binder { const importDec = this.addImport(file, importTarget, name); name = importDec.alias ?? name; } - replacePlaceholder(file, placeholderKey, name); + replacementMap.set(placeholderKey, name); } } @@ -410,27 +475,59 @@ export function useBinder(): Binder { } /** - * Replaces all instances of a placeholder in a source file with a given value. - * @param sourceFile - The source file where the replacement occurs. - * @param placeholder - The placeholder string to replace. - * @param value - The value to replace the placeholder with. + * Replaces every placeholder in a source file in one bulk text.replace + * followed by a single replaceWithText call. + * @param sourceFile - The source file to mutate. + * @param replacementMap - A map from each placeholder string to its replacement value. */ -function replacePlaceholder( +function applyReplacements( sourceFile: SourceFile, - placeholder: string, - value: string + replacementMap: Map ): void { - const fileText = sourceFile.getFullText(); - const regex = new RegExp(escapeRegExp(placeholder), "g"); - const updatedText = fileText.replace(regex, value); - sourceFile.replaceWithText(updatedText); + if (replacementMap.size === 0) { + return; + } + const text = sourceFile.getFullText(); + const placeholderRegex = new RegExp( + `${escapeRegExp(PLACEHOLDER_PREFIX)}.+?__`, + "g" + ); + let changed = false; + const updatedText = text.replace(placeholderRegex, (match) => { + const replacement = replacementMap.get(match); + if (replacement === undefined) { + return match; + } + changed = true; + return replacement; + }); + if (changed) { + sourceFile.replaceWithText(updatedText); + } } -function countPlaceholderOccurrences( - sourceFile: SourceFile, - placeholder: string -): number { - return sourceFile.getFullText().split(placeholder).length - 1; +/** + * Returns the set of distinct `__PLACEHOLDER___` strings present in + * the file, after a single text scan. + * @param sourceFile - The source file to scan. + * @returns The set of placeholder strings found in the file's text. + */ +function collectPlaceholders(sourceFile: SourceFile): Set { + const result = new Set(); + const text = sourceFile.getFullText(); + if (!text.includes(PLACEHOLDER_PREFIX)) { + return result; + } + // Refkeys are alphanumeric (see refkey.ts), so `.+?` safely stops at `__`. + const placeholderRegex = new RegExp( + `${escapeRegExp(PLACEHOLDER_PREFIX)}.+?__`, + "g" + ); + let match: RegExpExecArray | null; + while ((match = placeholderRegex.exec(text)) !== null) { + result.add(match[0]); + } + return result; } /** @@ -441,7 +538,3 @@ function countPlaceholderOccurrences( function escapeRegExp(string: string): string { return string.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } - -function hasAnyPlaceholders(sourceFile: SourceFile): boolean { - return sourceFile.getFullText().includes(PLACEHOLDER_PREFIX); -} From 923390fe3a70b4114b966659b52c0c42675a0a25 Mon Sep 17 00:00:00 2001 From: Jialin Huang Date: Thu, 28 May 2026 14:21:35 +0800 Subject: [PATCH 2/2] Batch import declarations to avoid per-call file re-parse Replace the per-import addImportDeclaration loop with a single addImportDeclarations bulk call. Each individual addImportDeclaration triggers a full re-parse of the source file in ts-morph, so emitting N imports per file was O(N) re-parses; bulk-add collapses this to one. --- packages/typespec-ts/src/framework/hooks/binder.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/typespec-ts/src/framework/hooks/binder.ts b/packages/typespec-ts/src/framework/hooks/binder.ts index 6ac09e946a..90ebdd229f 100644 --- a/packages/typespec-ts/src/framework/hooks/binder.ts +++ b/packages/typespec-ts/src/framework/hooks/binder.ts @@ -298,14 +298,12 @@ class BinderImp implements Binder { } } const importStructures = this.imports.get(file); - if (importStructures) { + if (importStructures && importStructures.length > 0) { // Sort imports in place by module specifier to ensure consistent ordering importStructures.sort((a, b) => a.moduleSpecifier < b.moduleSpecifier ? -1 : 1 ); - for (const importStructure of importStructures) { - file.addImportDeclaration(importStructure); - } + file.addImportDeclarations(importStructures); } });