Instead of creating filter for subDirectories to watch in the ancestor directory of root, watch those subDirectories for failed lookup locations

sheetalkamat · sheetalkamat · commit 939e3e478042 · 2018-05-29T14:49:19.000-07:00
Before this change, when failed lookup location didnt fall in root directory, we tried to find the ancestor directory of the rootDirectory to watch. We also created subDirectory map for the directories that are being watched so we dont go through invalidation if path is unwanted directory With this change, we will watch those subdirectories instead of root. On windows node supports file system level recursive watching so the earlier approach was better because we reduced number of watches created But on other os, since node doesnt support it, we create the watches for existing folders outselves, so earlier approach becomes expensive. This should be better compromize to satisfy both types of OS. Fixes microsoft#24434
diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts
@@ -60,15 +60,12 @@ namespace ts {
         watcher: FileWatcher;
         /** ref count keeping this directory watch alive */
         refCount: number;
-        /** map of refcount for the subDirectory */
-        subDirectoryMap?: Map<number>;
     }
 
     interface DirectoryOfFailedLookupWatch {
         dir: string;
         dirPath: Path;
         ignore?: true;
-        subDirectory?: Path;
     }
 
     export const maxNumberOfFilesToIterateForInvalidation = 256;
@@ -403,20 +400,21 @@ namespace ts {
             }
 
             // Use some ancestor of the root directory
-            let subDirectory: Path | undefined;
+            let subDirectoryPath: Path | undefined, subDirectory: string | undefined;
             if (rootPath !== undefined) {
                 while (!isInDirectoryPath(dirPath, rootPath)) {
                     const parentPath = getDirectoryPath(dirPath);
                     if (parentPath === dirPath) {
                         break;
                     }
-                    subDirectory = dirPath.slice(parentPath.length + directorySeparator.length) as Path;
+                    subDirectoryPath = dirPath;
+                    subDirectory = dir;
                     dirPath = parentPath;
                     dir = getDirectoryPath(dir);
                 }
             }
 
