Skip to content

Perf: Throttle parallel typechecks in Find All References - #20128

Open
xperiandri wants to merge 4 commits into
dotnet:mainfrom
xperiandri:fix-find-references-throttle
Open

Perf: Throttle parallel typechecks in Find All References#20128
xperiandri wants to merge 4 commits into
dotnet:mainfrom
xperiandri:fix-find-references-throttle

Conversation

@xperiandri

Copy link
Copy Markdown
Contributor

Fixes #20127

Problem

Project.FindFSharpReferencesAsync launched one CancellableTask per document in the project simultaneously via CancellableTask.whenAll, causing an unbounded number of parallel typechecks (one per document) at once during Find All References / Rename.

Fix

  • Added CancellableTask.whenAllThrottled maxDegreeOfParallelism in CancellableTasks.fs, using a SemaphoreSlim to cap concurrency while preserving cancellation semantics.
  • Applied it in Project.FindFSharpReferencesAsync (WorkspaceExtensions.fs), capping concurrency to max 1 Environment.ProcessorCount.

@xperiandri
xperiandri marked this pull request as ready for review August 3, 2026 19:32
@xperiandri
xperiandri requested a review from a team as a code owner August 3, 2026 19:33
@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Aug 3, 2026

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 AI review (@expert-reviewer): no significant issues found. Please verify independently.

Reviewed the throttling implementation for correctness:

  • use semaphore combined with return! Task.WhenAll(tasks) correctly awaits within the use scope, avoiding the common SemaphoreSlim use-after-dispose pitfall (this would be a bug with return instead of return!).
  • semaphore.WaitAsync(ct) is placed before the try, so Release() runs only when a permit was actually acquired — no risk of over-release, and cancelled waiters correctly skip the release.
  • The expensive start ct task runs only after acquiring a permit, so concurrency is genuinely capped.
  • max 1 Environment.ProcessorCount guarantees a valid (>= 1) semaphore count.

Cancellation and exception-aggregation semantics match the existing whenAll. LGTM.

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Aug 4, 2026
@T-Gro
T-Gro self-requested a review August 4, 2026 09:11
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Aug 4, 2026
@xperiandri
xperiandri force-pushed the fix-find-references-throttle branch from 149109e to c5582ba Compare August 5, 2026 00:26
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`vsintegration/src` docs/release-notes/.VisualStudio/18.vNext.md

@xperiandri
xperiandri force-pushed the fix-find-references-throttle branch from c5582ba to a86b21f Compare August 5, 2026 08:37
}

let tasks = seq { for task in tasks do yield runThrottled task }
return! Task.WhenAll (tasks)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Task.WhenAll starts these tasks before the builder checks cancellation. Cancellation can dispose the semaphore before Release(), which then throws ObjectDisposedException. Keep the semaphore alive until all started tasks complete.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the latest commit

@xperiandri
xperiandri force-pushed the fix-find-references-throttle branch 6 times, most recently from 3164857 to b63942a Compare August 13, 2026 02:58
Refactored the `whenAllThrottled` function to use an array of background tasks, ensuring each acquires/releases the semaphore properly. Explicitly disposes the semaphore after all tasks complete using a continuation on `Task.WhenAll`. Simplified the XML doc comment. This replaces the previous sequence expression and `use` binding with more robust disposal logic.
@xperiandri
xperiandri force-pushed the fix-find-references-throttle branch from b63942a to 7268a33 Compare August 13, 2026 20:53
@xperiandri
xperiandri requested a review from T-Gro August 14, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Perf: Throttle parallel typechecks in Find All References

2 participants