From 254e39306f771430539fbe94dfe8f11c30e024cd Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 28 Aug 2017 12:34:58 -0700 Subject: [PATCH] Watch failed lookups recursively to reduce number of directory watches Also we dont need to watch type roots any more --- src/compiler/core.ts | 2 + src/compiler/resolutionCache.ts | 87 +++++++++++++++++-- src/compiler/watchedProgram.ts | 9 +- .../unittests/tsserverProjectSystem.ts | 71 ++++----------- src/harness/virtualFileSystemWithWatch.ts | 2 +- src/server/editorServices.ts | 21 ----- src/server/project.ts | 38 +++----- 7 files changed, 115 insertions(+), 115 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index bb4cf903a98..9c761b47fa0 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1809,6 +1809,8 @@ namespace ts { * Removes a trailing directory separator from a path. * @param path The path. */ + export function removeTrailingDirectorySeparator(path: Path): Path; + export function removeTrailingDirectorySeparator(path: string): string; export function removeTrailingDirectorySeparator(path: string) { if (path.charAt(path.length - 1) === directorySeparator) { return path.substr(0, path.length - 1); diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index d1bd285019f..ca82a68f489 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -14,6 +14,8 @@ namespace ts { invalidateResolutionOfFile(filePath: Path): void; createHasInvalidatedResolution(): HasInvalidatedResolution; + setRootDirectory(dir: string): void; + clear(): void; } @@ -32,13 +34,15 @@ namespace ts { export interface ResolutionCacheHost extends ModuleResolutionHost { toPath(fileName: string): Path; getCompilationSettings(): CompilerOptions; - watchDirectoryOfFailedLookupLocation(directory: string, cb: DirectoryWatcherCallback): FileWatcher; + watchDirectoryOfFailedLookupLocation(directory: string, cb: DirectoryWatcherCallback, flags: WatchDirectoryFlags): FileWatcher; onInvalidatedResolution(): void; getCachedPartialSystem?(): CachedPartialSystem; projectName?: string; getGlobalCache?(): string | undefined; } + const MAX_DIRPATHS_TO_RECURSE = 5; + export function createResolutionCache(resolutionHost: ResolutionCacheHost): ResolutionCache { let filesWithChangedSetOfUnresolvedImports: Path[] | undefined; let filesWithInvalidatedResolutions: Map | undefined; @@ -50,6 +54,9 @@ namespace ts { const resolvedTypeReferenceDirectives = createMap>(); const directoryWatchesOfFailedLookups = createMap(); + const failedLookupLocationToDirPath = createMap(); + let rootDir: string; + let rootPath: Path; return { startRecordingFilesWithChangedResolutions, finishRecordingFilesWithChangedResolutions, @@ -57,12 +64,27 @@ namespace ts { resolveTypeReferenceDirectives, invalidateResolutionOfFile, createHasInvalidatedResolution, + setRootDirectory, clear }; + function setRootDirectory(dir: string) { + Debug.assert(!resolvedModuleNames.size && !resolvedTypeReferenceDirectives.size && !directoryWatchesOfFailedLookups.size); + rootDir = removeTrailingDirectorySeparator(getNormalizedAbsolutePath(dir, resolutionHost.getCurrentDirectory())); + rootPath = resolutionHost.toPath(rootDir); + } + + function isInDirectoryPath(dir: Path, file: Path) { + if (dir === undefined || file.length <= dir.length) { + return false; + } + return startsWith(file, dir) && file[dir.length] === directorySeparator; + } + function clear() { // Close all the watches for failed lookup locations, irrespective of refcounts for them since this is to clear the cache clearMap(directoryWatchesOfFailedLookups, closeFileWatcherOf); + failedLookupLocationToDirPath.clear(); resolvedModuleNames.clear(); resolvedTypeReferenceDirectives.clear(); } @@ -196,16 +218,58 @@ namespace ts { } function watchFailedLookupLocation(failedLookupLocation: string, failedLookupLocationPath: Path) { - const dirPath = getDirectoryPath(failedLookupLocationPath); + const cachedDir = failedLookupLocationToDirPath.get(failedLookupLocationPath); + if (cachedDir) { + watchFailedLookupLocationInDirectory(cachedDir, failedLookupLocation, failedLookupLocationPath, /*dir*/ undefined); + return; + } + + if (isInDirectoryPath(rootPath, failedLookupLocationPath)) { + // Watch in directory of rootPath + watchFailedLookupLocationInDirectory(rootPath, failedLookupLocation, failedLookupLocationPath, rootDir); + return; + } + + let dirPath = getDirectoryPath(failedLookupLocationPath); + let dir = getDirectoryPath(getNormalizedAbsolutePath(failedLookupLocation, resolutionHost.getCurrentDirectory())); + for (let i = 0; i < MAX_DIRPATHS_TO_RECURSE; i++) { + const parentPath = getDirectoryPath(dirPath); + if (directoryWatchesOfFailedLookups.has(dirPath) || parentPath === dirPath) { + watchFailedLookupLocationInDirectory(dirPath, failedLookupLocation, failedLookupLocationPath, dir); + return; + } + dirPath = parentPath; + dir = getDirectoryPath(dir); + } + + // Verify there are no watches in parent directory + const ancestorDirPath = getAncestorDirectoryWithWatches(dirPath); + // We wont need directory if we are using ancestor since its already cached + watchFailedLookupLocationInDirectory(ancestorDirPath || dirPath, failedLookupLocation, failedLookupLocationPath, dir); + } + + function getAncestorDirectoryWithWatches(dirPath: Path) { + for (let parentDirPath = getDirectoryPath(dirPath); parentDirPath !== dirPath; parentDirPath = getDirectoryPath(parentDirPath)) { + if (directoryWatchesOfFailedLookups.has(parentDirPath)) { + return parentDirPath; + } + dirPath = parentDirPath; + } + return undefined; + } + + function watchFailedLookupLocationInDirectory(dirPath: Path, failedLookupLocation: string, failedLookupLocationPath: Path, dir: string | undefined) { + failedLookupLocationToDirPath.set(failedLookupLocationPath, dirPath); const watches = directoryWatchesOfFailedLookups.get(dirPath); if (watches) { watches.mapLocations.add(failedLookupLocationPath, failedLookupLocation); } else { + Debug.assert(dir !== undefined); const mapLocations = createMultiMap(); mapLocations.add(failedLookupLocationPath, failedLookupLocation); directoryWatchesOfFailedLookups.set(dirPath, { - watcher: createDirectoryWatcher(getDirectoryPath(failedLookupLocation), dirPath), + watcher: createDirectoryWatcher(dir, dirPath), mapLocations }); } @@ -224,16 +288,22 @@ namespace ts { onAddOrRemoveDirectoryOfFailedLookup(dirPath); resolutionHost.onInvalidatedResolution(); } - else if (onFileAddOrRemoveInDirectoryOfFailedLookup(fileOrFolderPath)) { + else if (onFileAddOrRemoveInDirectoryOfFailedLookup(dirPath, fileOrFolderPath)) { resolutionHost.onInvalidatedResolution(); } - }); + }, WatchDirectoryFlags.Recursive); } function closeFailedLookupLocationWatcher(failedLookupLocation: string, failedLookupLocationPath: Path) { - const dirPath = getDirectoryPath(failedLookupLocationPath); + const dirPath = failedLookupLocationToDirPath.get(failedLookupLocationPath); const watches = directoryWatchesOfFailedLookups.get(dirPath); watches.mapLocations.remove(failedLookupLocationPath, failedLookupLocation); + // If this was last failed lookup location being tracked by the dir watcher, + // remove the failed lookup location path to dir Path entry + if (!watches.mapLocations.has(failedLookupLocationPath)) { + failedLookupLocationToDirPath.delete(failedLookupLocationPath); + } + // If there are no more files that need this watcher alive, close the watcher if (watches.mapLocations.size === 0) { watches.watcher.close(); directoryWatchesOfFailedLookups.delete(dirPath); @@ -311,8 +381,7 @@ namespace ts { invalidateResolutionCacheOfDeletedFile(filePath, resolvedTypeReferenceDirectives, m => m.resolvedTypeReferenceDirective, r => r.resolvedFileName); } - function onFileAddOrRemoveInDirectoryOfFailedLookup(fileOrFolder: Path) { - const dirPath = getDirectoryPath(fileOrFolder); + function onFileAddOrRemoveInDirectoryOfFailedLookup(dirPath: Path, fileOrFolder: Path) { const watches = directoryWatchesOfFailedLookups.get(dirPath); const isFailedLookupFile = watches.mapLocations.has(fileOrFolder); if (isFailedLookupFile) { @@ -324,7 +393,7 @@ namespace ts { } function onAddOrRemoveDirectoryOfFailedLookup(dirPath: Path) { - const isInDirPath: (location: string) => boolean = location => getDirectoryPath(resolutionHost.toPath(location)) === dirPath; + const isInDirPath: (location: string) => boolean = location => isInDirectoryPath(dirPath, resolutionHost.toPath(location)); invalidateResolutionCacheOfChangedFailedLookupLocation(resolvedModuleNames, isInDirPath); invalidateResolutionCacheOfChangedFailedLookupLocation(resolvedTypeReferenceDirectives, isInDirPath); } diff --git a/src/compiler/watchedProgram.ts b/src/compiler/watchedProgram.ts index 9058588ade1..07a783c3c47 100644 --- a/src/compiler/watchedProgram.ts +++ b/src/compiler/watchedProgram.ts @@ -297,7 +297,10 @@ namespace ts { }; // Cache for the module resolution const resolutionCache = createResolutionCache(compilerHost); - + resolutionCache.setRootDirectory(configFileName ? + getDirectoryPath(getNormalizedAbsolutePath(configFileName, getCurrentDirectory())) : + getCurrentDirectory() + ); // There is no extra check needed since we can just rely on the program to decide emit const builder = createBuilder(getCanonicalFileName, getFileEmitOutput, computeHash, _sourceFile => true); @@ -576,8 +579,8 @@ namespace ts { } } - function watchDirectoryOfFailedLookupLocation(directory: string, cb: DirectoryWatcherCallback) { - return watchDirectory(system, directory, cb, WatchDirectoryFlags.None, writeLog); + function watchDirectoryOfFailedLookupLocation(directory: string, cb: DirectoryWatcherCallback, flags: WatchDirectoryFlags) { + return watchDirectory(system, directory, cb, flags, writeLog); } function watchMissingFilePath(missingFilePath: Path) { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 9068afa98cf..f5cd1766c39 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -321,34 +321,6 @@ namespace ts.projectSystem { verifyDiagnostics(actual, []); } - function getPathsForTypesOrModules(base: string, rootPaths: string[], typesOrModules: string[], map: Map, rootDir?: string) { - while (1) { - forEach(rootPaths, r => { - const rp = combinePaths(base, r); - forEach(typesOrModules, tm => { - map.set(tm === "" ? rp : combinePaths(rp, tm), true); - }); - }); - const parentDir = getDirectoryPath(base); - if (base === rootDir || parentDir === base) { - break; - } - base = parentDir; - } - return map; - } - - function getNodeModulesWatchedDirectories(path: string, modules: string[], map = createMap()) { - forEach(modules, module => { - getPathsForTypesOrModules(path, ["node_modules"], ["", module, "@types", `@types/${module}`], map); - }); - return map; - } - - function getTypesWatchedDirectories(path: string, typeRoots: string[], types: string[], map = createMap()) { - return getPathsForTypesOrModules(path, typeRoots, types.concat(""), map, path); - } - describe("tsserverProjectSystem", () => { const commonFile1: FileOrFolder = { path: "/a/b/commonFile1.ts", @@ -386,8 +358,8 @@ namespace ts.projectSystem { const configFileLocations = ["/a/b/c/", "/a/b/", "/a/", "/"]; const configFiles = flatMap(configFileLocations, location => [location + "tsconfig.json", location + "jsconfig.json"]); checkWatchedFiles(host, configFiles.concat(libFile.path, moduleFile.path)); - checkWatchedDirectories(host, ["/a/b/c"], /*recursive*/ false); - checkWatchedDirectories(host, [], /*recursive*/ true); + checkWatchedDirectories(host, [], /*recursive*/ false); + checkWatchedDirectories(host, ["/"], /*recursive*/ true); }); it("can handle tsconfig file name with difference casing", () => { @@ -4040,7 +4012,7 @@ namespace ts.projectSystem { const calledMaps = getCallsTrackingMap(); return { verifyNoCall, - verifyCalledOnEachEntryOnce, + verifyCalledOnEachEntryNTimes, verifyCalledOnEachEntry, verifyNoHostCalls, verifyNoHostCallsExceptFileExistsOnce, @@ -4090,8 +4062,8 @@ namespace ts.projectSystem { }); } - function verifyCalledOnEachEntryOnce(callback: keyof CalledMaps, expectedKeys: string[]) { - return verifyCalledOnEachEntry(callback, zipToMap(expectedKeys, expectedKeys.map(() => 1))); + function verifyCalledOnEachEntryNTimes(callback: keyof CalledMaps, expectedKeys: string[], nTimes: number) { + return verifyCalledOnEachEntry(callback, zipToMap(expectedKeys, expectedKeys.map(() => nTimes))); } function verifyNoHostCalls() { @@ -4101,7 +4073,7 @@ namespace ts.projectSystem { } function verifyNoHostCallsExceptFileExistsOnce(expectedKeys: string[]) { - verifyCalledOnEachEntryOnce("fileExists", expectedKeys); + verifyCalledOnEachEntryNTimes("fileExists", expectedKeys, 1); verifyNoCall("directoryExists"); verifyNoCall("getDirectories"); verifyNoCall("readFile"); @@ -4253,17 +4225,7 @@ namespace ts.projectSystem { const { configFileName } = projectService.openClientFile(file1.path); assert.equal(configFileName, tsconfigFile.path, `should find config`); checkNumberOfConfiguredProjects(projectService, 1); - const watchedModuleDirectories = arrayFrom( - getNodeModulesWatchedDirectories( - canonicalFrontendDir, - types, - getTypesWatchedDirectories( - canonicalFrontendDir, - typeRoots, - types - ) - ).keys() - ); + const watchingRecursiveDirectories = [`${canonicalFrontendDir}/src`, canonicalFrontendDir, "/"]; const project = projectService.configuredProjects.get(canonicalConfigPath); verifyProjectAndWatchedDirectories(); @@ -4276,7 +4238,7 @@ namespace ts.projectSystem { host.runQueuedTimeoutCallbacks(); const canonicalFile3Path = useCaseSensitiveFileNames ? file3.path : file3.path.toLocaleLowerCase(); - callsTrackingHost.verifyCalledOnEachEntryOnce("fileExists", [canonicalFile3Path]); + callsTrackingHost.verifyCalledOnEachEntryNTimes("fileExists", [canonicalFile3Path], watchingRecursiveDirectories.length); // Called for type root resolution const directoryExistsCalled = createMap(); @@ -4286,11 +4248,11 @@ namespace ts.projectSystem { directoryExistsCalled.set(`/node_modules`, 2); directoryExistsCalled.set(`${frontendDir}/types`, 2); directoryExistsCalled.set(`${frontendDir}/node_modules/@types`, 2); - directoryExistsCalled.set(canonicalFile3Path, 1); + directoryExistsCalled.set(canonicalFile3Path, watchingRecursiveDirectories.length); callsTrackingHost.verifyCalledOnEachEntry("directoryExists", directoryExistsCalled); callsTrackingHost.verifyNoCall("getDirectories"); - callsTrackingHost.verifyCalledOnEachEntryOnce("readFile", [file3.path]); + callsTrackingHost.verifyCalledOnEachEntryNTimes("readFile", [file3.path], 1); callsTrackingHost.verifyNoCall("readDirectory"); checkNumberOfConfiguredProjects(projectService, 1); @@ -4316,8 +4278,8 @@ namespace ts.projectSystem { function verifyProjectAndWatchedDirectories() { checkProjectActualFiles(project, map(projectFiles, f => f.path)); checkWatchedFiles(host, mapDefined(projectFiles, getFilePathIfOpen)); - checkWatchedDirectories(host, [`${canonicalFrontendDir}/src`], /*recursive*/ true); - checkWatchedDirectories(host, watchedModuleDirectories, /*recursive*/ false); + checkWatchedDirectories(host, watchingRecursiveDirectories, /*recursive*/ true); + checkWatchedDirectories(host, [], /*recursive*/ false); } } @@ -4372,7 +4334,7 @@ namespace ts.projectSystem { const projectService = createProjectService(host); const { configFileName } = projectService.openClientFile(app.path); assert.equal(configFileName, tsconfigJson.path, `should find config`); - const watchedModuleLocations = arrayFrom(getNodeModulesWatchedDirectories(appFolder, ["lodash"]).keys()); + const recursiveWatchedDirectories: string[] = [appFolder, "/"]; verifyProject(); let timeoutAfterReloadFs = timeoutDuringPartialInstallation; @@ -4450,7 +4412,8 @@ namespace ts.projectSystem { const lodashIndexPath = "/a/b/node_modules/@types/lodash/index.d.ts"; projectFiles.push(find(filesAndFoldersToAdd, f => f.path === lodashIndexPath)); - watchedModuleLocations.length = indexOf(watchedModuleLocations, getDirectoryPath(lodashIndexPath)); + // we would now not have failed lookup in the parent of appFolder since lodash is available + recursiveWatchedDirectories.length = 1; // npm installation complete, timeout after reload fs timeoutAfterReloadFs = true; verifyAfterPartialOrCompleteNpmInstall(2); @@ -4475,8 +4438,8 @@ namespace ts.projectSystem { const filesWatched = filter(projectFilePaths, p => p !== app.path); checkWatchedFiles(host, filesWatched); - checkWatchedDirectories(host, [appFolder], /*recursive*/ true); - checkWatchedDirectories(host, watchedModuleLocations, /*recursive*/ false); + checkWatchedDirectories(host, recursiveWatchedDirectories, /*recursive*/ true); + checkWatchedDirectories(host, [], /*recursive*/ false); } } diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts index 4b249c78e15..724326d3eb6 100644 --- a/src/harness/virtualFileSystemWithWatch.ts +++ b/src/harness/virtualFileSystemWithWatch.ts @@ -141,7 +141,7 @@ namespace ts.TestFSWithWatch { } export function checkWatchedDirectories(host: TestServerHost, expectedDirectories: string[], recursive = false) { - checkMapKeys("watchedDirectories", recursive ? host.watchedDirectoriesRecursive : host.watchedDirectories, expectedDirectories); + checkMapKeys(`watchedDirectories${recursive ? " recursive" : ""}`, recursive ? host.watchedDirectoriesRecursive : host.watchedDirectories, expectedDirectories); } export function checkOutputContains(host: TestServerHost, expected: ReadonlyArray) { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 9195083bb68..e6976008c83 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -244,7 +244,6 @@ namespace ts.server { ConfigFilePath = "Config file for the program", MissingFilePath = "Missing file from program", WildcardDirectories = "Wild card directory", - TypeRoot = "Type root of the project", ClosedScriptInfo = "Closed Script info", ConfigFileForInferredRoot = "Config file for the inferred project root", FailedLookupLocation = "Directory of Failed lookup locations in module resolution" @@ -711,23 +710,6 @@ namespace ts.server { } } - /*@internal*/ - watchTypeRootDirectory(root: Path, project: ConfiguredProject) { - // TODO: This is not needed anymore with watches for failed lookup locations? - return this.watchDirectory( - this.host, - root, - fileOrFolder => { - project.getCachedPartialSystem().addOrDeleteFileOrFolder(fileOrFolder, this.toPath(fileOrFolder)); - project.updateTypes(); - this.delayUpdateProjectGraphAndInferredProjectsRefresh(project); - }, - WatchDirectoryFlags.None, - WatchType.TypeRoot, - project - ); - } - /** * This is to watch whenever files are added or removed to the wildcard directories */ @@ -1434,7 +1416,6 @@ namespace ts.server { ); if (languageServiceEnabled) { project.watchWildcards(projectOptions.wildcardDirectories); - project.watchTypeRoots(); } project.setProjectErrors(configFileErrors); @@ -1534,12 +1515,10 @@ namespace ts.server { if (this.exceededTotalSizeLimitForNonTsFiles(project.canonicalConfigFilePath, projectOptions.compilerOptions, projectOptions.files, fileNamePropertyReader)) { project.disableLanguageService(); project.stopWatchingWildCards(); - project.stopWatchingTypeRoots(); } else { project.enableLanguageService(); project.watchWildcards(projectOptions.wildcardDirectories); - project.watchTypeRoots(); } this.updateNonInferredProject(project, projectOptions.files, fileNamePropertyReader, projectOptions.compilerOptions, projectOptions.typeAcquisition, projectOptions.compileOnSave); diff --git a/src/server/project.ts b/src/server/project.ts index 1f6b762b674..8d391c312b1 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -351,12 +351,12 @@ namespace ts.server { } /*@internal*/ - watchDirectoryOfFailedLookupLocation(directory: string, cb: DirectoryWatcherCallback) { + watchDirectoryOfFailedLookupLocation(directory: string, cb: DirectoryWatcherCallback, flags: WatchDirectoryFlags) { return this.projectService.watchDirectory( this.projectService.host, directory, cb, - WatchDirectoryFlags.None, + flags, WatchType.FailedLookupLocation, this ); @@ -1012,6 +1012,13 @@ namespace ts.server { this.toggleJsInferredProject(/*isJsInferredProject*/ true); } super.addRoot(info); + // For first root set the resolution cache root dir + if (this.getRootFiles.length === 1) { + const rootDirPath = this.getProjectRootPath(); + if (rootDirPath) { + this.resolutionCache.setRootDirectory(rootDirPath); + } + } } removeRoot(info: ScriptInfo) { @@ -1064,7 +1071,6 @@ namespace ts.server { /* @internal */ configFileWatcher: FileWatcher; private directoriesWatchedForWildcards: Map | undefined; - private typeRootsWatchers: Map | undefined; readonly canonicalConfigFilePath: NormalizedPath; /* @internal */ @@ -1091,6 +1097,7 @@ namespace ts.server { super(configFileName, ProjectKind.Configured, projectService, documentRegistry, hasExplicitListOfFiles, languageServiceEnabled, compilerOptions, compileOnSaveEnabled, cachedPartialSystem); this.canonicalConfigFilePath = asNormalizedPath(projectService.toCanonicalFileName(configFileName)); this.enablePlugins(); + this.resolutionCache.setRootDirectory(getDirectoryPath(configFileName)); } /** @@ -1252,29 +1259,6 @@ namespace ts.server { } } - /*@internal*/ - watchTypeRoots() { - const newTypeRoots = arrayToSet(this.getEffectiveTypeRoots(), dir => this.projectService.toCanonicalFileName(dir)); - mutateMap( - this.typeRootsWatchers || (this.typeRootsWatchers = createMap()), - newTypeRoots, - { - // Create new watch - createNewValue: root => this.projectService.watchTypeRootDirectory(root as Path, this), - // Close existing watch thats not needed any more - onDeleteValue: closeFileWatcher - } - ); - } - - /*@internal*/ - stopWatchingTypeRoots() { - if (this.typeRootsWatchers) { - clearMap(this.typeRootsWatchers, closeFileWatcher); - this.typeRootsWatchers = undefined; - } - } - close() { super.close(); @@ -1283,7 +1267,6 @@ namespace ts.server { this.configFileWatcher = undefined; } - this.stopWatchingTypeRoots(); this.stopWatchingWildCards(); } @@ -1325,6 +1308,7 @@ namespace ts.server { public compileOnSaveEnabled: boolean, private readonly projectFilePath?: string) { super(externalProjectName, ProjectKind.External, projectService, documentRegistry, /*hasExplicitListOfFiles*/ true, languageServiceEnabled, compilerOptions, compileOnSaveEnabled, projectService.host); + this.resolutionCache.setRootDirectory(this.getProjectRootPath()); } getProjectRootPath() {