-            return filterFSRootDirectoriesToWatch({ dir, dirPath, subDirectory }, dirPath);
+            return filterFSRootDirectoriesToWatch({ dir: subDirectory || dir, dirPath: subDirectoryPath || dirPath }, dirPath);
         }
 
         function isPathWithDefaultFailedLookupExtension(path: Path) {
@@ -439,7 +437,7 @@ namespace ts {
             let setAtRoot = false;
             for (const failedLookupLocation of failedLookupLocations) {
                 const failedLookupLocationPath = resolutionHost.toPath(failedLookupLocation);
-                const { dir, dirPath, ignore , subDirectory } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
+                const { dir, dirPath, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
                 if (!ignore) {
                     // If the failed lookup location path is not one of the supported extensions,
                     // store it in the custom path
@@ -451,7 +449,7 @@ namespace ts {
                         setAtRoot = true;
                     }
                     else {
-                        setDirectoryWatcher(dir, dirPath, subDirectory);
+                        setDirectoryWatcher(dir, dirPath);
                     }
                 }
             }
@@ -461,20 +459,13 @@ namespace ts {
             }
         }
 
-        function setDirectoryWatcher(dir: string, dirPath: Path, subDirectory?: Path) {
-            let dirWatcher = directoryWatchesOfFailedLookups.get(dirPath);
+        function setDirectoryWatcher(dir: string, dirPath: Path) {
+            const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath);
             if (dirWatcher) {
                 dirWatcher.refCount++;
             }
             else {
-                dirWatcher = { watcher: createDirectoryWatcher(dir, dirPath), refCount: 1 };
-                directoryWatchesOfFailedLookups.set(dirPath, dirWatcher);
-            }
-
-            if (subDirectory) {
-                const subDirectoryMap = dirWatcher.subDirectoryMap || (dirWatcher.subDirectoryMap = createMap());
-                const existing = subDirectoryMap.get(subDirectory) || 0;
-                subDirectoryMap.set(subDirectory, existing + 1);
+                directoryWatchesOfFailedLookups.set(dirPath, { watcher: createDirectoryWatcher(dir, dirPath), refCount: 1 });
             }
         }
 
@@ -492,7 +483,7 @@ namespace ts {
             let removeAtRoot = false;
             for (const failedLookupLocation of failedLookupLocations) {
                 const failedLookupLocationPath = resolutionHost.toPath(failedLookupLocation);
-                const { dirPath, ignore, subDirectory } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
+                const { dirPath, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
                 if (!ignore) {
                     const refCount = customFailedLookupPaths.get(failedLookupLocationPath);
                     if (refCount) {
@@ -509,7 +500,7 @@ namespace ts {
                         removeAtRoot = true;
                     }
                     else {
-                        removeDirectoryWatcher(dirPath, subDirectory);
+                        removeDirectoryWatcher(dirPath);
                     }
                 }
             }
@@ -518,30 +509,12 @@ namespace ts {
             }
         }
 
-        function removeDirectoryWatcher(dirPath: string, subDirectory?: Path) {
+        function removeDirectoryWatcher(dirPath: string) {
             const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath)!;
-            if (subDirectory) {
-                const existing = dirWatcher.subDirectoryMap!.get(subDirectory)!;
-                if (existing === 1) {
-                    dirWatcher.subDirectoryMap!.delete(subDirectory);
-                }
-                else {
-                    dirWatcher.subDirectoryMap!.set(subDirectory, existing - 1);
-                }
-            }
             // Do not close the watcher yet since it might be needed by other failed lookup locations.
             dirWatcher.refCount--;
         }
 
-        function inWatchedSubdirectory(dirPath: Path, fileOrDirectoryPath: Path) {
-            const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath);
-            if (!dirWatcher || !dirWatcher.subDirectoryMap) return false;
-            return forEachKey(dirWatcher.subDirectoryMap, subDirectory => {
-                const fullSubDirectory = `${dirPath}/${subDirectory}` as Path;
-                return fullSubDirectory === fileOrDirectoryPath || isInDirectoryPath(fullSubDirectory, fileOrDirectoryPath);
-            });
-        }
-
         function createDirectoryWatcher(directory: string, dirPath: Path) {
             return resolutionHost.watchDirectoryOfFailedLookupLocation(directory, fileOrDirectory => {
                 const fileOrDirectoryPath = resolutionHost.toPath(fileOrDirectory);
@@ -550,13 +523,8 @@ namespace ts {
                     cachedDirectoryStructureHost.addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath);
                 }
 
-                // If the files are added to project root or node_modules directory, always run through the invalidation process
-                // Otherwise run through invalidation only if adding to the immediate directory
-                if (!allFilesHaveInvalidatedResolution &&
-                    (dirPath === rootPath || isNodeModulesDirectory(dirPath) || getDirectoryPath(fileOrDirectoryPath) === dirPath || inWatchedSubdirectory(dirPath, fileOrDirectoryPath))) {
-                    if (invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath)) {
-                        resolutionHost.onInvalidatedResolution();
-                    }
+                if (!allFilesHaveInvalidatedResolution && invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath)) {
+                    resolutionHost.onInvalidatedResolution();
                 }
             }, WatchDirectoryFlags.Recursive);
         }
diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts
@@ -2533,7 +2533,7 @@ declare module "fs" {
             createWatchOfConfigFile("tsconfig.json", host);
             checkWatchedFilesDetailed(host, [libFile.path, mainFile.path, config.path, linkedPackageIndex.path, linkedPackageOther.path], 1);
             checkWatchedDirectories(host, emptyArray, /*recursive*/ false);
-            checkWatchedDirectoriesDetailed(host, [mainPackageRoot, projectRoot, `${mainPackageRoot}/node_modules/@types`, `${projectRoot}/node_modules/@types`], 1, /*recursive*/ true);
+            checkWatchedDirectoriesDetailed(host, [mainPackageRoot, linkedPackageRoot, `${mainPackageRoot}/node_modules/@types`, `${projectRoot}/node_modules/@types`], 1, /*recursive*/ true);
         });
     });
 }
diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts
@@ -7839,7 +7839,7 @@ new C();`
                     const filesAfterCompilation = [...filesWithNodeModulesSetup, recongnizerTextDistTypingFile];
 
                     const watchedDirectoriesWithResolvedModule = [`${recognizersDateTime}/src`, withPathMapping ? packages : recognizersDateTime, ...getTypeRootsFromLocation(recognizersDateTime)];
-                    const watchedDirectoriesWithUnresolvedModule = [recognizersDateTime, ...watchedDirectoriesWithResolvedModule, ...getNodeModuleDirectories(packages)];
+                    const watchedDirectoriesWithUnresolvedModule = [recognizersDateTime, ...(withPathMapping ? [recognizersText] : emptyArray), ...watchedDirectoriesWithResolvedModule, ...getNodeModuleDirectories(packages)];
 
                     function verifyProjectWithResolvedModule(session: TestSession) {
                         const projectService = session.getProjectService();

Original file line number	Diff line number	Diff line change
`@@ -60,15 +60,12 @@ namespace ts {`
`60`	`60`	`watcher: FileWatcher;`
`61`	`61`	`/** ref count keeping this directory watch alive */`
`62`	`62`	`refCount: number;`
`63`		`- /** map of refcount for the subDirectory */`
`64`		`- subDirectoryMap?: Map<number>;`
`65`	`63`	`}`
`66`	`64`
`67`	`65`	`interface DirectoryOfFailedLookupWatch {`
`68`	`66`	`dir: string;`
`69`	`67`	`dirPath: Path;`
`70`	`68`	`ignore?: true;`
`71`		`- subDirectory?: Path;`
`72`	`69`	`}`
`73`	`70`
`74`	`71`	`export const maxNumberOfFilesToIterateForInvalidation = 256;`
`@@ -403,20 +400,21 @@ namespace ts {`
`403`	`400`	`}`
`404`	`401`
`405`	`402`	`// Use some ancestor of the root directory`
`406`		`- let subDirectory: Path \| undefined;`
	`403`	`+ let subDirectoryPath: Path \| undefined, subDirectory: string \| undefined;`
