[release-2.4] Fixes the memory leak because of project and its corresponding script info even after project is removed (#16538)

* Dont create script snapshots for files that arent source files

* 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

* 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

* Add comment for deletion of orphan script infos in file open
This commit is contained in:
Sheetal Nandi
2017-06-14 15:59:27 -07:00
committed by Mohamed Hegazy
parent 8101dc89b2
commit f4298f10ca
5 changed files with 87 additions and 32 deletions

View File

@@ -2204,7 +2204,8 @@ 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.containingProjects.length, 0, `expect 0 containing projects for '${f.path}'`);
}

View File

@@ -565,10 +565,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);
}
}
}
}
@@ -829,10 +836,29 @@ namespace ts.server {
this.assignScriptInfoToInferredProjectIfNecessary(f, /*addToListOfOpenFiles*/ false);
}
}
// 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 (info.containingProjects.length === 0) {
// if there are not projects that include this script info - delete it
this.filenameToScriptInfo.remove(info.path);
// 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);
}
}
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);
}
}
}
@@ -1303,6 +1329,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) {
@@ -1318,15 +1352,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();
@@ -1421,6 +1453,7 @@ namespace ts.server {
for (const p of this.inferredProjects) {
p.updateGraph();
}
this.printProjects();
}
@@ -1454,6 +1487,11 @@ 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 };
}

View File

@@ -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;
}
}
}
}

View File

@@ -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;
}
}
}
}

View File

@@ -822,7 +822,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<HostFileInformation>();
@@ -857,22 +857,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);
}
@@ -889,12 +884,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;
}
}
@@ -1159,12 +1154,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);
@@ -1316,7 +1318,9 @@ namespace ts {
if (program) {
forEach(program.getSourceFiles(), f =>
documentRegistry.releaseDocument(f.fileName, program.getCompilerOptions()));
program = undefined;
}
host = undefined;
}
/// Diagnostics