Skip to content

settings: use local StopWatch to avoid timing corruption between concurrent searches#304361

Merged
rzhao271 merged 1 commit intomicrosoft:mainfrom
xingsy97:wt/settings-local-stopwatch
Mar 24, 2026
Merged

settings: use local StopWatch to avoid timing corruption between concurrent searches#304361
rzhao271 merged 1 commit intomicrosoft:mainfrom
xingsy97:wt/settings-local-stopwatch

Conversation

@xingsy97
Copy link
Copy Markdown
Member

@xingsy97 xingsy97 commented Mar 24, 2026

Problem

The settings editor uses a shared class-level StopWatch instance for measuring search duration telemetry. When AI search runs concurrently with local/remote searches (launched via createCancelablePromise without await), a new search can call stopWatch.reset() while a previous search is still timing, producing incorrect elapsed values.

Fix

Replace the shared member variable with local StopWatch instances in getLLMRankedResults() and searchWithProvider(). Each search path gets its own timer.

Copilot AI review requested due to automatic review settings March 24, 2026 05:40
@vs-code-engineering
Copy link
Copy Markdown
Contributor

📬 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

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 fixes search telemetry timing in the Settings editor by removing the shared StopWatch instance that could be reset by overlapping local/remote/AI searches, and instead measuring each provider call with its own local stopwatch.

Changes:

  • Removed the stopWatch class field to avoid shared state across concurrent searches.
  • Introduced per-call StopWatch instances in both AI LLM-ranked search and provider-backed preference searches.
  • Updated elapsed-time logging to use the local stopwatch values.

@rzhao271 rzhao271 enabled auto-merge (squash) March 24, 2026 17:18
@rzhao271 rzhao271 merged commit 4feb37f into microsoft:main Mar 24, 2026
21 of 22 checks passed
@xingsy97 xingsy97 deleted the wt/settings-local-stopwatch 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