From fd02c6d4a1650531ae7d54e86c1c73e40c80f7af Mon Sep 17 00:00:00 2001 From: Ito Date: Fri, 20 Oct 2023 16:50:08 -0700 Subject: [PATCH 1/5] filewatcher: add watch patterns once to avoid rebuilding n times --- src/codecatalyst/devfile.ts | 2 +- src/shared/cloudformation/activation.ts | 2 +- src/shared/fs/watchedFiles.ts | 28 ++++++++++++------- src/shared/sam/activation.ts | 16 ++++++----- .../shared/debug/launchConfiguration.test.ts | 2 +- .../cloudformation/templateRegistry.test.ts | 18 ++++++++---- 6 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/codecatalyst/devfile.ts b/src/codecatalyst/devfile.ts index 68f8eb88b60..91e6da097b5 100644 --- a/src/codecatalyst/devfile.ts +++ b/src/codecatalyst/devfile.ts @@ -110,7 +110,7 @@ export class DevfileCodeLensProvider implements vscode.CodeLensProvider { export function registerDevfileWatcher(devenvClient: DevEnvClient): vscode.Disposable { const registry = new DevfileRegistry() const codelensProvider = new DevfileCodeLensProvider(registry, devenvClient) - registry.addWatchPattern(devfileGlobPattern) + registry.addWatchPatterns(devfileGlobPattern) const codelensDisposable = vscode.languages.registerCodeLensProvider( { diff --git a/src/shared/cloudformation/activation.ts b/src/shared/cloudformation/activation.ts index b1d18f56cfd..2bd09de3fee 100644 --- a/src/shared/cloudformation/activation.ts +++ b/src/shared/cloudformation/activation.ts @@ -69,7 +69,7 @@ function setTemplateRegistryInGlobals(registry: CloudFormationTemplateRegistry) const registrySetupFunc = async (registry: CloudFormationTemplateRegistry) => { await registry.addExcludedPattern(devfileExcludePattern) await registry.addExcludedPattern(templateFileExcludePattern) - await registry.addWatchPattern(templateFileGlobPattern) + await registry.addWatchPatterns(templateFileGlobPattern) await registry.watchUntitledFiles() } diff --git a/src/shared/fs/watchedFiles.ts b/src/shared/fs/watchedFiles.ts index 68c0562136b..c9ee76afa24 100644 --- a/src/shared/fs/watchedFiles.ts +++ b/src/shared/fs/watchedFiles.ts @@ -90,9 +90,12 @@ export abstract class WatchedFiles implements vscode.Disposable { } /** - * Creates a watcher across all opened workspace folders (or see below to + * Creates watchers for each glob across all opened workspace folders (or see below to * watch _outside_ the workspace). * + * Fails if the watcher is disposed, or if globs have already been set; + * enforce setting once to reduce rebuilds looping through all existing globs + * * (since vscode 1.64): * - Watches RECURSIVELY if `pattern` is complex (e.g. contains `**` or * path segments), else watches NON-RECURSIVELY (i.e. only changes at the @@ -102,8 +105,8 @@ export abstract class WatchedFiles implements vscode.Disposable { * It cannot be used to add more folders for watching, nor will it report * events outside of workspace folders. * - To watch _outside_ the workspace, pass `vscode.RelativePattern(vscode.Uri(…))`: - * - non-recursive: `addWatchPattern(new RelativePattern(Uri.file(…), '*.js'))` - * - recursive: `addWatchPattern(new RelativePattern(Uri.file(…), '**x/*.js'))` + * - non-recursive: `addWatchPatterns(new RelativePattern(Uri.file(…), '*.js'))` + * - recursive: `addWatchPatterns(new RelativePattern(Uri.file(…), '**x/*.js'))` * - **Note** recursive files may be excluded by user configuration * (`files.watcherExclude`, e.g. "node_modules"). To avoid that, watch * simple (non-recursive) patterns. @@ -115,19 +118,24 @@ export abstract class WatchedFiles implements vscode.Disposable { * > workspace, because that would result in multiple watchers on the same * > paths competing against each other. * - * @param glob Pattern to match against (across all opened workspace folders) + * @param globs Patterns to match against (across all opened workspace folders) */ - public async addWatchPattern(glob: vscode.GlobPattern): Promise { + public async addWatchPatterns(globs: vscode.GlobPattern[]): Promise { if (this._isDisposed) { throw new Error(`${this.name}: manager has already been disposed!`) } - if (typeof glob === 'string' && !vscode.workspace.workspaceFolders?.[0]) { - getLogger().warn(`${this.name}: addWatchPattern(${glob}): no workspace`) + if (this.globs.length > 0) { + throw new Error(`${this.name}: watch patterns have already been established`) } - this.globs.push(glob) + for (const glob of globs) { + if (typeof glob === 'string' && !vscode.workspace.workspaceFolders?.[0]) { + getLogger().warn(`${this.name}: addWatchPatterns(${glob}): no workspace`) + } + this.globs.push(glob) - const watcher = vscode.workspace.createFileSystemWatcher(glob) - this.addWatcher(watcher) + const watcher = vscode.workspace.createFileSystemWatcher(glob) + this.addWatcher(watcher) + } await this.rebuild() } diff --git a/src/shared/sam/activation.ts b/src/shared/sam/activation.ts index 44417b891ab..7c880ecc3f8 100644 --- a/src/shared/sam/activation.ts +++ b/src/shared/sam/activation.ts @@ -168,14 +168,16 @@ async function activateCodeLensRegistry(context: ExtContext) { // // "**/…" string patterns watch recursively across _all_ workspace - // folders (see documentation for addWatchPattern()). + // folders (see documentation for addWatchPatterns()). // - await registry.addWatchPattern(pyLensProvider.pythonBasePattern) - await registry.addWatchPattern(jsLensProvider.javascriptBasePattern) - await registry.addWatchPattern(csLensProvider.csharpBasePattern) - await registry.addWatchPattern(goLensProvider.goBasePattern) - await registry.addWatchPattern(javaLensProvider.gradleBasePattern) - await registry.addWatchPattern(javaLensProvider.mavenBasePattern) + await registry.addWatchPatterns([ + pyLensProvider.pythonBasePattern, + jsLensProvider.javascriptBasePattern, + csLensProvider.csharpBasePattern, + goLensProvider.goBasePattern, + javaLensProvider.gradleBasePattern, + javaLensProvider.mavenBasePattern, + ]) } catch (e) { vscode.window.showErrorMessage( localize( diff --git a/src/test/shared/debug/launchConfiguration.test.ts b/src/test/shared/debug/launchConfiguration.test.ts index 28f08d7a735..bf912ac75ef 100644 --- a/src/test/shared/debug/launchConfiguration.test.ts +++ b/src/test/shared/debug/launchConfiguration.test.ts @@ -105,7 +105,7 @@ describe('LaunchConfiguration', function () { const templateUriCsharp = vscode.Uri.file(path.join(workspace.uri.fsPath, 'csharp6-zip/template.yaml')) beforeEach(async function () { - await globals.templateRegistry.addWatchPattern(templateFileGlobPattern) + await globals.templateRegistry.addWatchPatterns(templateFileGlobPattern) // TODO: remove mocks in favor of testing src/testFixtures/ data. mockConfigSource = mock() diff --git a/src/testInteg/cloudformation/templateRegistry.test.ts b/src/testInteg/cloudformation/templateRegistry.test.ts index 3da0d457f87..68a8bead031 100644 --- a/src/testInteg/cloudformation/templateRegistry.test.ts +++ b/src/testInteg/cloudformation/templateRegistry.test.ts @@ -10,6 +10,7 @@ import { CloudFormationTemplateRegistry } from '../../shared/fs/templateRegistry import { makeSampleSamTemplateYaml, strToYamlFile } from '../../test/shared/cloudformation/cloudformationTestUtils' import { getTestWorkspaceFolder } from '../integrationTestsUtilities' import { sleep, waitUntil } from '../../shared/utilities/timeoutUtils' +import assert from 'assert' /** * Note: these tests are pretty shallow right now. They do not test the following: @@ -43,13 +44,13 @@ describe('CloudFormation Template Registry', async function () { await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDir, 'test.yaml')) await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDirNested, 'test.yml')) - await registry.addWatchPattern('**/test.{yaml,yml}') + await registry.addWatchPatterns(['**/test.{yaml,yml}']) await registryHasTargetNumberOfFiles(registry, 2) }) it.skip('adds dynamically-added template files with yaml and yml extensions at various nesting levels', async function () { - await registry.addWatchPattern('**/test.{yaml,yml}') + await registry.addWatchPatterns(['**/test.{yaml,yml}']) await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDir, 'test.yml')) await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDirNested, 'test.yaml')) @@ -58,7 +59,7 @@ describe('CloudFormation Template Registry', async function () { }) it('Ignores templates matching excluded patterns', async function () { - await registry.addWatchPattern('**/test.{yaml,yml}') + await registry.addWatchPatterns(['**/test.{yaml,yml}']) await registry.addExcludedPattern(/.*nested.*/) await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDir, 'test.yml')) @@ -71,7 +72,7 @@ describe('CloudFormation Template Registry', async function () { const filepath = path.join(testDir, 'changeMe.yml') await strToYamlFile(makeSampleSamTemplateYaml(false), filepath) - await registry.addWatchPattern('**/changeMe.yml') + await registry.addWatchPatterns(['**/changeMe.yml']) await registryHasTargetNumberOfFiles(registry, 1) @@ -83,7 +84,7 @@ describe('CloudFormation Template Registry', async function () { }) it('can handle deleted files', async function () { - await registry.addWatchPattern('**/deleteMe.yml') + await registry.addWatchPatterns(['**/deleteMe.yml']) // Specifically creating the file after the watcher is added // Otherwise, it seems the file is deleted before the file watcher realizes the file exists @@ -97,6 +98,13 @@ describe('CloudFormation Template Registry', async function () { await registryHasTargetNumberOfFiles(registry, 0) }) + + it('fails if you set watch patterns multiple times', async function () { + await registry.addWatchPatterns(['first/set']) + await assert.rejects(async () => { + await registry.addWatchPatterns(['second/set']) + }, 'CloudFormationTemplateRegistry: watch patterns have already been established') + }) }) async function registryHasTargetNumberOfFiles(registry: CloudFormationTemplateRegistry, target: number) { From 1e6e2af0850ddda49fe51795f9ec4d59024f5330 Mon Sep 17 00:00:00 2001 From: Ito Date: Fri, 20 Oct 2023 17:00:11 -0700 Subject: [PATCH 2/5] missed arrays --- src/codecatalyst/devfile.ts | 2 +- src/shared/cloudformation/activation.ts | 2 +- src/test/shared/debug/launchConfiguration.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/codecatalyst/devfile.ts b/src/codecatalyst/devfile.ts index 91e6da097b5..3bbaaa80c61 100644 --- a/src/codecatalyst/devfile.ts +++ b/src/codecatalyst/devfile.ts @@ -110,7 +110,7 @@ export class DevfileCodeLensProvider implements vscode.CodeLensProvider { export function registerDevfileWatcher(devenvClient: DevEnvClient): vscode.Disposable { const registry = new DevfileRegistry() const codelensProvider = new DevfileCodeLensProvider(registry, devenvClient) - registry.addWatchPatterns(devfileGlobPattern) + registry.addWatchPatterns([devfileGlobPattern]) const codelensDisposable = vscode.languages.registerCodeLensProvider( { diff --git a/src/shared/cloudformation/activation.ts b/src/shared/cloudformation/activation.ts index 2bd09de3fee..eae9fcf8366 100644 --- a/src/shared/cloudformation/activation.ts +++ b/src/shared/cloudformation/activation.ts @@ -69,7 +69,7 @@ function setTemplateRegistryInGlobals(registry: CloudFormationTemplateRegistry) const registrySetupFunc = async (registry: CloudFormationTemplateRegistry) => { await registry.addExcludedPattern(devfileExcludePattern) await registry.addExcludedPattern(templateFileExcludePattern) - await registry.addWatchPatterns(templateFileGlobPattern) + await registry.addWatchPatterns([templateFileGlobPattern]) await registry.watchUntitledFiles() } diff --git a/src/test/shared/debug/launchConfiguration.test.ts b/src/test/shared/debug/launchConfiguration.test.ts index bf912ac75ef..d328549b62f 100644 --- a/src/test/shared/debug/launchConfiguration.test.ts +++ b/src/test/shared/debug/launchConfiguration.test.ts @@ -105,7 +105,7 @@ describe('LaunchConfiguration', function () { const templateUriCsharp = vscode.Uri.file(path.join(workspace.uri.fsPath, 'csharp6-zip/template.yaml')) beforeEach(async function () { - await globals.templateRegistry.addWatchPatterns(templateFileGlobPattern) + await globals.templateRegistry.addWatchPatterns([templateFileGlobPattern]) // TODO: remove mocks in favor of testing src/testFixtures/ data. mockConfigSource = mock() From d503e9047eb42077faa8c1348eba6c8ff68d0975 Mon Sep 17 00:00:00 2001 From: Ito Date: Mon, 23 Oct 2023 08:17:23 -0700 Subject: [PATCH 3/5] use error in assert --- src/testInteg/cloudformation/templateRegistry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/testInteg/cloudformation/templateRegistry.test.ts b/src/testInteg/cloudformation/templateRegistry.test.ts index 68a8bead031..56c16aded53 100644 --- a/src/testInteg/cloudformation/templateRegistry.test.ts +++ b/src/testInteg/cloudformation/templateRegistry.test.ts @@ -103,7 +103,7 @@ describe('CloudFormation Template Registry', async function () { await registry.addWatchPatterns(['first/set']) await assert.rejects(async () => { await registry.addWatchPatterns(['second/set']) - }, 'CloudFormationTemplateRegistry: watch patterns have already been established') + }, new Error('CloudFormationTemplateRegistry: watch patterns have already been established')) }) }) From fc7f302fcd4afb8192f4156a872d3b051547f1bf Mon Sep 17 00:00:00 2001 From: Ito Date: Mon, 23 Oct 2023 08:37:32 -0700 Subject: [PATCH 4/5] changelog --- .../Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json diff --git a/.changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json b/.changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json new file mode 100644 index 00000000000..865b6ddcf55 --- /dev/null +++ b/.changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "Improved CloudFormation file watcher startup" +} \ No newline at end of file From 069298e80f1bc0d69a97be0bb05a6da4c8f79950 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Mon, 23 Oct 2023 08:45:26 -0700 Subject: [PATCH 5/5] Update .changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json --- .../Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json b/.changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json index 865b6ddcf55..ad95da24cc6 100644 --- a/.changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json +++ b/.changes/next-release/Bug Fix-b3e30500-5dbe-4b08-ae36-aaba6b020c8f.json @@ -1,4 +1,4 @@ { "type": "Bug Fix", - "description": "Improved CloudFormation file watcher startup" + "description": "Improved performance of CloudFormation file watcher startup" } \ No newline at end of file