`407`	`404`	`if (rootPath !== undefined) {`
`408`	`405`	`while (!isInDirectoryPath(dirPath, rootPath)) {`
`409`	`406`	`const parentPath = getDirectoryPath(dirPath);`
`410`	`407`	`if (parentPath === dirPath) {`
`411`	`408`	`break;`
`412`	`409`	`}`
`413`		`- subDirectory = dirPath.slice(parentPath.length + directorySeparator.length) as Path;`
	`410`	`+ subDirectoryPath = dirPath;`
	`411`	`+ subDirectory = dir;`
`414`	`412`	`dirPath = parentPath;`
`415`	`413`	`dir = getDirectoryPath(dir);`
`416`	`414`	`}`
`417`	`415`	`}`
`418`	`416`
`419`		`- return filterFSRootDirectoriesToWatch({ dir, dirPath, subDirectory }, dirPath);`
	`417`	`+ return filterFSRootDirectoriesToWatch({ dir: subDirectory \|\| dir, dirPath: subDirectoryPath \|\| dirPath }, dirPath);`
`420`	`418`	`}`
`421`	`419`
`422`	`420`	`function isPathWithDefaultFailedLookupExtension(path: Path) {`
`@@ -439,7 +437,7 @@ namespace ts {`
`439`	`437`	`let setAtRoot = false;`
`440`	`438`	`for (const failedLookupLocation of failedLookupLocations) {`
`441`	`439`	`const failedLookupLocationPath = resolutionHost.toPath(failedLookupLocation);`
`442`		`- const { dir, dirPath, ignore , subDirectory } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);`
	`440`	`+ const { dir, dirPath, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);`
`443`	`441`	`if (!ignore) {`
`444`	`442`	`// If the failed lookup location path is not one of the supported extensions,`
`445`	`443`	`// store it in the custom path`
`@@ -451,7 +449,7 @@ namespace ts {`
`451`	`449`	`setAtRoot = true;`
`452`	`450`	`}`
`453`	`451`	`else {`
`454`		`- setDirectoryWatcher(dir, dirPath, subDirectory);`
	`452`	`+ setDirectoryWatcher(dir, dirPath);`
`455`	`453`	`}`
`456`	`454`	`}`
`457`	`455`	`}`
`@@ -461,20 +459,13 @@ namespace ts {`
`461`	`459`	`}`
`462`	`460`	`}`
`463`	`461`
`464`		`- function setDirectoryWatcher(dir: string, dirPath: Path, subDirectory?: Path) {`
`465`		`- let dirWatcher = directoryWatchesOfFailedLookups.get(dirPath);`
	`462`	`+ function setDirectoryWatcher(dir: string, dirPath: Path) {`
	`463`	`+ const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath);`
`466`	`464`	`if (dirWatcher) {`
`467`	`465`	`dirWatcher.refCount++;`
`468`	`466`	`}`
`469`	`467`	`else {`
`470`		`- dirWatcher = { watcher: createDirectoryWatcher(dir, dirPath), refCount: 1 };`
`471`		`- directoryWatchesOfFailedLookups.set(dirPath, dirWatcher);`
`472`		`- }`
`473`		`-`
`474`		`- if (subDirectory) {`
`475`		`- const subDirectoryMap = dirWatcher.subDirectoryMap \|\| (dirWatcher.subDirectoryMap = createMap());`
`476`		`- const existing = subDirectoryMap.get(subDirectory) \|\| 0;`
`477`		`- subDirectoryMap.set(subDirectory, existing + 1);`
	`468`	`+ directoryWatchesOfFailedLookups.set(dirPath, { watcher: createDirectoryWatcher(dir, dirPath), refCount: 1 });`
`478`	`469`	`}`
`479`	`470`	`}`
`480`	`471`
`@@ -492,7 +483,7 @@ namespace ts {`
`492`	`483`	`let removeAtRoot = false;`
`493`	`484`	`for (const failedLookupLocation of failedLookupLocations) {`
`494`	`485`	`const failedLookupLocationPath = resolutionHost.toPath(failedLookupLocation);`
`495`		`- const { dirPath, ignore, subDirectory } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);`
	`486`	`+ const { dirPath, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);`
