Parse each file of a multi-targeted F# project once in Go To All by xperiandri · Pull Request #20483 · dotnet/fsharp

xperiandri · 2026-09-07T20:00:52Z

In multi-targeted F# solutions, Go To All (Ctrl+T) parsed every file once per target framework, so a search stalled on the same text being parsed again and again. A file is now parsed once and every target framework reuses the parse, unless conditional compilation makes the frameworks differ, and a project's documents are searched through a throttle instead of all at once.

F# results still appear only after the solution has loaded.

🤖 Generated with Claude Code

github-actions · 2026-09-07T20:01:03Z

✅ Release notes checked

✅ Found changes and release notes in following paths:

Change path Release notes path Description

`vsintegration/src` docs/release-notes/.VisualStudio/18.vNext.md

T-Gro

🤖🕵️ Please shorten the description using this guidance. Focus on the problem and why the change is needed, in simplified technical English. Leave the implementation inventory to the Files tab and retain necessary caveats.

github-actions · 2026-09-22T16:45:12Z

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: Compiler service or Visual Studio behavior changed.

Generated by PR Tooling Safety Check · gpt56 1.9M · ◷

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 1 · 1

Open (2)

Cache key omits parse options and can return incorrect results · New
Duplicate release note bullets describe the same fix · New

What changed in this PR

This PR optimizes Go To All for multi-targeted F# projects by caching parses, delaying text reads, and throttling document searches.

Changes:

Adds define-aware navigable-item caching and deferred source reads.
Adds multi-target test infrastructure and regression tests.
Documents the performance improvement in Visual Studio release notes.

File	Description
vsintegration/tests/FSharp.Editor.Tests/MultiTargetNavigateToSearchTests.fs	Updated as part of this pull request.
vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs	Updated as part of this pull request.
vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj	Updated as part of this pull request.
vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs	Updated as part of this pull request.
docs/release-notes/.VisualStudio/18.vNext.md	Updated as part of this pull request.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

T-Gro

🤖🕵️ AI review — verify independently.

T-Gro

🤖 🕵️ Re-reviewed all recent changes as of f20b658; LGTM

