From f15c1f91a59db9ffb22990da3bdef2ddb49f7bdd Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 9 Sep 2024 12:44:10 -0400 Subject: [PATCH] Use a task queue to ensure code blocks are executed sequentially (#510) * Only `await` on the last code block This ensures that all of the blocks are sent over in a single sequential batch, with no way for anyone else to execute their own block between two of ours, which would cause ordering issues * Introduce a task queue for running `blocks` * Abstract out a `callback` * Move the queue to its own file * Simplify API and add ability to have multiple queues * `await` calls to `run()` * Switch to more official PQueue Lmao at the fact that I implemented this from scratch * Rename to `ExecuteQueue` * Tweak docs * Delete `queue.ts` --- apps/vscode/package.json | 1 + apps/vscode/src/host/execute-queue.ts | 69 +++++++++++++++++++++++++++ apps/vscode/src/host/hooks.ts | 28 +++++++---- yarn.lock | 18 +++++++ 4 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 apps/vscode/src/host/execute-queue.ts diff --git a/apps/vscode/package.json b/apps/vscode/package.json index 9190b8c5..f66ff46a 100644 --- a/apps/vscode/package.json +++ b/apps/vscode/package.json @@ -1356,6 +1356,7 @@ "markdown-it": "^13.0.1", "markdown-it-highlightjs": "^4.0.1", "nanoid": "^4.0.0", + "p-queue": "^8.0.1", "picomatch": "^2.3.1", "quarto-core": "*", "quarto-lsp": "*", diff --git a/apps/vscode/src/host/execute-queue.ts b/apps/vscode/src/host/execute-queue.ts new file mode 100644 index 00000000..bb76ab5a --- /dev/null +++ b/apps/vscode/src/host/execute-queue.ts @@ -0,0 +1,69 @@ +/* + * execute-queue.ts + * + * Copyright (C) 2024 by Posit Software, PBC + * + * Unless you have received this program directly from Posit Software pursuant + * to the terms of a commercial license agreement with Posit Software, then + * this program is licensed to you under the terms of version 3 of the + * GNU Affero General Public License. This program is distributed WITHOUT + * ANY EXPRESS OR IMPLIED WARRANTY, INCLUDING THOSE OF NON-INFRINGEMENT, + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. Please refer to the + * AGPL (http://www.gnu.org/licenses/agpl-3.0.txt) for more details. + * + */ + +import PQueue from 'p-queue'; + +/** + * Language specific execution queue + * + * A singleton class that constructs and manages multiple execution queues, identified by + * their `key` (typically the language name). + */ +export class ExecuteQueue { + /// Singleton instance + private static _instance: ExecuteQueue; + + /// Maps a `key` to its `queue` + private _queues = new Map(); + + /** + * Constructor + * + * Private since we only want one of these. Access using `instance()` instead. + */ + private constructor() { } + + /** + * Accessor for the singleton instance + * + * Creates it if it doesn't exist. + */ + static get instance(): ExecuteQueue { + if (!ExecuteQueue._instance) { + // Initialize if we've never accessed it + ExecuteQueue._instance = new ExecuteQueue(); + } + + return ExecuteQueue._instance; + } + + /** + * Add a `callback` for execution on `key`'s task queue + * + * Returns a promise that resolves when the task finishes + */ + async add(key: string, callback: () => Promise): Promise { + let queue = this._queues.get(key); + + if (queue === undefined) { + // If we've never initialized this key's queue, do so now. + // Limit `concurrency` to 1, because we don't want tasks run out of order. + queue = new PQueue({ concurrency: 1 }); + this._queues.set(key, queue); + } + + return queue.add(callback); + } +} diff --git a/apps/vscode/src/host/hooks.ts b/apps/vscode/src/host/hooks.ts index 6750cc82..91aa8946 100644 --- a/apps/vscode/src/host/hooks.ts +++ b/apps/vscode/src/host/hooks.ts @@ -18,6 +18,7 @@ import * as hooks from 'positron'; import { ExtensionHost, HostWebviewPanel, HostStatementRangeProvider } from '.'; import { CellExecutor, cellExecutorForLanguage, executableLanguages, isKnitrDocument, pythonWithReticulate } from './executors'; +import { ExecuteQueue } from './execute-queue'; import { MarkdownEngine } from '../markdown/engine'; import { virtualDoc, virtualDocUri, adjustedPosition } from "../vdoc/vdoc"; @@ -56,14 +57,26 @@ export function hooksExtensionHost() : ExtensionHost { case "r": return { execute: async (blocks: string[], _editorUri?: vscode.Uri) : Promise => { - for (const block of blocks) { - let code = block; - if (language === "python" && isKnitrDocument(document, engine)) { - language = "r"; - code = pythonWithReticulate(block); + const runtime = hooksApi()?.runtime; + + if (runtime === undefined) { + // Can't do anything without a runtime + return; + } + + if (language === "python" && isKnitrDocument(document, engine)) { + language = "r"; + blocks = blocks.map(pythonWithReticulate); + } + + // Our callback executes each block sequentially + const callback = async () => { + for (const block of blocks) { + await runtime.executeCode(language, block, false); } - await hooksApi()?.runtime.executeCode(language, code, false); - } + } + + await ExecuteQueue.instance.add(language, callback); }, executeSelection: async () : Promise => { await vscode.commands.executeCommand('workbench.action.positronConsole.executeCode', {languageId: language}); @@ -166,4 +179,3 @@ async function getStatementRange( position ); } - diff --git a/yarn.lock b/yarn.lock index 1661c7d7..e84b8253 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4306,6 +4306,11 @@ eventemitter3@^4.0.0: resolved "https://registry.yarnpkg.com/eventemitter3/-/eventemitter3-4.0.7.tgz#2de9b68f6528d5644ef5c59526a1b4a07306169f" integrity sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw== +eventemitter3@^5.0.1: + version "5.0.1" + resolved "https://registry.yarnpkg.com/eventemitter3/-/eventemitter3-5.0.1.tgz#53f5ffd0a492ac800721bb42c66b841de96423c4" + integrity sha512-GWkBvjiSZK87ELrYOSESUYeVIc9mvLLf/nXalMOS5dYrgZq9o5OVkbZAVM06CVxYsCwH9BDZFPlQTlPA1j4ahA== + exceljs@^4.3.0: version "4.3.0" resolved "https://registry.yarnpkg.com/exceljs/-/exceljs-4.3.0.tgz#939bc0d4c59c200acadb7051be34d25c109853c4" @@ -6342,6 +6347,19 @@ p-locate@^5.0.0: dependencies: p-limit "^3.0.2" +p-queue@^8.0.1: + version "8.0.1" + resolved "https://registry.yarnpkg.com/p-queue/-/p-queue-8.0.1.tgz#718b7f83836922ef213ddec263ff4223ce70bef8" + integrity sha512-NXzu9aQJTAzbBqOt2hwsR63ea7yvxJc0PwN/zobNAudYfb1B7R08SzB4TsLeSbUCuG467NhnoT0oO6w1qRO+BA== + dependencies: + eventemitter3 "^5.0.1" + p-timeout "^6.1.2" + +p-timeout@^6.1.2: + version "6.1.2" + resolved "https://registry.yarnpkg.com/p-timeout/-/p-timeout-6.1.2.tgz#22b8d8a78abf5e103030211c5fc6dee1166a6aa5" + integrity sha512-UbD77BuZ9Bc9aABo74gfXhNvzC9Tx7SxtHSh1fxvx3jTLLYvmVhiQZZrJzqqU0jKbN32kb5VOKiLEQI/3bIjgQ== + pako@~1.0.2: version "1.0.11" resolved "https://registry.yarnpkg.com/pako/-/pako-1.0.11.tgz#6c9599d340d54dfd3946380252a35705a6b992bf"