Skip to content

Commit a1572d3

Browse files
mjbvzmjbvz
authored
Try to fix typings perf issues on web (microsoft#224640)
Try to fix ata perf issues on web For microsoft#182791 With this change, we now make a single call to the package manager per root instead of per package. This simplifies the code and should be better for perf Still seeing a bunch of errors in the console but TS typing is working ok. Needs more exploration for ATA
1 parent 2de172c commit a1572d3

File tree

3 files changed

+128
-52
lines changed

3 files changed

+128
-52
lines changed

‎extensions/typescript-language-features/src/filesystems/ata.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@ export function registerAtaSupport(): vscode.Disposable {
1818
requireGlobalConfiguration('typescript', 'tsserver.web.typeAcquisition.enabled'),
1919
], () => {
2020
return vscode.Disposable.from(
21+
// Ata
2122
vscode.workspace.registerFileSystemProvider('vscode-global-typings', new MemFs(), {
2223
isCaseSensitive: true,
23-
isReadonly: false
24+
isReadonly: false,
2425
}),
26+
27+
// Read accesses to node_modules
2528
vscode.workspace.registerFileSystemProvider('vscode-node-modules', new AutoInstallerFs(), {
2629
isCaseSensitive: true,
2730
isReadonly: false

‎extensions/typescript-language-features/src/filesystems/autoInstallerFs.ts‎

Lines changed: 33 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -3,49 +3,31 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { PackageManager } from '@vscode/ts-package-manager';
7+
import { basename, join } from 'path';
68
import * as vscode from 'vscode';
7-
import { MemFs } from './memFs';
89
import { URI } from 'vscode-uri';
9-
import { PackageManager, FileSystem, packagePath } from '@vscode/ts-package-manager';
10-
import { join, basename, dirname } from 'path';
10+
import { Throttler } from '../utils/async';
11+
import { Disposable } from '../utils/dispose';
12+
import { MemFs } from './memFs';
1113

1214
const TEXT_DECODER = new TextDecoder('utf-8');
1315
const TEXT_ENCODER = new TextEncoder();
1416

15-
export class AutoInstallerFs implements vscode.FileSystemProvider {
17+
export class AutoInstallerFs extends Disposable implements vscode.FileSystemProvider {
1618

1719
private readonly memfs = new MemFs();
18-
private readonly fs: FileSystem;
19-
private readonly projectCache = new Map<string, Set<string>>();
20-
private readonly watcher: vscode.FileSystemWatcher;
21-
private readonly _emitter = new vscode.EventEmitter<vscode.FileChangeEvent[]>();
20+
private readonly packageManager: PackageManager;
21+
private readonly _projectCache = new Map</* root */ string, {
22+
readonly throttler: Throttler;
23+
}>();
2224

23-
readonly onDidChangeFile: vscode.Event<vscode.FileChangeEvent[]> = this._emitter.event;
25+
private readonly _emitter = this._register(new vscode.EventEmitter<vscode.FileChangeEvent[]>());
26+
readonly onDidChangeFile = this._emitter.event;
2427

2528
constructor() {
26-
this.watcher = vscode.workspace.createFileSystemWatcher('**/{package.json,package-lock.json,package-lock.kdl}');
27-
const handler = (uri: URI) => {
28-
const root = dirname(uri.path);
29-
if (this.projectCache.delete(root)) {
30-
(async () => {
31-
const pm = new PackageManager(this.fs);
32-
const opts = await this.getInstallOpts(uri, root);
33-
const proj = await pm.resolveProject(root, opts);
34-
proj.pruneExtraneous();
35-
// TODO: should this fire on vscode-node-modules instead?
36-
// NB(kmarchan): This should tell TSServer that there's
37-
// been changes inside node_modules and it needs to
38-
// re-evaluate things.
39-
this._emitter.fire([{
40-
type: vscode.FileChangeType.Changed,
41-
uri: uri.with({ path: join(root, 'node_modules') })
42-
}]);
43-
})();
44-
}
45-
};
46-
this.watcher.onDidChange(handler);
47-
this.watcher.onDidCreate(handler);
48-
this.watcher.onDidDelete(handler);
29+
super();
30+
4931
const memfs = this.memfs;
5032
memfs.onDidChangeFile((e) => {
5133
this._emitter.fire(e.map(ev => ({
@@ -54,7 +36,8 @@ export class AutoInstallerFs implements vscode.FileSystemProvider {
5436
uri: ev.uri.with({ scheme: 'memfs' })
5537
})));
5638
});
57-
this.fs = {
39+
40+
this.packageManager = new PackageManager({
5841
readDirectory(path: string, _extensions?: readonly string[], _exclude?: readonly string[], _include?: readonly string[], _depth?: number): string[] {
5942
return memfs.readDirectory(URI.file(path)).map(([name, _]) => name);
6043
},
@@ -87,7 +70,7 @@ export class AutoInstallerFs implements vscode.FileSystemProvider {
8770
return undefined;
8871
}
8972
}
90-
};
73+
});
9174
}
9275

9376
watch(resource: vscode.Uri): vscode.Disposable {
@@ -151,8 +134,6 @@ export class AutoInstallerFs implements vscode.FileSystemProvider {
151134
}
152135

153136
private async ensurePackageContents(incomingUri: MappedUri): Promise<void> {
154-
// console.log('ensurePackageContents', incomingUri.path);
155-
156137
// If we're not looking for something inside node_modules, bail early.
157138
if (!incomingUri.path.includes('node_modules')) {
158139
throw vscode.FileSystemError.FileNotFound();
@@ -164,25 +145,26 @@ export class AutoInstallerFs implements vscode.FileSystemProvider {
164145
}
165146

166147
const root = this.getProjectRoot(incomingUri.path);
167-
168-
const pkgPath = packagePath(incomingUri.path);
169-
if (!root || this.projectCache.get(root)?.has(pkgPath)) {
148+
if (!root) {
170149
return;
171150
}
151+
console.log('ensurePackageContents', incomingUri.path, root);
172152

173-
const proj = await (new PackageManager(this.fs)).resolveProject(root, await this.getInstallOpts(incomingUri.original, root));
174-
175-
const restore = proj.restorePackageAt(incomingUri.path);
176-
try {
177-
await restore;
178-
} catch (e) {
179-
console.error(`failed to restore package at ${incomingUri.path}: `, e);
180-
throw e;
153+
let projectEntry = this._projectCache.get(root);
154+
if (!projectEntry) {
155+
projectEntry = { throttler: new Throttler() };
156+
this._projectCache.set(root, projectEntry);
181157
}
182-
if (!this.projectCache.has(root)) {
183-
this.projectCache.set(root, new Set());
184-
}
185-
this.projectCache.get(root)!.add(pkgPath);
158+
159+
projectEntry.throttler.queue(async () => {
160+
const proj = await this.packageManager.resolveProject(root, await this.getInstallOpts(incomingUri.original, root));
161+
try {
162+
await proj.restore();
163+
} catch (e) {
164+
console.error(`failed to restore package at ${incomingUri.path}: `, e);
165+
throw e;
166+
}
167+
});
186168
}
187169

188170
private async getInstallOpts(originalUri: URI, root: string) {

‎extensions/typescript-language-features/src/utils/async.ts‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,94 @@ export function setImmediate(callback: (...args: any[]) => void, ...args: any[])
7070
return { dispose: () => clearTimeout(handle) };
7171
}
7272
}
73+
74+
75+
/**
76+
* A helper to prevent accumulation of sequential async tasks.
77+
*
78+
* Imagine a mail man with the sole task of delivering letters. As soon as
79+
* a letter submitted for delivery, he drives to the destination, delivers it
80+
* and returns to his base. Imagine that during the trip, N more letters were submitted.
81+
* When the mail man returns, he picks those N letters and delivers them all in a
82+
* single trip. Even though N+1 submissions occurred, only 2 deliveries were made.
83+
*
84+
* The throttler implements this via the queue() method, by providing it a task
85+
* factory. Following the example:
86+
*
87+
* const throttler = new Throttler();
88+
* const letters = [];
89+
*
90+
* function deliver() {
91+
* const lettersToDeliver = letters;
92+
* letters = [];
93+
* return makeTheTrip(lettersToDeliver);
94+
* }
95+
*
96+
* function onLetterReceived(l) {
97+
* letters.push(l);
98+
* throttler.queue(deliver);
99+
* }
100+
*/
101+
export class Throttler {
102+
103+
private activePromise: Promise<any> | null;
104+
private queuedPromise: Promise<any> | null;
105+
private queuedPromiseFactory: ITask<Promise<any>> | null;
106+
107+
private isDisposed = false;
108+
109+
constructor() {
110+
this.activePromise = null;
111+
this.queuedPromise = null;
112+
this.queuedPromiseFactory = null;
113+
}
114+
115+
queue<T>(promiseFactory: ITask<Promise<T>>): Promise<T> {
116+
if (this.isDisposed) {
117+
return Promise.reject(new Error('Throttler is disposed'));
118+
}
119+
120+
if (this.activePromise) {
121+
this.queuedPromiseFactory = promiseFactory;
122+
123+
if (!this.queuedPromise) {
124+
const onComplete = () => {
125+
this.queuedPromise = null;
126+
127+
if (this.isDisposed) {
128+
return;
129+
}
130+
131+
const result = this.queue(this.queuedPromiseFactory!);
132+
this.queuedPromiseFactory = null;
133+
134+
return result;
135+
};
136+
137+
this.queuedPromise = new Promise(resolve => {
138+
this.activePromise!.then(onComplete, onComplete).then(resolve);
139+
});
140+
}
141+
142+
return new Promise((resolve, reject) => {
143+
this.queuedPromise!.then(resolve, reject);
144+
});
145+
}
146+
147+
this.activePromise = promiseFactory();
148+
149+
return new Promise((resolve, reject) => {
150+
this.activePromise!.then((result: T) => {
151+
this.activePromise = null;
152+
resolve(result);
153+
}, (err: unknown) => {
154+
this.activePromise = null;
155+
reject(err);
156+
});
157+
});
158+
}
159+
160+
dispose(): void {
161+
this.isDisposed = true;
162+
}
163+
}

0 commit comments

Comments
 (0)