Skip to content

Fix an issue with updateReadonly perf#310262

Open
pwang347 wants to merge 2 commits intomainfrom
pawang/updateReadonlyPerf
Open

Fix an issue with updateReadonly perf#310262
pwang347 wants to merge 2 commits intomainfrom
pawang/updateReadonlyPerf

Conversation

@pwang347
Copy link
Copy Markdown
Member

@pwang347 pwang347 commented Apr 15, 2026

Fixes #280253
If there's a large amount of customizations, we don't want to fire readonly change events in rapid succession. Let's debounce / group these.

Copilot AI review requested due to automatic review settings April 15, 2026 22:06
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2026

Screenshot Changes

Base: 34754a86 Current: 8e99269e

Changed (2)

chat/aiCustomizations/aiCustomizationManagementEditor/McpBrowseMode/Light
Before After
before after
editor/inlineCompletions/other/JumpToHint/Dark
Before After
before after

blocks-ci screenshots changed

Replace the contents of test/componentFixtures/blocks-ci-screenshots.md with:

Updated blocks-ci-screenshots.md
<!-- auto-generated by CI — do not edit manually -->

#### editor/codeEditor/CodeEditor/Dark
![screenshot](https://hediet-screenshots.azurewebsites.net/images/cb32a3e854b5734fe5aaca2318f2e0a42ee821b05ea97883ea42c5ba95edb3c3)

#### editor/codeEditor/CodeEditor/Light
![screenshot](https://hediet-screenshots.azurewebsites.net/images/42624fbba5e0db7f32c224b5eb9c5dd3b08245697ae2e7d2a88be0d7c287129b)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a performance issue caused by repeated updateReadonly calls firing onDidChangeReadonly for each file, which can cascade into expensive event handling (notably in prompt file registration).

Changes:

  • Extend IFilesConfigurationService.updateReadonly to accept URI[] and batch the readonly override updates into a single onDidChangeReadonly event.
  • Batch readonly updates in PromptsService (both provider listing and contributed file registration) to avoid per-file events.
  • Add browser tests to validate event batching behavior for updateReadonly with single and multiple resources.
Show a summary per file
File Description
src/vs/workbench/services/filesConfiguration/test/browser/filesConfigurationService.test.ts Adds tests ensuring updateReadonly batches events for arrays and handles empty arrays/reset correctly.
src/vs/workbench/services/filesConfiguration/common/filesConfigurationService.ts Updates the service contract and implementation to support URI[] and fire a single readonly-change event per batch.
src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts Batches prompt-file readonly updates to prevent renderer freezes caused by per-file readonly change events.

Copilot's findings

Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts:948

  • The microtask flush calls filesConfigService.updateReadonly(...) without awaiting/handling the Promise. Please make the intent explicit (e.g. void ...) and add .catch(...) logging to avoid potential unhandled rejections (especially if a different IFilesConfigurationService implementation throws).
			queueMicrotask(() => {
				const uris = this._pendingReadonlyUris;
				this._pendingReadonlyUris = [];
				this._pendingReadonlyFlush = false;
				this.filesConfigService.updateReadonly(uris, true);
			});
  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment thread src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts Outdated
Comment thread src/vs/workbench/services/filesConfiguration/common/filesConfigurationService.ts Outdated
@pwang347 pwang347 requested a review from bpasero April 15, 2026 22:25
@pwang347 pwang347 marked this pull request as ready for review April 15, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI becomes unresponsive when opening workspace with Copilot installed

2 participants