…t the multi-target tests Brings the fixes pushed to dotnet#20518 and dotnet#20483 onto the integration branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The port of dotnet#20483: parses are kept per file path and define set, a parse with no conditional directives is shared by every target-framework instance, and TryGetCachedNavigableItems takes the document. The mention picker commits already call it that way, so the branch did not build without this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A standalone script has its options kept per document, so its project never produces the defines and language version a parse is keyed by. A parse that holds conditional directives went into the cache under defines such a document never reads back, and a document without a path was not kept at all, so every search of the unchanged document asked for the parse again. Such a document's navigable items are now kept under its DocumentId with the text version. Nothing is shared across projects, and a project whose first parse makes it produce its options is still keyed by them from the next search on. This is 1b15890 of dotnet#20483 on the shared items cache this branch has. A parse without directives was already kept here, under the key every instance reads, so the test also searches a script with an #if. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch still had only that bullet for dotnet#20483. It gives way to the one that describes what the PR does now, as on the PR branch (998ae4a). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…t the multi-target tests Brings the fixes pushed to dotnet#20518 and dotnet#20483 onto the integration branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The port of dotnet#20483: parses are kept per file path and define set, a parse with no conditional directives is shared by every target-framework instance, and TryGetCachedNavigableItems takes the document. The mention picker commits already call it that way, so the branch did not build without this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A standalone script has its options kept per document, so its project never produces the defines and language version a parse is keyed by. A parse that holds conditional directives went into the cache under defines such a document never reads back, and a document without a path was not kept at all, so every search of the unchanged document asked for the parse again. Such a document's navigable items are now kept under its DocumentId with the text version. Nothing is shared across projects, and a project whose first parse makes it produce its options is still keyed by them from the next search on. This is 1b15890 of dotnet#20483 on the shared items cache this branch has. A parse without directives was already kept here, under the key every instance reads, so the test also searches a script with an #if. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch still had only that bullet for dotnet#20483. It gives way to the one that describes what the PR does now, as on the PR branch (998ae4a). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…r tests Test helpers so far put every synthetic file into one Roslyn project. CreateMultiProjectSolution creates one project per synthetic project with project references, the way VS wires project-to-project references; CreateMultiTargetSolution creates one project per target instance sharing the project path and the document paths, the way VS loads a multi-targeted project. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ead its text only for matches Roslyn's NavigateTo searcher hands the F# service every target-framework instance of a project, one after another, and the service parsed every file of each instance, read the text of every file before matching anything, and started all of that for a project at once. On a solution with 135 project instances that meant one parse and one file read per file per framework, thousands of concurrent tasks, and results that arrived long after the user stopped typing. The first instance of a project file in the solution now searches every file; the other instances only search the files they alone compile and the files whose parse depends on the defines, known from whichever instance parsed the file first. A file's text is read only when one of its declarations matched, and a project's files are searched at most ProcessorCount at a time. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Skipping a file in every instance but the first is wrong twice over, as the review showed. Go To All scoped to the current project submits only that project, so the instance that was told to stay quiet is the only one asked, and a shared declaration disappears from it once the parse has taught the service the file holds no directives. And the flag that decision reads was keyed by path alone, so an edit that puts a declaration behind `#if` left the previous answer in place. Nothing needs skipping. `NavigateToSearcher` pools its seen set with `NavigateToSearchResultComparer`, which already collapses results by file path and span, so every instance can report what it compiles. What the instances should share is the parse, which is what costs. A parse whose tree holds no conditional directives does not depend on the defines, so it is kept under a key no define set can equal and every instance reuses it; one that does hold them is kept per define set, since those instances genuinely parse the file differently. The entry carries the text version, so an edit is a new entry rather than a stale flag. The test that asserted a declaration is reported once across the instances went with the skip: that is the searcher's job, not this service's. In its place are the two cases from the review — a lone instance reporting a shared declaration, and a declaration an edit puts behind a directive — and one that keeps the define-keyed parse honest by checking such a declaration does not reach the instance without the define. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The cache was a dictionary from a struct tuple of two strings to an anonymous struct record, spelled out in the type arguments at the one place it is declared, and read back as `struct (key, path)` wherever it is used. Nothing at those sites said which string was the defines and which the path. Give both a name at the top of the file. They stay struct records, so the key keeps the same equality and the entries allocate no more than before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The defines a file was parsed with are not the whole of what its tree depends on: the language version decides what the parser accepts, and the defines were taken from the editing defaults when the project had not produced its options yet - a set another project really has, so its parse answered for a project that never asked for it, with the branch of a `#if` that project chose. The key now carries the language version, and both it and the defines are read only where the project has produced them: a project without options parses for itself instead of reading an entry that is not about it. The rest of the parsing options cannot differ for one path - the editor never applies line directives, and whether a file is interactive follows from its extension. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The multi-project and multi-target helpers now match the ones dotnet#20462 adds, struct results included, so this branch rebases onto it without reconciling two copies. A struct tuple cannot be bound by a `let private` pattern, so the module-level fixture bindings lose the modifier. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A struct tuple cannot be bound with `let private` (FS0010: the grammar takes an access modifier only before a name), so the binding sits in a private AutoOpen module: the tests here see both names, nothing outside does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A standalone script has its options kept per document, so its project never produces the defines and language version a parse is keyed by, and every search of the unchanged script asked for the parse again. Such a document's navigable items are now kept under its DocumentId with the text version. Nothing is shared across projects, and a project whose first parse makes it produce its options is still keyed by them from the next search on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The cache key is built from the parsing options the document's parse is made with: the ones the options reactor already holds, else the awaited ones. That removes the per-document cache, the cold-project branch, the sentinel define set and the forwarding members for the options peek, and lets the key say in its type whether a parse depends on the defines. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

One search helper for the three that repeated it, the unused synthetic files and excluded-file data gone, the edit test run in both search orders, and the comments that restated the test names replaced by those names. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Documents that AdhocWorkspace.AddSolution loads each carry their own version, so no instance could read another's parse and the tests passed on main. They now share a version per file, as the projects of a loaded solution do, and a test counts the parse requests across the instances. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Replaying the branch onto main left some files short of the branch's own content; this restores it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…t the multi-target tests Brings the fixes pushed to dotnet#20518 and dotnet#20483 onto the integration branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The port of dotnet#20483: parses are kept per file path and define set, a parse with no conditional directives is shared by every target-framework instance, and TryGetCachedNavigableItems takes the document. The mention picker commits already call it that way, so the branch did not build without this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A standalone script has its options kept per document, so its project never produces the defines and language version a parse is keyed by. A parse that holds conditional directives went into the cache under defines such a document never reads back, and a document without a path was not kept at all, so every search of the unchanged document asked for the parse again. Such a document's navigable items are now kept under its DocumentId with the text version. Nothing is shared across projects, and a project whose first parse makes it produce its options is still keyed by them from the next search on. This is 1b15890 of dotnet#20483 on the shared items cache this branch has. A parse without directives was already kept here, under the key every instance reads, so the test also searches a script with an #if. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch still had only that bullet for dotnet#20483. It gives way to the one that describes what the PR does now, as on the PR branch (998ae4a). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

