Skip to content

Make builder not depend on information from dts emit about really needed modules #57800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Make builder not depend on information from dts emit about really needed modules #57800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d3a02f
Fix incorrect printing of information about signature calculation bec…
sheetalkamat Mar 15, 2024
dcdc654
Use referencedMap in place of exportedModulesMap in preparation for n…
sheetalkamat Mar 15, 2024
7b29ccb
Test baseline updates showing extra work we do in absense of using in…
sheetalkamat Mar 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
47 changes: 8 additions & 39 deletions src/compiler/builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import {
isNumber,
isString,
map,
mapDefined,
maybeBind,
noop,
notImplemented,
Expand All @@ -73,6 +72,7 @@ import {
returnUndefined,
sameMap,
SemanticDiagnosticsBuilderProgram,
SignatureInfo,
skipTypeChecking,
SourceFile,
sourceFileMayBeEmitted,
Expand Down Expand Up @@ -242,8 +242,6 @@ export interface BuilderProgramState extends BuilderState, ReusableBuilderProgra
* Already seen emitted files
*/
seenEmittedFiles: Map<Path, BuilderFileEmit> | undefined;
/** Stores list of files that change signature during emit - test only */
filesChangingSignature?: Set<Path>;
}

/** @internal */
Expand Down Expand Up @@ -633,7 +631,6 @@ function getNextAffectedFile(
state.currentChangedFilePath = undefined;
// Commit the changes in file signature
state.oldSignatures?.clear();
state.oldExportedModulesMap?.clear();
state.affectedFiles = undefined;
}

Expand Down Expand Up @@ -845,7 +842,7 @@ function handleDtsMayChangeOfReferencingExportOfAffectedFile(
) {
// If there was change in signature (dts output) for the changed file,
// then only we need to handle pending file emit
if (!state.exportedModulesMap || !state.changedFilesSet.has(affectedFile.resolvedPath)) return;
if (!state.referencedMap || !state.changedFilesSet.has(affectedFile.resolvedPath)) return;
if (!isChangedSignature(state, affectedFile.resolvedPath)) return;

// Since isolated modules dont change js files, files affected by change in signature is itself
Expand All @@ -869,9 +866,8 @@ function handleDtsMayChangeOfReferencingExportOfAffectedFile(
}

const seenFileAndExportsOfFile = new Set<string>();
// Go through exported modules from cache first
// If exported modules has path, all files referencing file exported from are affected
state.exportedModulesMap.getKeys(affectedFile.resolvedPath)?.forEach(exportedFromPath => {
// Go through files that reference affected file and handle dts emit and semantic diagnostics for them and their references
state.referencedMap.getKeys(affectedFile.resolvedPath)?.forEach(exportedFromPath => {
if (handleDtsMayChangeOfGlobalScope(state, exportedFromPath, cancellationToken, host)) return true;
const references = state.referencedMap!.getKeys(exportedFromPath);
return references && forEachKey(references, filePath =>
Expand Down Expand Up @@ -901,23 +897,12 @@ function handleDtsMayChangeOfFileAndExportsOfFile(
if (handleDtsMayChangeOfGlobalScope(state, filePath, cancellationToken, host)) return true;
handleDtsMayChangeOf(state, filePath, cancellationToken, host);

// If exported modules has path, all files referencing file exported from are affected
state.exportedModulesMap!.getKeys(filePath)?.forEach(exportedFromPath =>
handleDtsMayChangeOfFileAndExportsOfFile(
state,
exportedFromPath,
seenFileAndExportsOfFile,
cancellationToken,
host,
)
);

// Remove diagnostics of files that import this file (without going to exports of referencing files)
// Remove the diagnostics of files that import this file and handle all its exports too
state.referencedMap!.getKeys(filePath)?.forEach(referencingFilePath =>
!seenFileAndExportsOfFile.has(referencingFilePath) && // Not already removed diagnostic file
handleDtsMayChangeOf( // Dont add to seen since this is not yet done with the export removal
handleDtsMayChangeOfFileAndExportsOfFile(
state,
referencingFilePath,
seenFileAndExportsOfFile,
cancellationToken,
host,
)
Expand Down Expand Up @@ -1012,7 +997,6 @@ export interface ProgramMultiFileEmitBuildInfo {
options: CompilerOptions | undefined;
fileIdsList: readonly (readonly ProgramBuildInfoFileId[])[] | undefined;
referencedMap: ProgramBuildInfoReferencedMap | undefined;
exportedModulesMap: ProgramBuildInfoReferencedMap | undefined;
semanticDiagnosticsPerFile: ProgramBuildInfoDiagnostic[] | undefined;
emitDiagnosticsPerFile: ProgramBuildInfoDiagnostic[] | undefined;
affectedFilesPendingEmit: ProgramBuilderInfoFilePendingEmit[] | undefined;
Expand Down Expand Up @@ -1139,17 +1123,6 @@ function getBuildInfo(state: BuilderProgramState): BuildInfo {
]);
}

let exportedModulesMap: ProgramBuildInfoReferencedMap | undefined;
if (state.exportedModulesMap) {
exportedModulesMap = mapDefined(arrayFrom(state.exportedModulesMap.keys()).sort(compareStringsCaseSensitive), key => {
const oldValue = state.oldExportedModulesMap?.get(key);
// Not in temporary cache, use existing value
if (oldValue === undefined) return [toFileId(key), toFileIdListId(state.exportedModulesMap!.getValues(key)!)];
if (oldValue) return [toFileId(key), toFileIdListId(oldValue)];
return undefined;
});
}

const semanticDiagnosticsPerFile = convertToProgramBuildInfoDiagnostics(state.semanticDiagnosticsPerFile);
let affectedFilesPendingEmit: ProgramBuilderInfoFilePendingEmit[] | undefined;
if (state.affectedFilesPendingEmit?.size) {
Expand Down Expand Up @@ -1185,7 +1158,6 @@ function getBuildInfo(state: BuilderProgramState): BuildInfo {
options: convertToProgramBuildInfoCompilerOptions(state.compilerOptions),
fileIdsList,
referencedMap,
exportedModulesMap,
semanticDiagnosticsPerFile,
emitDiagnosticsPerFile,
affectedFilesPendingEmit,
Expand Down Expand Up @@ -1602,8 +1574,7 @@ export function createBuilderProgram(kind: BuilderProgramKind, { newProgram, hos
// With d.ts diagnostics they are also part of the signature so emitSignature will be different from it since its just hash of d.ts
if (!data?.diagnostics?.length) emitSignature = signature;
if (signature !== file.version) { // Update it
if (host.storeFilesChangingSignatureDuringEmit) (state.filesChangingSignature ??= new Set()).add(file.resolvedPath);
if (state.exportedModulesMap) BuilderState.updateExportedModules(state, file, file.exportedModulesFromDeclarationEmit);
if (host.storeSignatureInfo) (state.signatureInfo ??= new Map()).set(file.resolvedPath, SignatureInfo.StoredSignatureAtEmit);
if (state.affectedFiles) {
// Keep old signature so we know what to undo if cancellation happens
const existing = state.oldSignatures?.get(file.resolvedPath);
Expand All @@ -1613,7 +1584,6 @@ export function createBuilderProgram(kind: BuilderProgramKind, { newProgram, hos
else {
// These are directly committed
info.signature = signature;
state.oldExportedModulesMap?.clear();
}
}
}
Expand Down Expand Up @@ -1862,7 +1832,6 @@ export function createBuilderProgramUsingProgramBuildInfo(buildInfo: BuildInfo,
fileInfos,
compilerOptions: program.options ? convertToOptionsWithAbsolutePaths(program.options, toAbsolutePath) : {},
referencedMap: toManyToManyPathMap(program.referencedMap),
exportedModulesMap: toManyToManyPathMap(program.exportedModulesMap),
semanticDiagnosticsPerFile: toPerFileDiagnostics(program.semanticDiagnosticsPerFile),
emitDiagnosticsPerFile: toPerFileDiagnostics(program.emitDiagnosticsPerFile),
hasReusableDiagnostic: true,
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/builderPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ export interface BuilderProgramHost {
*/
writeFile?: WriteFileCallback;
/**
* Store the list of files that update signature during the emit
* Store information about the signature
*
* @internal
*/
storeFilesChangingSignatureDuringEmit?: boolean;
storeSignatureInfo?: boolean;
}

/** @internal */
export type HostForComputeHash = Pick<BuilderProgramHost, "createHash">;
export type HostForComputeHash = Pick<BuilderProgramHost, "createHash" | "storeSignatureInfo">;

/**
* Builder to manage the program state changes
Expand Down
78 changes: 11 additions & 67 deletions src/compiler/builderState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
Debug,
EmitOutput,
emptyArray,
ExportedModulesFromDeclarationEmit,
GetCanonicalFileName,
getDirectoryPath,
getIsolatedModules,
Expand Down Expand Up @@ -52,6 +51,12 @@ export function getFileEmitOutput(
}
}
/** @internal */
export enum SignatureInfo {
ComputedDts,
StoredSignatureAtEmit,
UsedVersion,
}
/** @internal */
export interface BuilderState {
/**
* Information of the file eg. its version, signature etc
Expand All @@ -63,14 +68,6 @@ export interface BuilderState {
* Thus non undefined value indicates, module emit
*/
readonly referencedMap?: BuilderState.ReadonlyManyToManyPathMap | undefined;
/**
* Contains the map of exported modules ReferencedSet=exported module files from the file if module emit is enabled
* Otherwise undefined
*
* This is equivalent to referencedMap, but for the emitted .d.ts file.
*/
readonly exportedModulesMap?: BuilderState.ManyToManyPathMap | undefined;

/**
* true if file version is used as signature
* This helps in delaying the calculation of the d.ts hash as version for the file till reasonable time
Expand All @@ -86,10 +83,6 @@ export interface BuilderState {
* Stores signatures before before the update till affected file is committed
*/
oldSignatures?: Map<Path, string | false>;
/**
* Stores exportedModulesMap before the update till affected file is committed
*/
oldExportedModulesMap?: Map<Path, ReadonlySet<Path> | false>;
/**
* Cache of all files excluding default library file for the current program
*/
Expand All @@ -98,6 +91,8 @@ export interface BuilderState {
* Cache of all the file names
*/
allFileNames?: readonly string[];
/** Information about the signature computation - test only */
signatureInfo?: Map<Path, SignatureInfo>;
}
/** @internal */
export namespace BuilderState {
Expand Down Expand Up @@ -306,7 +301,6 @@ export namespace BuilderState {
const isOutFile = options.outFile;
const referencedMap = options.module !== ModuleKind.None && !isOutFile ?
createManyToManyPathMap() : undefined;
const exportedModulesMap = referencedMap ? createManyToManyPathMap() : undefined;
const useOldState = canReuseOldState(referencedMap, oldState);

// Ensure source files have parent pointers set
Expand All @@ -324,16 +318,6 @@ export namespace BuilderState {
if (newReferences) {
referencedMap.set(sourceFile.resolvedPath, newReferences);
}
// Copy old visible to outside files map
if (useOldState) {
const oldUncommittedExportedModules = oldState!.oldExportedModulesMap?.get(sourceFile.resolvedPath);
const exportedModules = oldUncommittedExportedModules === undefined ?
oldState!.exportedModulesMap!.getValues(sourceFile.resolvedPath) :
oldUncommittedExportedModules || undefined;
if (exportedModules) {
exportedModulesMap!.set(sourceFile.resolvedPath, exportedModules);
}
}
}
fileInfos.set(sourceFile.resolvedPath, {
version,
Expand All @@ -347,7 +331,6 @@ export namespace BuilderState {
return {
fileInfos,
referencedMap,
exportedModulesMap,
useFileVersionAsSignature: !disableUseFileVersionAsSignature && !useOldState,
};
}
Expand Down Expand Up @@ -378,7 +361,6 @@ export namespace BuilderState {
host,
);
state.oldSignatures?.clear();
state.oldExportedModulesMap?.clear();
return result;
}

Expand Down Expand Up @@ -453,60 +435,22 @@ export namespace BuilderState {
const prevSignature = info.signature;
let latestSignature: string | undefined;
if (!sourceFile.isDeclarationFile && !useFileVersionAsSignature) {
computeDtsSignature(programOfThisState, sourceFile, cancellationToken, host, (signature, sourceFiles) => {
computeDtsSignature(programOfThisState, sourceFile, cancellationToken, host, signature => {
latestSignature = signature;
if (latestSignature !== prevSignature) {
updateExportedModules(state, sourceFile, sourceFiles[0].exportedModulesFromDeclarationEmit);
}
if (host.storeSignatureInfo) (state.signatureInfo ??= new Map()).set(sourceFile.resolvedPath, SignatureInfo.ComputedDts);
});
}
// Default is to use file version as signature
if (latestSignature === undefined) {
latestSignature = sourceFile.version;
if (state.exportedModulesMap && latestSignature !== prevSignature) {
(state.oldExportedModulesMap ||= new Map()).set(sourceFile.resolvedPath, state.exportedModulesMap.getValues(sourceFile.resolvedPath) || false);
// All the references in this file are exported
const references = state.referencedMap ? state.referencedMap.getValues(sourceFile.resolvedPath) : undefined;
if (references) {
state.exportedModulesMap.set(sourceFile.resolvedPath, references);
}
else {
state.exportedModulesMap.deleteKey(sourceFile.resolvedPath);
}
}
if (host.storeSignatureInfo) (state.signatureInfo ??= new Map()).set(sourceFile.resolvedPath, SignatureInfo.UsedVersion);
}
(state.oldSignatures ||= new Map()).set(sourceFile.resolvedPath, prevSignature || false);
(state.hasCalledUpdateShapeSignature ||= new Set()).add(sourceFile.resolvedPath);
info.signature = latestSignature;
return latestSignature !== prevSignature;
}

/**
* Coverts the declaration emit result into exported modules map
*/
export function updateExportedModules(state: BuilderState, sourceFile: SourceFile, exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined) {
if (!state.exportedModulesMap) return;
(state.oldExportedModulesMap ||= new Map()).set(sourceFile.resolvedPath, state.exportedModulesMap.getValues(sourceFile.resolvedPath) || false);
const exportedModules = getExportedModules(exportedModulesFromDeclarationEmit);
if (exportedModules) {
state.exportedModulesMap.set(sourceFile.resolvedPath, exportedModules);
}
else {
state.exportedModulesMap.deleteKey(sourceFile.resolvedPath);
}
}

export function getExportedModules(exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined) {
let exportedModules: Set<Path> | undefined;
exportedModulesFromDeclarationEmit?.forEach(
symbol =>
getReferencedFilesFromImportedModuleSymbol(symbol).forEach(
path => (exportedModules ??= new Set()).add(path),
),
);
return exportedModules;
}

/**
* Get all the dependencies of the sourceFile
*/
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/sys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1458,7 +1458,7 @@ export interface System {

// For testing
/** @internal */ now?(): Date;
/** @internal */ storeFilesChangingSignatureDuringEmit?: boolean;
/** @internal */ storeSignatureInfo?: boolean;
}

export interface FileWatcher {
Expand Down
4 changes: 1 addition & 3 deletions src/compiler/transformers/declarations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,7 @@ export function transformDeclarations(context: TransformationContext) {
combinedStatements = setTextRange(factory.createNodeArray([...combinedStatements, createEmptyExports(factory)]), combinedStatements);
}
}
const updated = factory.updateSourceFile(node, combinedStatements, /*isDeclarationFile*/ true, references, getFileReferencesForUsedTypeReferences(), node.hasNoDefaultLib, getLibReferences());
updated.exportedModulesFromDeclarationEmit = exportedModulesFromDeclarationEmit;
return updated;
return factory.updateSourceFile(node, combinedStatements, /*isDeclarationFile*/ true, references, getFileReferencesForUsedTypeReferences(), node.hasNoDefaultLib, getLibReferences());

function getLibReferences() {
return arrayFrom(libs.keys(), lib => ({ fileName: lib, pos: -1, end: -1 }));
Expand Down
6 changes: 1 addition & 5 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4326,7 +4326,6 @@ export interface SourceFile extends Declaration, LocalsContainer {
/** @internal */ localJsxFactory?: EntityName;
/** @internal */ localJsxFragmentFactory?: EntityName;

/** @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
/** @internal */ endFlowNode?: FlowNode;

/** @internal */ jsDocParsingMode?: JSDocParsingMode;
Expand Down Expand Up @@ -4369,9 +4368,6 @@ export const enum CommentDirectiveType {
Ignore,
}

/** @internal */
export type ExportedModulesFromDeclarationEmit = readonly Symbol[];

export interface Bundle extends Node {
readonly kind: SyntaxKind.Bundle;
readonly sourceFiles: readonly SourceFile[];
Expand Down Expand Up @@ -7799,7 +7795,7 @@ export interface CompilerHost extends ModuleResolutionHost {
/** @internal */ getSymlinkCache?(): SymlinkCache;

// For testing:
/** @internal */ storeFilesChangingSignatureDuringEmit?: boolean;
/** @internal */ storeSignatureInfo?: boolean;
/** @internal */ getBuildInfo?(fileName: string, configFilePath: string | undefined): BuildInfo | undefined;

jsDocParsingMode?: JSDocParsingMode;
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/watch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ export function createCompilerHostFromProgramHost(host: ProgramHost<any>, getCom
getEnvironmentVariable: maybeBind(host, host.getEnvironmentVariable) || (() => ""),
createHash: maybeBind(host, host.createHash),
readDirectory: maybeBind(host, host.readDirectory),
storeFilesChangingSignatureDuringEmit: host.storeFilesChangingSignatureDuringEmit,
storeSignatureInfo: host.storeSignatureInfo,
jsDocParsingMode: host.jsDocParsingMode,
};
return compilerHost;
Expand Down Expand Up @@ -847,7 +847,7 @@ export function createProgramHost<T extends BuilderProgram = EmitAndSemanticDiag
writeFile: (path, data, writeByteOrderMark) => system.writeFile(path, data, writeByteOrderMark),
createHash: maybeBind(system, system.createHash),
createProgram: createProgram || createEmitAndSemanticDiagnosticsBuilderProgram as any as CreateProgram<T>,
storeFilesChangingSignatureDuringEmit: system.storeFilesChangingSignatureDuringEmit,
storeSignatureInfo: system.storeSignatureInfo,
now: maybeBind(system, system.now),
};
}
Expand Down
Loading