From 6547c1816f23ddcc42eb7f8ec3fd6026893eafd1 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 27 Apr 2017 20:27:48 -0700 Subject: [PATCH] check ambient modules before reusing old state --- src/compiler/diagnosticMessages.json | 8 +- src/compiler/program.ts | 126 +++++++++--------- src/compiler/types.ts | 1 + src/compiler/utilities.ts | 2 +- .../unittests/reuseProgramStructure.ts | 3 +- 5 files changed, 74 insertions(+), 66 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index ee379787768..b045def34e7 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3201,9 +3201,13 @@ "category": "Message", "code": 6182 }, - "Reusing module resolutions originating in '{0}' since this file was not modified.": { + "Reusing resolution of module '{0}' to file '{1}' from old program.": { "category": "Message", - "code": 6183 + "code": 618 + }, + "Reusing module resolutions originating in '{0}' since resolutions are unchanged from old program.": { + "category": "Message", + "code": 6184 }, "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 8bd7bbbc765..15dd8a097d4 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -480,6 +480,7 @@ namespace ts { interface OldProgramState { program: Program | undefined; file: SourceFile; + /** The collection of paths modified *since* the old program. */ modifiedFilePaths: Path[]; } @@ -497,91 +498,83 @@ namespace ts { return resolveModuleNamesWorker(moduleNames, containingFile); } - const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); - if (oldSourceFile === file) { - // `file` is unchanged from the old program, so we can reuse old module resolutions. - if (isTraceEnabled(options, host)) { - trace(host, Diagnostics.Reusing_module_resolutions_originating_in_0_since_this_file_was_not_modified, file.path); - } - const oldSourceFileResult: ResolvedModuleFull[] = []; - for (const moduleName of moduleNames) { - const resolvedModule = oldSourceFile.resolvedModules.get(moduleName); - oldSourceFileResult.push(resolvedModule); - } - return oldSourceFileResult; - } - - // at this point we know that either + // at this point we know at least one of the following hold: // - file has local declarations for ambient modules - // OR // - old program state is available - // OR - // - both of items above - // With this it is possible that we can tell how some module names from the initial list will be resolved - // without doing actual resolution (in particular if some name was resolved to ambient module). - // Such names should be excluded from the list of module names that will be provided to `resolveModuleNamesWorker` - // since we don't want to resolve them again. + // With this information, we can infer some module resolutions without performing resolution. - // this is a list of modules for which we cannot predict resolution so they should be actually resolved + /** An ordered list of module names for which we cannot recover the resolution. */ let unknownModuleNames: string[]; - // this is a list of combined results assembles from predicted and resolved results. - // Order in this list matches the order in the original list of module names `moduleNames` which is important - // so later we can split results to resolutions of modules and resolutions of module augmentations. + /** + * The indexing of elements in this list matches that of `moduleNames`. + * + * Before combining results, result[i] is in one of the following states: + * * undefined: needs to be recomputed, + * * predictedToResolveToAmbientModuleMarker: known to be an ambient module. + * Needs to be reset to undefined before returning, + * * ResolvedModuleFull instance: can be reused. + */ let result: ResolvedModuleFull[]; - // a transient placeholder that is used to mark predicted resolution in the result list + /** A transient placeholder used to mark predicted resolution in the result list. */ const predictedToResolveToAmbientModuleMarker: ResolvedModuleFull = {}; + const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); + for (let i = 0; i < moduleNames.length; i++) { const moduleName = moduleNames[i]; - // module name is known to be resolved to ambient module if - // - module name is contained in the list of ambient modules that are locally declared in the file - // - in the old program module name was resolved to ambient module whose declaration is in non-modified file + // TODO: if we want to reuse resolutions more aggressively, refine this to check for whether the + // text of the corresponding modulenames has changed. + if (file === oldSourceFile) { + const oldResolvedModule = oldSourceFile && oldSourceFile.resolvedModules.get(moduleName); + if (oldResolvedModule) { + if (isTraceEnabled(options, host)) { + trace(host, Diagnostics.Reusing_resolution_of_module_0_to_file_1_from_old_program, moduleName, containingFile); + } + (result || (result = new Array(moduleNames.length)))[i] = oldResolvedModule; + continue; + } + } + // We know moduleName resolves to an ambient module provided that moduleName: + // - is in the list of ambient modules locally declared in the current source file. + // - resolved to an ambient module in the old program whose declaration is in an unmodified file // (so the same module declaration will land in the new program) - let isKnownToResolveToAmbientModule = false; + let resolvesToAmbientModuleInNonModifiedFile = false; if (contains(file.ambientModuleNames, moduleName)) { - isKnownToResolveToAmbientModule = true; + resolvesToAmbientModuleInNonModifiedFile = true; if (isTraceEnabled(options, host)) { trace(host, Diagnostics.Module_0_was_resolved_as_locally_declared_ambient_module_in_file_1, moduleName, containingFile); } } else { - isKnownToResolveToAmbientModule = checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName, oldProgramState); + resolvesToAmbientModuleInNonModifiedFile = moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName, oldProgramState); } - if (isKnownToResolveToAmbientModule) { - if (!unknownModuleNames) { - // found a first module name for which result can be prediced - // this means that this module name should not be passed to `resolveModuleNamesWorker`. - // We'll use a separate list for module names that are definitely unknown. - result = new Array(moduleNames.length); - // copy all module names that appear before the current one in the list - // since they are known to be unknown - unknownModuleNames = moduleNames.slice(0, i); - } - // Mark predicted resolution in the result list. - result[i] = predictedToResolveToAmbientModuleMarker; + if (resolvesToAmbientModuleInNonModifiedFile) { + (result || (result = new Array(moduleNames.length)))[i] = predictedToResolveToAmbientModuleMarker; } - else if (unknownModuleNames) { - // found unknown module name and we are already using separate list for those - add it to the list - unknownModuleNames.push(moduleName); + else { + // Resolution failed in the old program, or resolved to an ambient module for which we can't reuse the result. + (unknownModuleNames || (unknownModuleNames = [])).push(moduleName); } } - if (!unknownModuleNames) { - // we've looked throught the list but have not seen any predicted resolution - // use default logic - return resolveModuleNamesWorker(moduleNames, containingFile); - } - - const resolutions = unknownModuleNames.length + const resolutions = unknownModuleNames && unknownModuleNames.length ? resolveModuleNamesWorker(unknownModuleNames, containingFile) : emptyArray; - // combine results of resolutions and predicted results + // Combine results of resolutions and predicted results + if (!result) { + // There were no unresolved/ambient resolutions. + Debug.assert(resolutions.length === moduleNames.length); + return resolutions; + } + let j = 0; for (let i = 0; i < result.length; i++) { - if (result[i] === predictedToResolveToAmbientModuleMarker) { - result[i] = undefined; + if (result[i]) { + if (result[i] === predictedToResolveToAmbientModuleMarker) { + result[i] = undefined; + } } else { result[i] = resolutions[j]; @@ -589,9 +582,12 @@ namespace ts { } } Debug.assert(j === resolutions.length); + return result; - function checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean { + // TODO: if we change our policy of rechecking failed lookups on each program create, + // we should adjust the value returned here. + function moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean { const resolutionToFile = getResolvedModule(oldProgramState.file, moduleName); if (resolutionToFile) { // module used to be resolved to file - ignore it @@ -706,7 +702,7 @@ namespace ts { modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path); // try to verify results of module resolution - for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) { + modifiedSourceFilesLoop: for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) { const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory); if (resolveModuleNamesWorker) { const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), getTextOfLiteral); @@ -715,7 +711,8 @@ namespace ts { // ensure that module resolution results are still correct const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo); if (resolutionsChanged) { - return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + continue modifiedSourceFilesLoop; } } if (resolveTypeReferenceDirectiveNamesWorker) { @@ -724,7 +721,8 @@ namespace ts { // ensure that types resolutions are still correct const resolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, resolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, typeDirectiveIsEqualTo); if (resolutionsChanged) { - return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + continue modifiedSourceFilesLoop; } } // pass the cache of module/types resolutions from the old source file @@ -732,6 +730,10 @@ namespace ts { newSourceFile.resolvedTypeReferenceDirectiveNames = oldSourceFile.resolvedTypeReferenceDirectiveNames; } + if (oldProgram.structureIsReused !== StructureIsReused.Completely) { + return oldProgram.structureIsReused; + } + // update fileName -> file mapping for (let i = 0; i < newSourceFiles.length; i++) { filesByName.set(filePaths[i], newSourceFiles[i]); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index ec722a9309e..4c850d673a9 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2409,6 +2409,7 @@ namespace ts { /* @internal */ structureIsReused?: StructureIsReused; } + /* @internal */ export const enum StructureIsReused { Completely, ModulesInUneditedFiles, diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index f34c15ee346..4381682c9f4 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -123,7 +123,7 @@ namespace ts { /* @internal */ export function hasChangesInResolutions(names: string[], newResolutions: T[], oldResolutions: Map, comparer: (oldResolution: T, newResolution: T) => boolean): boolean { if (names.length !== newResolutions.length) { - return false; + return true; } for (let i = 0; i < names.length; i++) { const newResolution = newResolutions[i]; diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts index 8810d8a6401..d7140cfc8b2 100644 --- a/src/harness/unittests/reuseProgramStructure.ts +++ b/src/harness/unittests/reuseProgramStructure.ts @@ -547,7 +547,8 @@ namespace ts { "File 'node_modules/@types/typerefs2/package.json' does not exist.", "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", - "Reusing module resolutions originating in 'f2.ts' since this file was not modified." + "Reusing resolution of module './b2' to file 'f2.ts' from old program.", + "Reusing resolution of module './f1' to file 'f2.ts' from old program." ], "should reuse module resolutions in f2 since it is unchanged"); }); });