`496`	`487`	`if (!ignore) {`
`497`	`488`	`const refCount = customFailedLookupPaths.get(failedLookupLocationPath);`
`498`	`489`	`if (refCount) {`
`@@ -509,7 +500,7 @@ namespace ts {`
`509`	`500`	`removeAtRoot = true;`
`510`	`501`	`}`
`511`	`502`	`else {`
`512`		`- removeDirectoryWatcher(dirPath, subDirectory);`
	`503`	`+ removeDirectoryWatcher(dirPath);`
`513`	`504`	`}`
`514`	`505`	`}`
`515`	`506`	`}`
`@@ -518,30 +509,12 @@ namespace ts {`
`518`	`509`	`}`
`519`	`510`	`}`
`520`	`511`
`521`		`- function removeDirectoryWatcher(dirPath: string, subDirectory?: Path) {`
	`512`	`+ function removeDirectoryWatcher(dirPath: string) {`
`522`	`513`	`const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath)!;`
`523`		`- if (subDirectory) {`
`524`		`- const existing = dirWatcher.subDirectoryMap!.get(subDirectory)!;`
`525`		`- if (existing === 1) {`
`526`		`- dirWatcher.subDirectoryMap!.delete(subDirectory);`
`527`		`- }`
`528`		`- else {`
`529`		`- dirWatcher.subDirectoryMap!.set(subDirectory, existing - 1);`
`530`		`- }`
`531`		`- }`
`532`	`514`	`// Do not close the watcher yet since it might be needed by other failed lookup locations.`
`533`	`515`	`dirWatcher.refCount--;`
`534`	`516`	`}`
`535`	`517`
`536`		`- function inWatchedSubdirectory(dirPath: Path, fileOrDirectoryPath: Path) {`
`537`		`- const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath);`
`538`		`- if (!dirWatcher \|\| !dirWatcher.subDirectoryMap) return false;`
`539`		`- return forEachKey(dirWatcher.subDirectoryMap, subDirectory => {`
`540`		- const fullSubDirectory = `${dirPath}/${subDirectory}` as Path;
`541`		`- return fullSubDirectory === fileOrDirectoryPath \|\| isInDirectoryPath(fullSubDirectory, fileOrDirectoryPath);`
`542`		`- });`
`543`		`- }`
`544`		`-`
`545`	`518`	`function createDirectoryWatcher(directory: string, dirPath: Path) {`
`546`	`519`	`return resolutionHost.watchDirectoryOfFailedLookupLocation(directory, fileOrDirectory => {`
`547`	`520`	`const fileOrDirectoryPath = resolutionHost.toPath(fileOrDirectory);`
`@@ -550,13 +523,8 @@ namespace ts {`
`550`	`523`	`cachedDirectoryStructureHost.addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath);`
`551`	`524`	`}`
`552`	`525`
`553`		`- // If the files are added to project root or node_modules directory, always run through the invalidation process`
`554`		`- // Otherwise run through invalidation only if adding to the immediate directory`
`555`		`- if (!allFilesHaveInvalidatedResolution &&`
`556`		`- (dirPath === rootPath \|\| isNodeModulesDirectory(dirPath) \|\| getDirectoryPath(fileOrDirectoryPath) === dirPath \|\| inWatchedSubdirectory(dirPath, fileOrDirectoryPath))) {`
`557`		`- if (invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath)) {`
`558`		`- resolutionHost.onInvalidatedResolution();`
`559`		`- }`
	`526`	`+ if (!allFilesHaveInvalidatedResolution && invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath)) {`
	`527`	`+ resolutionHost.onInvalidatedResolution();`
`560`	`528`	`}`
`561`	`529`	`}, WatchDirectoryFlags.Recursive);`
`562`	`530`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2533,7 +2533,7 @@ declare module "fs" {`
`2533`	`2533`	`createWatchOfConfigFile("tsconfig.json", host);`
`2534`	`2534`	`checkWatchedFilesDetailed(host, [libFile.path, mainFile.path, config.path, linkedPackageIndex.path, linkedPackageOther.path], 1);`
`2535`	`2535`	`checkWatchedDirectories(host, emptyArray, /recursive/ false);`
`2536`		- checkWatchedDirectoriesDetailed(host, [mainPackageRoot, projectRoot, `${mainPackageRoot}/node_modules/@types`, `${projectRoot}/node_modules/@types`], 1, /recursive/ true);
	`2536`	+ checkWatchedDirectoriesDetailed(host, [mainPackageRoot, linkedPackageRoot, `${mainPackageRoot}/node_modules/@types`, `${projectRoot}/node_modules/@types`], 1, /recursive/ true);
`2537`	`2537`	`});`
`2538`	`2538`	`});`
`2539`	`2539`	`}`