xperiandri requested a review from a team as a code owner September 7, 2026 20:00

github-project-automation Bot added this to F# Compiler and Tooling Sep 7, 2026

github-project-automation Bot moved this to New in F# Compiler and Tooling Sep 7, 2026

xperiandri commented Sep 7, 2026

View reviewed changes

Comment thread vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs Resolved

xperiandri commented Sep 7, 2026

View reviewed changes

Comment thread vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs Resolved

xperiandri commented Sep 7, 2026

View reviewed changes

Comment thread vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs Resolved

github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Sep 7, 2026

xperiandri mentioned this pull request Sep 9, 2026

Answer the Navigate To search that runs while the solution loads #20492

Draft

T-Gro reviewed Sep 9, 2026

View reviewed changes

Comment thread vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs Resolved

Comment thread vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs Resolved

T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 9, 2026

T-Gro self-requested a review September 9, 2026 09:24

github-actions Bot added the ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager label Sep 10, 2026

This comment has been minimized.

Sign in to view

xperiandri force-pushed the perf/navigate-to-multitarget branch from 576ffda to e81a37e Compare September 11, 2026 15:40

T-Gro requested changes Sep 15, 2026

View reviewed changes

github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Sep 15, 2026

T-Gro added the vsintegration-only Changes only Visual Studio integration, plus optional docs, release notes, or tests label Sep 21, 2026

xperiandri mentioned this pull request Sep 22, 2026

Search a project's priority documents first in Navigate To #20615

Open

xperiandri force-pushed the perf/navigate-to-multitarget branch from e81a37e to 952d987 Compare September 22, 2026 11:34

Copilot AI lite review requested due to automatic review settings September 26, 2026 01:00

xperiandri force-pushed the perf/navigate-to-multitarget branch from 952d987 to 48fc560 Compare September 26, 2026 01:00

Copilot started reviewing on behalf of xperiandri September 26, 2026 01:01

View session

Copilot AI reviewed Sep 26, 2026

View reviewed changes

Comment thread vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs Resolved

Comment thread docs/release-notes/.VisualStudio/18.vNext.md Resolved

T-Gro reviewed Oct 1, 2026

View reviewed changes

Comment thread vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs Resolved

This was referenced Oct 1, 2026

Resolve Go To Definition targets through the workspace's current solution #20462

Open

Keep Navigate To's items on disk between sessions #20689

Draft

xperiandri force-pushed the perf/navigate-to-multitarget branch from 2dd51b3 to f20b658 Compare October 2, 2026 16:03

T-Gro reviewed Oct 3, 2026

View reviewed changes

xperiandri and others added 16 commits October 29, 2026 20:54

Test NavigateTo across the target-framework instances of a project

7944f84

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Add the release note for the NavigateTo multi-target fix

0836284

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Format the multi-target NavigateTo tests

f63716a

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Build the synthetic projects' id list through Seq and materialize once

caa2b4c

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Bring the rebased branch to what it had before the rebase

064ba58

Replaying the branch onto main left some files short of the branch's own content; this restores it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

auto-merge was automatically disabled October 29, 2026 22:03
Head branch was pushed to by a user without write access

xperiandri force-pushed the perf/navigate-to-multitarget branch from 502ab34 to 064ba58 Compare October 29, 2026 22:03

Navigation Menu

Repository navigation

Parse each file of a multi-targeted F# project once in Go To All - #20483

Parse each file of a multi-targeted F# project once in Go To All#20483
xperiandri wants to merge 16 commits into
dotnet:mainfrom
xperiandri:perf/navigate-to-multitarget

xperiandri commented Sep 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

T-Gro left a comment

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

T-Gro left a comment

Uh oh!

Uh oh!

T-Gro left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

xperiandri commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Release notes checked

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xperiandri commented Sep 7, 2026 •

edited

Loading

github-actions Bot commented Sep 7, 2026 •

edited

Loading