Skip to content

Settings editor - avoid repeated extension list refresh#303957

Merged
rzhao271 merged 1 commit intomicrosoft:mainfrom
xingsy97:wt/settings-refresh-outside-loop
Mar 27, 2026
Merged

Settings editor - avoid repeated extension list refresh#303957
rzhao271 merged 1 commit intomicrosoft:mainfrom
xingsy97:wt/settings-refresh-outside-loop

Conversation

@xingsy97
Copy link
Copy Markdown
Member

Move refreshInstalledExtensionsList() from inside the recommended-extensions loop to before it.

Problem

In onConfigUpdate(), the loop over toggleData.settingsEditorRecommendedExtensions calls await this.refreshInstalledExtensionsList() on every iteration. This method calls extensionManagementService.getInstalled() (full IPC roundtrip) to fetch all installed extensions.

The installed extension list does not change between loop iterations, so N recommended extensions produce N identical IPC calls.

Fix

Call refreshInstalledExtensionsList() once before the loop. This still prevents the race between concurrent onConfigUpdate calls (the original comment's concern) while avoiding redundant IPC.

Copilot AI review requested due to automatic review settings March 23, 2026 04:53
@vs-code-engineering vs-code-engineering bot added this to the 1.113.0 milestone Mar 23, 2026
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 optimizes SettingsEditor2.onConfigUpdate() by reducing redundant calls to refreshInstalledExtensionsList() (and therefore extensionManagementService.getInstalled()) when processing recommended extensions for the Settings editor.

Changes:

  • Move await this.refreshInstalledExtensionsList() out of the per-recommended-extension loop so it runs once per onConfigUpdate() invocation.
  • Update inline comments to describe the new refresh behavior.

@xingsy97 xingsy97 force-pushed the wt/settings-refresh-outside-loop branch from ba1dbd9 to 7a6de9f Compare March 23, 2026 05:03
@xingsy97 xingsy97 marked this pull request as ready for review March 23, 2026 05:04
@vs-code-engineering
Copy link
Copy Markdown
Contributor

vs-code-engineering bot commented Mar 23, 2026

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@rzhao271

Matched files:

  • src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts

@xingsy97 xingsy97 changed the title Settings editor - avoid redundant refreshInstalledExtensionsList() calls in loop Settings editor - avoid repeated extension list refresh Mar 23, 2026
@rzhao271 rzhao271 modified the milestones: 1.113.0, 1.114.0 Mar 23, 2026
@xingsy97 xingsy97 force-pushed the wt/settings-refresh-outside-loop branch from 7a6de9f to 420b149 Compare March 23, 2026 19:12
@rzhao271 rzhao271 enabled auto-merge (squash) March 27, 2026 15:38
@rzhao271 rzhao271 merged commit 9b0dd7c into microsoft:main Mar 27, 2026
18 checks passed
@xingsy97 xingsy97 deleted the wt/settings-refresh-outside-loop branch March 31, 2026 06:19
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.

4 participants