Skip to content

Commit bc858d0

Browse files
committed
wasm: share Process between DocumentLinter-s
Allow a single WebAssembly memory space (Process) to hold multiple DocumentForVSCode objects through DocumentLinter/DocumentProcessManager. This has a few benefits: * reduced memory usage (minor) * shared caches (minor) * unlock future changes, such as sharing a configuration between multiple .js files Untested: crash tolerance in the presence of multiple documents being changed concurrenly. There are likely bugs. This commit does not affect the VS Code extension because it currently uses one DocumentProcessManager per DocumentLinter.
1 parent b2b13ff commit bc858d0

File tree

2 files changed

+115
-6
lines changed

2 files changed

+115
-6
lines changed

wasm/quick-lint-js.js

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,8 @@ class DocumentLinter {
7171
this._parserPromise = (async () => {
7272
let parser;
7373
try {
74-
let factory = await this._documentProcessManager._processFactoryPromise;
75-
// TODO(strager): Reuse processes across documents.
76-
let process = await factory.createProcessAsync();
74+
let process =
75+
await this._documentProcessManager.getOrCreateProcessAsync();
7776
parser = await process.createDocumentForVSCodeAsync();
7877
} catch (e) {
7978
if (e instanceof ProcessCrashed) {
@@ -268,9 +267,10 @@ class DocumentLinter {
268267
this._recoveryPromise = (async () => {
269268
let diags;
270269
try {
271-
// TODO(strager): Reuse processes across documents.
272-
let factory = await this._documentProcessManager._processFactoryPromise;
273-
let process = await factory.createProcessAsync();
270+
let process =
271+
await this._documentProcessManager.getOrCreateDifferentProcessAsync(
272+
this._parser.process
273+
);
274274
let parser = await process.createDocumentForVSCodeAsync();
275275

276276
// BEGIN CRITICAL SECTION (no awaiting below)
@@ -310,6 +310,47 @@ class DocumentProcessManager {
310310
constructor() {
311311
// TODO(strager): Allow developers to reload the .wasm file.
312312
this._processFactoryPromise = createProcessFactoryAsync();
313+
this._processesCreated = 0;
314+
this._processPromise = null;
315+
this._process = null;
316+
}
317+
318+
async createNewProcessAsync() {
319+
let processFactory = await this._processFactoryPromise;
320+
let process = await processFactory.createProcessAsync();
321+
this._processesCreated += 1;
322+
this._process = process;
323+
return process;
324+
}
325+
326+
async getOrCreateProcessAsync() {
327+
// BEGIN CRITICAL SECTION (no awaiting below)
328+
if (this._processPromise === null) {
329+
this._processPromise = this.createNewProcessAsync();
330+
// END CRITICAL SECTION (no awaiting above)
331+
}
332+
return await this._processPromise;
333+
}
334+
335+
async getOrCreateDifferentProcessAsync(existingProcess) {
336+
if (!(existingProcess instanceof Process)) {
337+
throw new TypeError(`Expected Process, but got ${existingProcess}`);
338+
}
339+
if (this._process !== existingProcess) {
340+
return this._process;
341+
}
342+
this._forgetCurrentProcess();
343+
return await this.getOrCreateProcessAsync();
344+
}
345+
346+
_forgetCurrentProcess() {
347+
this._process = null;
348+
this._processPromise = null;
349+
}
350+
351+
// For testing only.
352+
get numberOfProcessesEverCreated() {
353+
return this._processesCreated;
313354
}
314355
}
315356
exports.DocumentProcessManager = DocumentProcessManager;
@@ -550,6 +591,10 @@ class DocumentForVSCode {
550591
this._process._vscodeDestroyDocument(this._parser);
551592
this._parser = null;
552593
}
594+
595+
get process() {
596+
return this._process;
597+
}
553598
}
554599

555600
class DocumentForWebDemo {

wasm/test-document.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,70 @@ describe("DocumentLinter", () => {
369369
crashedProcesses.clear(); // Avoid out-of-memory errors.
370370
}
371371
}, /*timeout=*/ 60_000);
372+
373+
it("multiple documents with shared process manager lint independently", async () => {
374+
let documentProcessManager = new DocumentProcessManager();
375+
let document1 = new MockDocument("let x;let x;");
376+
let linter1 = disposeAfterTest(
377+
new DocumentLinter(document1, documentProcessManager)
378+
);
379+
let document2 = new MockDocument("let y;let y;");
380+
let linter2 = disposeAfterTest(
381+
new DocumentLinter(document2, documentProcessManager)
382+
);
383+
384+
await linter1.editorChangedVisibilityAsync();
385+
assert.deepStrictEqual(document1.getDiagnosticMessages(), [
386+
"redeclaration of variable: x",
387+
]);
388+
389+
await linter2.editorChangedVisibilityAsync();
390+
assert.deepStrictEqual(document1.getDiagnosticMessages(), [
391+
"redeclaration of variable: x",
392+
]);
393+
assert.deepStrictEqual(document2.getDiagnosticMessages(), [
394+
"redeclaration of variable: y",
395+
]);
396+
397+
document1.text = "let x;let x2;";
398+
await linter1.textChangedAsync([
399+
{
400+
range: {
401+
start: { line: 0, character: "let x;let x".length },
402+
end: { line: 0, character: "let x;let x".length },
403+
},
404+
text: "2",
405+
},
406+
]);
407+
assert.deepStrictEqual(document1.getDiagnosticMessages(), []);
408+
assert.deepStrictEqual(document2.getDiagnosticMessages(), [
409+
"redeclaration of variable: y",
410+
]);
411+
412+
document2.text = "let z;let z;";
413+
await linter2.textChangedAsync([
414+
{
415+
range: {
416+
start: { line: 0, character: "let ".length },
417+
end: { line: 0, character: "let y".length },
418+
},
419+
text: "z",
420+
},
421+
{
422+
range: {
423+
start: { line: 0, character: "let z;let ".length },
424+
end: { line: 0, character: "let z;let y".length },
425+
},
426+
text: "z",
427+
},
428+
]);
429+
assert.deepStrictEqual(document1.getDiagnosticMessages(), []);
430+
assert.deepStrictEqual(document2.getDiagnosticMessages(), [
431+
"redeclaration of variable: z",
432+
]);
433+
434+
assert.strictEqual(documentProcessManager.numberOfProcessesEverCreated, 1);
435+
});
372436
});
373437

374438
describe("ExhaustiveRNG", () => {

0 commit comments

Comments
 (0)