From 5739b6897a3bad40ff5551dd4df39ea4fe32924b Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 8 Sep 2017 18:02:10 -0700 Subject: [PATCH] Do not create map just to store empty reference files. Also update file as changed if file text is same but it had invalidated resolution --- src/compiler/builder.ts | 62 ++++++++++++++++--- src/compiler/utilities.ts | 48 +++++++------- .../unittests/tsserverProjectSystem.ts | 2 +- 3 files changed, 76 insertions(+), 36 deletions(-) diff --git a/src/compiler/builder.ts b/src/compiler/builder.ts index 68fb36b9144..40f3cdadb64 100644 --- a/src/compiler/builder.ts +++ b/src/compiler/builder.ts @@ -48,6 +48,7 @@ namespace ts { addScriptInfo(program: Program, sourceFile: SourceFile): void; removeScriptInfo(path: Path): void; updateScriptInfo(program: Program, sourceFile: SourceFile): void; + updaterScriptInfoWithSameVersion(program: Program, sourceFile: SourceFile): boolean; /** * Gets the files affected by the script info which has updated shape from the known one */ @@ -143,11 +144,15 @@ namespace ts { } function updateExistingFileInfo(program: Program, existingInfo: FileInfo, sourceFile: SourceFile, hasInvalidatedResolution: HasInvalidatedResolution) { - if (existingInfo.version !== sourceFile.version || hasInvalidatedResolution(sourceFile.path)) { + if (existingInfo.version !== sourceFile.version) { registerChangedFile(sourceFile.path, sourceFile.fileName); existingInfo.version = sourceFile.version; emitHandler.updateScriptInfo(program, sourceFile); } + else if (hasInvalidatedResolution(sourceFile.path) && + emitHandler.updaterScriptInfoWithSameVersion(program, sourceFile)) { + registerChangedFile(sourceFile.path, sourceFile.fileName); + } } function ensureProgramGraph(program: Program) { @@ -338,8 +343,8 @@ namespace ts { /** * Gets the referenced files for a file from the program with values for the keys as referenced file's path to be true */ - function getReferencedFiles(program: Program, sourceFile: SourceFile): Map { - const referencedFiles = createMap(); + function getReferencedFiles(program: Program, sourceFile: SourceFile): Map | undefined { + let referencedFiles: Map | undefined; // We need to use a set here since the code can contain the same import twice, // but that will only be one dependency. @@ -351,7 +356,7 @@ namespace ts { if (symbol && symbol.declarations && symbol.declarations[0]) { const declarationSourceFile = getSourceFileOfNode(symbol.declarations[0]); if (declarationSourceFile) { - referencedFiles.set(declarationSourceFile.path, true); + addReferencedFile(declarationSourceFile.path); } } } @@ -362,7 +367,7 @@ namespace ts { if (sourceFile.referencedFiles && sourceFile.referencedFiles.length > 0) { for (const referencedFile of sourceFile.referencedFiles) { const referencedPath = toPath(referencedFile.fileName, sourceFileDirectory, getCanonicalFileName); - referencedFiles.set(referencedPath, true); + addReferencedFile(referencedPath); } } @@ -375,11 +380,18 @@ namespace ts { const fileName = resolvedTypeReferenceDirective.resolvedFileName; const typeFilePath = toPath(fileName, sourceFileDirectory, getCanonicalFileName); - referencedFiles.set(typeFilePath, true); + addReferencedFile(typeFilePath); }); } return referencedFiles; + + function addReferencedFile(referencedPath: Path) { + if (!referencedFiles) { + referencedFiles = createMap(); + } + referencedFiles.set(referencedPath, true); + } } /** @@ -402,6 +414,7 @@ namespace ts { addScriptInfo: noop, removeScriptInfo: noop, updateScriptInfo: noop, + updaterScriptInfoWithSameVersion: returnFalse, getFilesAffectedByUpdatedShape }; @@ -421,12 +434,45 @@ namespace ts { return { addScriptInfo: setReferences, removeScriptInfo, - updateScriptInfo: setReferences, + updateScriptInfo: updateReferences, + updaterScriptInfoWithSameVersion: updateReferencesTrackingChangedReferences, getFilesAffectedByUpdatedShape }; function setReferences(program: Program, sourceFile: SourceFile) { - references.set(sourceFile.path, getReferencedFiles(program, sourceFile)); + const newReferences = getReferencedFiles(program, sourceFile); + if (newReferences) { + references.set(sourceFile.path, getReferencedFiles(program, sourceFile)); + } + } + + function updateReferences(program: Program, sourceFile: SourceFile) { + const newReferences = getReferencedFiles(program, sourceFile); + if (newReferences) { + references.set(sourceFile.path, newReferences); + } + else { + references.delete(sourceFile.path); + } + } + + function updateReferencesTrackingChangedReferences(program: Program, sourceFile: SourceFile) { + const newReferences = getReferencedFiles(program, sourceFile); + if (!newReferences) { + // Changed if we had references + return references.delete(sourceFile.path); + } + + const oldReferences = references.get(sourceFile.path); + references.set(sourceFile.path, newReferences); + if (!oldReferences || oldReferences.size !== newReferences.size) { + return true; + } + + // If there are any new references that werent present previously there is change + return forEachEntry(newReferences, (_true, referencedPath) => !oldReferences.delete(referencedPath)) || + // Otherwise its changed if there are more references previously than now + !!oldReferences.size; } function removeScriptInfo(removedFilePath: Path) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 5785692017d..868e9e9420e 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3513,34 +3513,28 @@ namespace ts { * Mutates the map with newMap such that keys in map will be same as newMap. */ export function mutateMap(map: Map, newMap: ReadonlyMap, options: MutateMapOptions) { - // If there are new values update them - if (newMap) { - const { createNewValue, onDeleteValue, onExistingValue } = options; - // Needs update - map.forEach((existingValue, key) => { - const valueInNewMap = newMap.get(key); - // Not present any more in new map, remove it - if (valueInNewMap === undefined) { - map.delete(key); - onDeleteValue(existingValue, key); - } - // If present notify about existing values - else if (onExistingValue) { - onExistingValue(existingValue, valueInNewMap, key); - } - }); + const { createNewValue, onDeleteValue, onExistingValue } = options; + // Needs update + map.forEach((existingValue, key) => { + const valueInNewMap = newMap.get(key); + // Not present any more in new map, remove it + if (valueInNewMap === undefined) { + map.delete(key); + onDeleteValue(existingValue, key); + } + // If present notify about existing values + else if (onExistingValue) { + onExistingValue(existingValue, valueInNewMap, key); + } + }); - // Add new values that are not already present - newMap.forEach((valueInNewMap, key) => { - if (!map.has(key)) { - // New values - map.set(key, createNewValue(key, valueInNewMap)); - } - }); - } - else { - clearMap(map, options.onDeleteValue); - } + // Add new values that are not already present + newMap.forEach((valueInNewMap, key) => { + if (!map.has(key)) { + // New values + map.set(key, createNewValue(key, valueInNewMap)); + } + }); } /** diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 7d49e82151f..7752d26e208 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -5275,7 +5275,7 @@ namespace ts.projectSystem { watchedRecursiveDirectories.length = 2; verifyProject(); - const changedFiles = limitHit ? [file1.path, file2.path, file3.path, libFile.path] : [file1.path, file2.path]; + const changedFiles = [file1.path, file2.path]; verifyProjectChangedEventHandler([{ eventName: server.ProjectChangedEvent, data: {