From d549d936dc46c00b0ac0432295329171ad88551b Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Thu, 15 Aug 2024 17:05:37 -0700 Subject: [PATCH 1/4] add debounce and throttling to main query methods --- extensions/vscode/package-lock.json | 16 +++ extensions/vscode/package.json | 1 + extensions/vscode/src/constants.ts | 6 + extensions/vscode/src/utils/throttle.ts | 22 ++++ extensions/vscode/src/utils/webviewConduit.ts | 7 +- extensions/vscode/src/views/homeView.ts | 104 +++++++++++------- 6 files changed, 112 insertions(+), 44 deletions(-) create mode 100644 extensions/vscode/src/utils/throttle.ts diff --git a/extensions/vscode/package-lock.json b/extensions/vscode/package-lock.json index ef1100123..7bf39374a 100644 --- a/extensions/vscode/package-lock.json +++ b/extensions/vscode/package-lock.json @@ -11,6 +11,7 @@ "dependencies": { "@hypersphere/omnibus": "0.1.6", "@vscode/codicons": "^0.0.36", + "async-mutex": "^0.5.0", "axios": "^1.6.0", "debounce": "^2.1.0", "eventsource": "^2.0.2", @@ -1125,6 +1126,15 @@ "node": ">=8" } }, + "node_modules/async-mutex": { + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/async-mutex/-/async-mutex-0.5.0.tgz", + "integrity": "sha512-1A94B18jkJ3DYq284ohPxoXbfTA5HsQ7/Mf4DEhcyLx3Bz27Rh59iScbB6EPiP+B+joue6YCxcMXSbFC1tZKwA==", + "license": "MIT", + "dependencies": { + "tslib": "^2.4.0" + } + }, "node_modules/asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", @@ -4656,6 +4666,12 @@ "typescript": ">=4.2.0" } }, + "node_modules/tslib": { + "version": "2.6.3", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.6.3.tgz", + "integrity": "sha512-xNvxJEOUiWPGhUuUdQgAJPKOOJfGnIyKySOc09XkKsgdUV/3E2zvwZYdejjmRgPCgcym1juLH3226yA7sEFJKQ==", + "license": "0BSD" + }, "node_modules/ttf2eot": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/ttf2eot/-/ttf2eot-3.1.0.tgz", diff --git a/extensions/vscode/package.json b/extensions/vscode/package.json index c71357930..a6099410f 100644 --- a/extensions/vscode/package.json +++ b/extensions/vscode/package.json @@ -538,6 +538,7 @@ "dependencies": { "@hypersphere/omnibus": "0.1.6", "@vscode/codicons": "^0.0.36", + "async-mutex": "^0.5.0", "axios": "^1.6.0", "debounce": "^2.1.0", "eventsource": "^2.0.2", diff --git a/extensions/vscode/src/constants.ts b/extensions/vscode/src/constants.ts index acf6bbc6d..cc4ea93ee 100644 --- a/extensions/vscode/src/constants.ts +++ b/extensions/vscode/src/constants.ts @@ -116,3 +116,9 @@ export const enum Views { HelpAndFeedback = "posit.publisher.helpAndFeedback", Logs = "posit.publisher.logs", } + +export const DebounceDelaysMS = { + file: 1000, + refreshRPackages: 1000, + refreshPythonPackages: 1000, +}; diff --git a/extensions/vscode/src/utils/throttle.ts b/extensions/vscode/src/utils/throttle.ts new file mode 100644 index 000000000..afe309f2b --- /dev/null +++ b/extensions/vscode/src/utils/throttle.ts @@ -0,0 +1,22 @@ +// Copyright (C) 2024 by Posit Software, PBC. + +import { Mutex, MutexInterface } from "async-mutex"; + +export const throttleWithLastPending = async ( + mutex: Mutex, + fn: () => Promise, +) => { + if (mutex.isLocked()) { + // we have calls waiting, so cancel them, since we're here now and taking their place + mutex.cancel(); + } + let release: MutexInterface.Releaser | undefined; + try { + release = await mutex.acquire(); + return await fn(); + } finally { + if (release) { + release(); + } + } +}; diff --git a/extensions/vscode/src/utils/webviewConduit.ts b/extensions/vscode/src/utils/webviewConduit.ts index fe4bb4f15..e901abd11 100644 --- a/extensions/vscode/src/utils/webviewConduit.ts +++ b/extensions/vscode/src/utils/webviewConduit.ts @@ -63,14 +63,11 @@ export class WebviewConduit { }; public sendMsg = (msg: HostToWebviewMessage) => { - const e = JSON.stringify(msg); + // don't send messages if the Webview hasn't initialized yet if (!this.target) { - console.warn( - `Warning: WebviewConduit::sendMsg queueing up msg called before webview reference established with init(): ${e}`, - ); - this.pendingMsgs.push(msg); return; } + const e = JSON.stringify(msg); this.target.postMessage(e); }; } diff --git a/extensions/vscode/src/views/homeView.ts b/extensions/vscode/src/views/homeView.ts index eb285e0a1..b75a838de 100644 --- a/extensions/vscode/src/views/homeView.ts +++ b/extensions/vscode/src/views/homeView.ts @@ -20,6 +20,7 @@ import { workspace, } from "vscode"; import { isAxiosError } from "axios"; +import { Mutex } from "async-mutex"; import { Configuration, @@ -72,18 +73,17 @@ import { selectNewOrExistingConfig } from "src/multiStepInputs/selectNewOrExisti import { RPackage, RVersionConfig } from "src/api/types/packages"; import { calculateTitle } from "src/utils/titles"; import { ConfigWatcherManager, WatcherManager } from "src/watchers"; -import { Commands, Contexts, Views } from "src/constants"; +import { Commands, Contexts, DebounceDelaysMS, Views } from "src/constants"; import { showProgress } from "src/utils/progress"; import { newCredential } from "src/multiStepInputs/newCredential"; import { PublisherState } from "src/state"; +import { throttleWithLastPending } from "src/utils/throttle"; enum HomeViewInitialized { initialized = "initialized", uninitialized = "uninitialized", } -const fileEventDebounce = 200; - export class HomeViewProvider implements WebviewViewProvider, Disposable { private disposables: Disposable[] = []; @@ -141,8 +141,8 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { "activeConfigChanged", (cfg: Configuration | ConfigurationError | undefined) => { this.sendRefreshedFilesLists(); - this.onRefreshPythonPackages(); - this.onRefreshRPackages(); + this.refreshPythonPackages(); + this.refreshRPackages(); this.configWatchers?.dispose(); if (cfg && isConfigurationError(cfg)) { @@ -151,33 +151,33 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { this.configWatchers = new ConfigWatcherManager(cfg); this.configWatchers.configFile?.onDidChange( - this.sendRefreshedFilesLists, + this.debounceSendRefreshedFilesLists, this, ); this.configWatchers.pythonPackageFile?.onDidCreate( - this.onRefreshPythonPackages, + this.debounceRefreshPythonPackages, this, ); this.configWatchers.pythonPackageFile?.onDidChange( - this.onRefreshPythonPackages, + this.debounceRefreshPythonPackages, this, ); this.configWatchers.pythonPackageFile?.onDidDelete( - this.onRefreshPythonPackages, + this.debounceRefreshPythonPackages, this, ); this.configWatchers.rPackageFile?.onDidCreate( - this.onRefreshRPackages, + this.debounceRefreshRPackages, this, ); this.configWatchers.rPackageFile?.onDidChange( - this.onRefreshRPackages, + this.debounceRefreshRPackages, this, ); this.configWatchers.rPackageFile?.onDidDelete( - this.onRefreshRPackages, + this.debounceRefreshRPackages, this, ); }, @@ -201,9 +201,9 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { case WebviewToHostMessageType.SAVE_SELECTION_STATE: return await this.onSaveSelectionState(msg); case WebviewToHostMessageType.REFRESH_PYTHON_PACKAGES: - return await this.onRefreshPythonPackages(); + return await this.debounceRefreshPythonPackages(); case WebviewToHostMessageType.REFRESH_R_PACKAGES: - return await this.onRefreshRPackages(); + return await this.debounceRefreshRPackages(); case WebviewToHostMessageType.VSCODE_OPEN_RELATIVE: return await this.onRelativeOpenVSCode(msg); case WebviewToHostMessageType.SCAN_PYTHON_PACKAGE_REQUIREMENTS: @@ -213,7 +213,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { case WebviewToHostMessageType.VSCODE_OPEN: return await this.onVSCodeOpen(msg); case WebviewToHostMessageType.REQUEST_FILES_LISTS: - return this.sendRefreshedFilesLists(); + return this.debounceSendRefreshedFilesLists(); case WebviewToHostMessageType.INCLUDE_FILE: return this.updateFileList(msg.content.path, FileAction.INCLUDE); case WebviewToHostMessageType.EXCLUDE_FILE: @@ -474,7 +474,12 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { ); } - private async onRefreshPythonPackages() { + public debounceRefreshPythonPackages = debounce( + this.refreshPythonPackages, + DebounceDelaysMS.refreshPythonPackages, + ); + + private async refreshPythonPackages() { const activeConfiguration = await this.state.getSelectedConfiguration(); let pythonProject = true; let packages: string[] = []; @@ -516,7 +521,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { pythonProject = false; } else { const summary = getSummaryStringFromError( - "homeView::onRefreshPythonPackages", + "homeView::refreshPythonPackages", error, ); window.showInformationMessage(summary); @@ -536,7 +541,12 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { }); } - private async onRefreshRPackages() { + public debounceRefreshRPackages = debounce( + this.refreshRPackages, + DebounceDelaysMS.refreshRPackages, + ); + + private async refreshRPackages() { const activeConfiguration = await this.state.getSelectedConfiguration(); let rProject = true; let packages: RPackage[] = []; @@ -579,7 +589,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { rProject = false; } else { const summary = getSummaryStringFromError( - "homeView::onRefreshRPackages", + "homeView::refreshRPackages", error, ); window.showInformationMessage(summary); @@ -1249,21 +1259,33 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { } }; + private refreshContentRecordsMutex = new Mutex(); public refreshContentRecords = async () => { - await this.refreshContentRecordData(); - this.updateWebViewViewContentRecords(); - useBus().trigger( - "activeContentRecordChanged", - await this.state.getSelectedContentRecord(), + return throttleWithLastPending( + this.refreshContentRecordsMutex, + async () => { + await this.refreshContentRecordData(); + this.updateWebViewViewContentRecords(); + useBus().trigger( + "activeContentRecordChanged", + await this.state.getSelectedContentRecord(), + ); + }, ); }; + private refreshConfigurationsMutex = new Mutex(); public refreshConfigurations = async () => { - await this.refreshConfigurationData(); - this.updateWebViewViewConfigurations(); - useBus().trigger( - "activeConfigChanged", - await this.state.getSelectedConfiguration(), + return throttleWithLastPending( + this.refreshConfigurationsMutex, + async () => { + await this.refreshConfigurationData(); + this.updateWebViewViewConfigurations(); + useBus().trigger( + "activeConfigChanged", + await this.state.getSelectedConfiguration(), + ); + }, ); }; @@ -1297,6 +1319,10 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { } }; + public debounceSendRefreshedFilesLists = debounce(async () => { + return await this.sendRefreshedFilesLists(); + }, DebounceDelaysMS.file); + public initiatePublish(target: PublishProcessParams) { this.initiateDeployment( target.deploymentName, @@ -1608,7 +1634,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { ), commands.registerCommand( Commands.PythonPackages.Refresh, - this.onRefreshPythonPackages, + this.refreshPythonPackages, this, ), commands.registerCommand( @@ -1637,7 +1663,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { ), commands.registerCommand( Commands.RPackages.Refresh, - this.onRefreshRPackages, + this.refreshRPackages, this, ), commands.registerCommand( @@ -1647,6 +1673,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { ), ); + // directories watchers.positDir?.onDidDelete(() => { this.refreshContentRecords(); this.refreshConfigurations(); @@ -1657,19 +1684,18 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { }, this); watchers.contentRecordsDir?.onDidDelete(this.refreshContentRecords, this); + // configurations watchers.configurations?.onDidCreate(this.refreshConfigurations, this); - watchers.configurations?.onDidDelete(this.refreshConfigurations, this); watchers.configurations?.onDidChange(this.refreshConfigurations, this); + watchers.configurations?.onDidDelete(this.refreshConfigurations, this); + // content records watchers.contentRecords?.onDidCreate(this.refreshContentRecords, this); - watchers.contentRecords?.onDidDelete(this.refreshContentRecords, this); watchers.contentRecords?.onDidChange(this.refreshContentRecords, this); + watchers.contentRecords?.onDidDelete(this.refreshContentRecords, this); - const fileEventCallback = debounce( - this.sendRefreshedFilesLists, - fileEventDebounce, - ); - watchers.allFiles?.onDidCreate(fileEventCallback, this); - watchers.allFiles?.onDidDelete(fileEventCallback, this); + // all files + watchers.allFiles?.onDidCreate(this.debounceSendRefreshedFilesLists, this); + watchers.allFiles?.onDidDelete(this.debounceSendRefreshedFilesLists, this); } } From 3f3e083c14a568b879075d705b8ad2cdac996ed7 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Thu, 15 Aug 2024 17:35:03 -0700 Subject: [PATCH 2/4] Add explanation comment. --- extensions/vscode/src/utils/throttle.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/extensions/vscode/src/utils/throttle.ts b/extensions/vscode/src/utils/throttle.ts index afe309f2b..24aa424ba 100644 --- a/extensions/vscode/src/utils/throttle.ts +++ b/extensions/vscode/src/utils/throttle.ts @@ -2,6 +2,23 @@ import { Mutex, MutexInterface } from "async-mutex"; +// This method allows throttling of a method (typically an api call), where only +// one call is active, and the last one which is requested while an existing call +// is still active, is then executed after the current call completes. +// +// We do not need any intermediate calls to be executed, because we're using this functionality +// with pure functions, which we simply need to execute the latest request. +// +// For example if all of these come in sequence very quickly, and the API call is long: +// call() // A +// call() // B +// call() // C +// call() // D +// Call A will be executed and if it takes a long time, only Call D will be executed. Under the +// coves, When Call B comes in, it is queued up using the mutex, but then Call C cancels Call B +// (using mutex.cancel()), and Call C waits on the mutex, then Call D cancels Call C and waits on +// the mutex, which is then executed when Call A releases the mutex. +// export const throttleWithLastPending = async ( mutex: Mutex, fn: () => Promise, From e6c218aaf46a16111115aa31b9ce47f63e179d65 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 21 Aug 2024 08:10:04 -0700 Subject: [PATCH 3/4] Update extensions/vscode/src/utils/throttle.ts Co-authored-by: Jordan Jensen --- extensions/vscode/src/utils/throttle.ts | 37 +++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/extensions/vscode/src/utils/throttle.ts b/extensions/vscode/src/utils/throttle.ts index 24aa424ba..a7f61821c 100644 --- a/extensions/vscode/src/utils/throttle.ts +++ b/extensions/vscode/src/utils/throttle.ts @@ -2,23 +2,26 @@ import { Mutex, MutexInterface } from "async-mutex"; -// This method allows throttling of a method (typically an api call), where only -// one call is active, and the last one which is requested while an existing call -// is still active, is then executed after the current call completes. -// -// We do not need any intermediate calls to be executed, because we're using this functionality -// with pure functions, which we simply need to execute the latest request. -// -// For example if all of these come in sequence very quickly, and the API call is long: -// call() // A -// call() // B -// call() // C -// call() // D -// Call A will be executed and if it takes a long time, only Call D will be executed. Under the -// coves, When Call B comes in, it is queued up using the mutex, but then Call C cancels Call B -// (using mutex.cancel()), and Call C waits on the mutex, then Call D cancels Call C and waits on -// the mutex, which is then executed when Call A releases the mutex. -// +/** + * This method allows throttling of a method (typically an API call), where only + * one call is active, and the last one which is requested while an existing call + * is still active, is then executed after the current call completes. + * + * We do not need any intermediate calls to be executed, because we're using this functionality + * with pure functions, which we simply need to execute the latest request. + * + * For example, if all of these come in sequence very quickly, and the API call is long: + * + * call() // A + * call() // B + * call() // C + * call() // D + * + * Call A will be executed and if it takes a long time, only Call D will be executed. Under the + * covers, when Call B comes in, it is queued up using the mutex, but then Call C cancels Call B + * (using mutex.cancel()), and Call C waits on the mutex, then Call D cancels Call C and waits on + * the mutex, which is then executed when Call A releases the mutex. + */ export const throttleWithLastPending = async ( mutex: Mutex, fn: () => Promise, From 542d1776fb60b80ded657afc2a708aa47effa559 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 21 Aug 2024 08:29:40 -0700 Subject: [PATCH 4/4] unneeded async --- extensions/vscode/src/views/homeView.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/vscode/src/views/homeView.ts b/extensions/vscode/src/views/homeView.ts index 27ae8161b..fc7b4e5da 100644 --- a/extensions/vscode/src/views/homeView.ts +++ b/extensions/vscode/src/views/homeView.ts @@ -1262,7 +1262,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { }; private refreshContentRecordsMutex = new Mutex(); - public refreshContentRecords = async () => { + public refreshContentRecords = () => { return throttleWithLastPending( this.refreshContentRecordsMutex, async () => { @@ -1277,7 +1277,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { }; private refreshConfigurationsMutex = new Mutex(); - public refreshConfigurations = async () => { + public refreshConfigurations = () => { return throttleWithLastPending( this.refreshConfigurationsMutex, async () => {