Skip to content

Commit 7f2873f

Browse files
kyliauatscott
authored andcommitted
perf: check diagnostics in most recently used order
This commit adds an optimization to check diagnostics for open files in most recently used order.
1 parent a2b77fa commit 7f2873f

File tree

3 files changed

+79
-8
lines changed

3 files changed

+79
-8
lines changed

server/src/session.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {readNgCompletionData, tsCompletionEntryToLspCompletionItem} from './comp
2121
import {tsDiagnosticToLspDiagnostic} from './diagnostic';
2222
import {resolveAndRunNgcc} from './ngcc';
2323
import {ServerHost} from './server_host';
24-
import {filePathToUri, isConfiguredProject, isDebugMode, lspPositionToTsPosition, lspRangeToTsPositions, tsTextSpanToLspRange, uriToFilePath} from './utils';
24+
import {filePathToUri, isConfiguredProject, isDebugMode, lspPositionToTsPosition, lspRangeToTsPositions, MruTracker, tsTextSpanToLspRange, uriToFilePath} from './utils';
2525
import {resolve, Version} from './version_provider';
2626

2727
export interface SessionOptions {
@@ -54,6 +54,7 @@ export class Session {
5454
private readonly ivy: boolean;
5555
private readonly configuredProjToExternalProj = new Map<string, string>();
5656
private readonly logToConsole: boolean;
57+
private readonly openFiles = new MruTracker();
5758
private diagnosticsTimeout: NodeJS.Timeout|null = null;
5859
private isProjectLoading = false;
5960
/**
@@ -308,12 +309,9 @@ export class Session {
308309
// not affect other files because it is local to the Component.
309310
files.push(file);
310311
} else {
311-
// Get all open files. Cast is needed because the key of the map is
312-
// actually ts.Path. See
313-
// https://github.com/microsoft/TypeScript/blob/496a1d3caa21c762daa95b1ac1b75823f8774575/src/server/editorServices.ts#L978
314-
const openFiles = toArray(this.projectService.openFiles.keys()) as ts.Path[];
315-
for (const openFile of openFiles) {
316-
const scriptInfo = this.projectService.getScriptInfoForPath(openFile);
312+
// Get all open files, most recently used first.
313+
for (const openFile of this.openFiles.getAll()) {
314+
const scriptInfo = this.projectService.getScriptInfo(openFile);
317315
if (scriptInfo) {
318316
files.push(scriptInfo.fileName);
319317
}
@@ -460,6 +458,7 @@ export class Session {
460458
if (!filePath) {
461459
return;
462460
}
461+
this.openFiles.update(filePath);
463462
// External templates (HTML files) should be tagged as ScriptKind.Unknown
464463
// so that they don't get parsed as TS files. See
465464
// https://github.com/microsoft/TypeScript/blob/b217f22e798c781f55d17da72ed099a9dee5c650/src/compiler/program.ts#L1897-L1899
@@ -521,6 +520,7 @@ export class Session {
521520
if (!filePath) {
522521
return;
523522
}
523+
this.openFiles.delete(filePath);
524524
this.projectService.closeClientFile(filePath);
525525
}
526526

@@ -557,6 +557,7 @@ export class Session {
557557
if (!filePath) {
558558
return;
559559
}
560+
this.openFiles.update(filePath);
560561
const scriptInfo = this.projectService.getScriptInfo(filePath);
561562
if (!scriptInfo) {
562563
this.error(`Failed to get script info for ${filePath}`);
@@ -582,6 +583,10 @@ export class Session {
582583
private onDidSaveTextDocument(params: lsp.DidSaveTextDocumentParams) {
583584
const {text, textDocument} = params;
584585
const filePath = uriToFilePath(textDocument.uri);
586+
if (!filePath) {
587+
return;
588+
}
589+
this.openFiles.update(filePath);
585590
const scriptInfo = this.projectService.getScriptInfo(filePath);
586591
if (!scriptInfo) {
587592
return;

server/src/tests/utils_spec.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {filePathToUri, uriToFilePath} from '../utils';
9+
import {filePathToUri, MruTracker, uriToFilePath} from '../utils';
1010

1111
describe('filePathToUri', () => {
1212
it('should return URI with File scheme', () => {
@@ -52,3 +52,40 @@ describe('uriToFilePath', () => {
5252
});
5353
}
5454
});
55+
56+
57+
describe('MruTracker', () => {
58+
it('should track new items', () => {
59+
const tracker = new MruTracker();
60+
tracker.update('a');
61+
expect(tracker.getAll()).toEqual(['a']);
62+
});
63+
64+
it('should delete existing items', () => {
65+
const tracker = new MruTracker();
66+
tracker.update('a');
67+
tracker.delete('a');
68+
expect(tracker.getAll()).toEqual([]);
69+
});
70+
71+
it('should allow deletion of item that does not exist', () => {
72+
const tracker = new MruTracker();
73+
tracker.delete('a');
74+
expect(tracker.getAll()).toEqual([]);
75+
});
76+
77+
it('should return items in most recently used order', () => {
78+
const tracker = new MruTracker();
79+
tracker.update('a');
80+
tracker.update('b');
81+
expect(tracker.getAll()).toEqual(['b', 'a']);
82+
});
83+
84+
it('should update existing item', () => {
85+
const tracker = new MruTracker();
86+
tracker.update('a');
87+
tracker.update('b');
88+
tracker.update('a');
89+
expect(tracker.getAll()).toEqual(['a', 'b']);
90+
});
91+
});

server/src/utils.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,32 @@ export function isConfiguredProject(project: ts.server.Project):
8181
project is ts.server.ConfiguredProject {
8282
return project.projectKind === ts.server.ProjectKind.Configured;
8383
}
84+
85+
/**
86+
* A class that tracks items in most recently used order.
87+
*/
88+
export class MruTracker {
89+
private readonly set = new Set<string>();
90+
91+
update(item: string) {
92+
if (this.set.has(item)) {
93+
this.set.delete(item);
94+
}
95+
this.set.add(item);
96+
}
97+
98+
delete(item: string) {
99+
this.set.delete(item);
100+
}
101+
102+
/**
103+
* Returns all items sorted by most recently used.
104+
*/
105+
getAll(): string[] {
106+
// Javascript Set maintains insertion order, see
107+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set
108+
// Since items are sorted from least recently used to most recently used,
109+
// we reverse the result.
110+
return [...this.set].reverse();
111+
}
112+
}

0 commit comments

Comments
 (0)