Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

watchedfiles: add watch patterns once to avoid rebuilding n times #3931

Merged
merged 5 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Improved performance of CloudFormation file watcher startup"
}
2 changes: 1 addition & 1 deletion src/codecatalyst/devfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand Down
2 changes: 1 addition & 1 deletion src/shared/cloudformation/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
28 changes: 18 additions & 10 deletions src/shared/fs/watchedFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,12 @@ export abstract class WatchedFiles<T> 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
Expand All @@ -102,8 +105,8 @@ export abstract class WatchedFiles<T> 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.
Expand All @@ -115,19 +118,24 @@ export abstract class WatchedFiles<T> 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<void> {
public async addWatchPatterns(globs: vscode.GlobPattern[]): Promise<void> {
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()
}
Expand Down
16 changes: 9 additions & 7 deletions src/shared/sam/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/test/shared/debug/launchConfiguration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
18 changes: 13 additions & 5 deletions src/testInteg/cloudformation/templateRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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'))
Expand All @@ -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'))
Expand All @@ -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)

Expand All @@ -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
Expand All @@ -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'])
}, new Error('CloudFormationTemplateRegistry: watch patterns have already been established'))
})
})

async function registryHasTargetNumberOfFiles(registry: CloudFormationTemplateRegistry, target: number) {
Expand Down