From 2ec92b9c02ba6ac6b97d3fb5af830d0ece6dcd92 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 9 Jun 2017 18:30:17 -0700 Subject: [PATCH 1/4] Dont create script snapshots for files that arent source files --- src/services/services.ts | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 21c756a9acc..9e17e9a45e1 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -815,7 +815,7 @@ namespace ts { private _compilationSettings: CompilerOptions; private currentDirectory: string; - constructor(private host: LanguageServiceHost, private getCanonicalFileName: (fileName: string) => string) { + constructor(private host: LanguageServiceHost, getCanonicalFileName: (fileName: string) => string) { // script id => script index this.currentDirectory = host.getCurrentDirectory(); this.fileNameToEntry = createFileMap(); @@ -850,22 +850,17 @@ namespace ts { return entry; } - private getEntry(path: Path): HostFileInformation { + public getEntryByPath(path: Path): HostFileInformation { return this.fileNameToEntry.get(path); } - private contains(path: Path): boolean { + public containsEntryByPath(path: Path): boolean { return this.fileNameToEntry.contains(path); } - public getOrCreateEntry(fileName: string): HostFileInformation { - const path = toPath(fileName, this.currentDirectory, this.getCanonicalFileName); - return this.getOrCreateEntryByPath(fileName, path); - } - public getOrCreateEntryByPath(fileName: string, path: Path): HostFileInformation { - return this.contains(path) - ? this.getEntry(path) + return this.containsEntryByPath(path) + ? this.getEntryByPath(path) : this.createEntry(fileName, path); } @@ -882,12 +877,12 @@ namespace ts { } public getVersion(path: Path): string { - const file = this.getEntry(path); + const file = this.getEntryByPath(path); return file && file.version; } public getScriptSnapshot(path: Path): IScriptSnapshot { - const file = this.getEntry(path); + const file = this.getEntryByPath(path); return file && file.scriptSnapshot; } } @@ -1152,12 +1147,19 @@ namespace ts { getCurrentDirectory: () => currentDirectory, fileExists: (fileName): boolean => { // stub missing host functionality - return hostCache.getOrCreateEntry(fileName) !== undefined; + const path = toPath(fileName, currentDirectory, getCanonicalFileName); + return hostCache.containsEntryByPath(path) ? + !!hostCache.getEntryByPath(path) : + (host.fileExists && host.fileExists(fileName)); }, readFile: (fileName): string => { // stub missing host functionality - const entry = hostCache.getOrCreateEntry(fileName); - return entry && entry.scriptSnapshot.getText(0, entry.scriptSnapshot.getLength()); + const path = toPath(fileName, currentDirectory, getCanonicalFileName); + if (hostCache.containsEntryByPath(path)) { + const entry = hostCache.getEntryByPath(path); + return entry && entry.scriptSnapshot.getText(0, entry.scriptSnapshot.getLength()); + } + return host.readFile && host.readFile(fileName); }, directoryExists: directoryName => { return directoryProbablyExists(directoryName, host); From 1bf1209f7ea045602507cc3f5915c99142a6ebff Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 12 Jun 2017 16:31:49 -0700 Subject: [PATCH 2/4] Cleanup script infos that are not part of any project when the project is closed or inferred projects are refreshed Also dispose some pointers so that the closures get disposed with project and script infos --- src/harness/unittests/tsserverProjectSystem.ts | 3 ++- src/server/editorServices.ts | 18 +++++++++++++++--- src/server/lsHost.ts | 11 ++++++++--- src/server/project.ts | 11 +++++++++-- src/services/services.ts | 2 ++ 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 7a19aa9167f..37bc454d614 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -2205,8 +2205,9 @@ namespace ts.projectSystem { projectService.checkNumberOfProjects({}); for (const f of [f2, f3]) { + // There shouldnt be any script info as we closed the file that resulted in creation of it const scriptInfo = projectService.getScriptInfoForNormalizedPath(server.toNormalizedPath(f.path)); - assert.equal(scriptInfo.containingProjects.length, 0, `expect 0 containing projects for '${f.path}'`); + assert.equal(scriptInfo, undefined, `expected script info to be closed: '${f.path}'`); } }); diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 7c0e0a0fb92..ad237bc0337 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -827,10 +827,20 @@ namespace ts.server { this.assignScriptInfoToInferredProjectIfNecessary(f, /*addToListOfOpenFiles*/ false); } } + + // Cleanup script infos that are not open and not part of any project + this.deleteOrphanScriptInfoNotInAnyProject(); } - if (info.containingProjects.length === 0) { - // if there are not projects that include this script info - delete it - this.filenameToScriptInfo.remove(info.path); + } + + private deleteOrphanScriptInfoNotInAnyProject() { + for (const path of this.filenameToScriptInfo.getKeys()) { + const info = this.filenameToScriptInfo.get(path); + if (!info.isScriptOpen() && info.containingProjects.length === 0) { + // if there are not projects that include this script info - delete it + info.stopWatcher(); + this.filenameToScriptInfo.remove(info.path); + } } } @@ -1418,6 +1428,8 @@ namespace ts.server { for (const p of this.inferredProjects) { p.updateGraph(); } + + this.deleteOrphanScriptInfoNotInAnyProject(); this.printProjects(); } diff --git a/src/server/lsHost.ts b/src/server/lsHost.ts index ec655c545ea..79429737240 100644 --- a/src/server/lsHost.ts +++ b/src/server/lsHost.ts @@ -11,11 +11,11 @@ namespace ts.server { private filesWithChangedSetOfUnresolvedImports: Path[]; - private readonly resolveModuleName: typeof resolveModuleName; + private resolveModuleName: typeof resolveModuleName; readonly trace: (s: string) => void; readonly realpath?: (path: string) => string; - constructor(private readonly host: ServerHost, private readonly project: Project, private readonly cancellationToken: HostCancellationToken) { + constructor(private readonly host: ServerHost, private project: Project, private readonly cancellationToken: HostCancellationToken) { this.cancellationToken = new ThrottledCancellationToken(cancellationToken, project.projectService.throttleWaitMilliseconds); this.getCanonicalFileName = ts.createGetCanonicalFileName(this.host.useCaseSensitiveFileNames); @@ -47,6 +47,11 @@ namespace ts.server { } } + dispose() { + this.project = undefined; + this.resolveModuleName = undefined; + } + public startRecordingFilesWithChangedResolutions() { this.filesWithChangedSetOfUnresolvedImports = []; } @@ -238,4 +243,4 @@ namespace ts.server { this.compilationSettings = opt; } } -} \ No newline at end of file +} diff --git a/src/server/project.ts b/src/server/project.ts index be854f948ea..33b345af833 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -116,7 +116,7 @@ namespace ts.server { public languageServiceEnabled = true; - protected readonly lsHost: LSHost; + protected lsHost: LSHost; builder: Builder; /** @@ -297,9 +297,15 @@ namespace ts.server { this.rootFiles = undefined; this.rootFilesMap = undefined; this.program = undefined; + this.builder = undefined; + this.cachedUnresolvedImportsPerFile = undefined; + this.projectErrors = undefined; + this.lsHost.dispose(); + this.lsHost = undefined; // signal language service to release source files acquired from document registry this.languageService.dispose(); + this.languageService = undefined; } getCompilerOptions() { @@ -1043,6 +1049,7 @@ namespace ts.server { if (this.projectFileWatcher) { this.projectFileWatcher.close(); + this.projectFileWatcher = undefined; } if (this.typeRootsWatchers) { @@ -1132,4 +1139,4 @@ namespace ts.server { this.typeAcquisition = newTypeAcquisition; } } -} \ No newline at end of file +} diff --git a/src/services/services.ts b/src/services/services.ts index 9e17e9a45e1..7adce648a62 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1311,7 +1311,9 @@ namespace ts { if (program) { forEach(program.getSourceFiles(), f => documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions())); + program = undefined; } + host = undefined; } /// Diagnostics From 98cb0ce815f76475c37ff0f97b61f052cf4118ee Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 14 Jun 2017 11:37:31 -0700 Subject: [PATCH 3/4] Move the cleanup of script infos to next file open This helps in reusing script infos even if the project is closed but next open recreates the same project --- .../unittests/tsserverProjectSystem.ts | 4 +- src/server/editorServices.ts | 44 ++++++++++++++----- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 37bc454d614..60309723527 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -2204,10 +2204,10 @@ namespace ts.projectSystem { projectService.closeClientFile(f1.path); projectService.checkNumberOfProjects({}); - for (const f of [f2, f3]) { + for (const f of [f1, f2, f3]) { // There shouldnt be any script info as we closed the file that resulted in creation of it const scriptInfo = projectService.getScriptInfoForNormalizedPath(server.toNormalizedPath(f.path)); - assert.equal(scriptInfo, undefined, `expected script info to be closed: '${f.path}'`); + assert.equal(scriptInfo.containingProjects.length, 0, `expect 0 containing projects for '${f.path}'`); } }); diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index ad237bc0337..5f4561a8b1e 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -563,10 +563,17 @@ namespace ts.server { } else { if (info && (!info.isScriptOpen())) { - // file has been changed which might affect the set of referenced files in projects that include - // this file and set of inferred projects - info.reloadFromFile(); - this.updateProjectGraphs(info.containingProjects); + if (info.containingProjects.length === 0) { + // Orphan script info, remove it as we can always reload it on next open + info.stopWatcher(); + this.filenameToScriptInfo.remove(info.path); + } + else { + // file has been changed which might affect the set of referenced files in projects that include + // this file and set of inferred projects + info.reloadFromFile(); + this.updateProjectGraphs(info.containingProjects); + } } } } @@ -828,8 +835,17 @@ namespace ts.server { } } - // Cleanup script infos that are not open and not part of any project - this.deleteOrphanScriptInfoNotInAnyProject(); + // Cleanup script infos that arent part of any project is postponed to + // next file open so that if file from same project is opened we wont end up creating same script infos + } + + // If the current info is being just closed - add the watcher file to track changes + // But if file was deleted, handle that part + if (this.host.fileExists(info.fileName)) { + this.watchClosedScriptInfo(info); + } + else { + this.handleDeletedFile(info); } } @@ -1310,6 +1326,14 @@ namespace ts.server { return this.getScriptInfoForNormalizedPath(toNormalizedPath(uncheckedFileName)); } + watchClosedScriptInfo(info: ScriptInfo) { + // do not watch files with mixed content - server doesn't know how to interpret it + if (!info.hasMixedContent) { + const { fileName } = info; + info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName))); + } + } + getOrCreateScriptInfoForNormalizedPath(fileName: NormalizedPath, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean) { let info = this.getScriptInfoForNormalizedPath(fileName); if (!info) { @@ -1325,15 +1349,13 @@ namespace ts.server { } } else { - // do not watch files with mixed content - server doesn't know how to interpret it - if (!hasMixedContent) { - info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName))); - } + this.watchClosedScriptInfo(info); } } } if (info) { if (openedByClient && !info.isScriptOpen()) { + info.stopWatcher(); info.open(fileContent); if (hasMixedContent) { info.registerFileUpdate(); @@ -1429,7 +1451,6 @@ namespace ts.server { p.updateGraph(); } - this.deleteOrphanScriptInfoNotInAnyProject(); this.printProjects(); } @@ -1463,6 +1484,7 @@ namespace ts.server { // at this point if file is the part of some configured/external project then this project should be created const info = this.getOrCreateScriptInfoForNormalizedPath(fileName, /*openedByClient*/ true, fileContent, scriptKind, hasMixedContent); this.assignScriptInfoToInferredProjectIfNecessary(info, /*addToListOfOpenFiles*/ true); + this.deleteOrphanScriptInfoNotInAnyProject(); this.printProjects(); return { configFileName, configFileErrors }; } From 428bc68baa3ca6d2027edb588a95a2d4c1bc6ed7 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 14 Jun 2017 15:02:24 -0700 Subject: [PATCH 4/4] Add comment for deletion of orphan script infos in file open --- src/server/editorServices.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 5f4561a8b1e..f2536b8dcb4 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1484,6 +1484,10 @@ namespace ts.server { // at this point if file is the part of some configured/external project then this project should be created const info = this.getOrCreateScriptInfoForNormalizedPath(fileName, /*openedByClient*/ true, fileContent, scriptKind, hasMixedContent); this.assignScriptInfoToInferredProjectIfNecessary(info, /*addToListOfOpenFiles*/ true); + // Delete the orphan files here because there might be orphan script infos (which are not part of project) + // when some file/s were closed which resulted in project removal. + // It was then postponed to cleanup these script infos so that they can be reused if + // the file from that old project is reopened because of opening file from here. this.deleteOrphanScriptInfoNotInAnyProject(); this.printProjects(); return { configFileName, configFileErrors };