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

[xperiandri]

Go To All (Ctrl+T / Code Search) on a multi-targeted F# solution showed F# results late — often only on the second search — and the window stalled while it searched. Reproduced on a solution with 135 project instances (26 project files, five target frameworks each for the app projects).

Roslyn hands the F# service every project instance one after another and publishes a project's results only when the whole project is done. The service parsed every file of every instance, since the parse cache is per DocumentId and the defines differ; read the text of every document before matching anything, which for a closed document is a file read, per file per framework per keystroke; and started all of a project's documents at once, unthrottled.

The first instance of a project file now searches every file, and the others search only the files they alone compile and the files whose parse depends on the defines. A document's text is read only after one of its declarations matched, and a project's documents are searched at most ProcessorCount at a time. Results for one file come from one instance, except under conditional compilation.

Not changed: while a solution is still loading Roslyn searches only its own persisted index, so F# results still appear only once it has loaded; that needs an ExternalAccess extension.

No timings are claimed: per search the work goes from one parse and one file read per file per framework to one parse per file, plus the files with conditional directives, and a read per matched file. The first commit (test helpers) is shared with #20462.

🤖 Generated with Claude Code

[github-actions]

✅ 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

[xperiandri]

Suggested change

	syntheticProject.GetAllProjects()
	|> List.distinctBy _.Name
	|> List.map (fun project -> project, ProjectId.CreateNewId())
	syntheticProject.GetAllProjects()
	|> Seq.distinctBy _.Name
	|> Seq.map (fun project -> project, ProjectId.CreateNewId())
	|> Seq.toList

[xperiandri]

Applied in e81a37e: the chain goes through Seq and is materialized once with Seq.toList.

[xperiandri]

I suppose it will be more performant, no?

Suggested change

	let instances =
	project.Solution.Projects
	|> Seq.filter (fun p -> p.FilePath = projectPath)
	|> Seq.map _.Id
	|> List.ofSeq
	fun (document: Document) ->
	match document.FilePath with
	| null -> true
	| path ->
	let documentIds = project.Solution.GetDocumentIdsWithFilePath path
	let owner =
	instances
	|> List.find (fun id -> documentIds |> Seq.exists (fun documentId -> documentId.ProjectId = id))
	let instances =
	project.Solution.Projects
	|> Seq.filter (fun p -> p.FilePath = projectPath)
	|> Seq.map _.Id
	|> Seq.toArray
	fun (document: Document) ->
	match document.FilePath with
	| null -> true
	| path ->
	let documentIds = project.Solution.GetDocumentIdsWithFilePath path
	let owner =
	instances
	|> Array.find (fun id -> documentIds |> Seq.exists (fun documentId -> documentId.ProjectId = id))

[xperiandri]

Moot now: the rework removed the owner lookup this was about. Every instance reports what it compiles and only the parse is shared, so there is no instances array and no find left in NavigateToSearchService.fs to make faster.

[T-Gro]

🤖🕵️ Shared declarations disappear from Current Project searches on a non-owner target after the cache warms.

// Existing multi-target fixture; fresh service, second target active.
let p = solution.GetProject instances[1]
let run () =
    service.SearchProjectAsync(p, ImmutableArray.Empty, "plainUse",
        service.KindsProvided, CancellationToken.None).Result
run () // contains plainUse
run () // empty

Roslyn submits only the active project for this scope. Preserve project-local results; the solution-order owner is not searched.

[xperiandri]

Both are real, and they killed the design rather than a line of it. Pushed a rework.

The skip is gone. I had missed that NavigateToSearcher pools its seen set with NavigateToSearchResultComparer, which already collapses results by file path and span — so the duplicates the skip existed to prevent were never reaching the user anyway, and every instance can safely report what it compiles. That is your first case: the current-project scope submits one project, and the instance that was told to stay quiet is the only one asked.

What the instances should share is the parse, which is the cost. A parse whose tree holds no conditional directives does not depend on the defines, so it is stored under a key no define set can equal and every instance reuses it; one that does hold them is stored per define set. The entry carries the text version, which is your second case: an edit that puts a declaration behind #if is a new entry rather than a flag left over from the parse before it.

The key for the define-independent entry is "?" on purpose — defines are identifiers, so an instance that happens to define nothing cannot read that entry as its own.

The test asserting a declaration is reported once across the instances went with the skip; that is the searcher's job. In its place are your two cases plus one that keeps the define-keyed parse honest — a declaration behind #if FOO must not reach the instance without FOO.

One thing I could not do: run the new tests against the old implementation as a control. Reverting just that file makes fsc.exe die with 0x80131506 in my worktree, reproducibly and regardless of my change, and I did not chase it. So the evidence that these tests catch the two bugs is your repro, not a run of mine. Full FSharp.Editor.Tests on the rework: 7205 passed, 0 failed.

The release note said the old scheme out loud, so it is rewritten too.

[T-Gro]

🤖🕵️ Newly conditional declarations are missing from the first solution search after an edit. Warm the cache on a shared file without directives, then replace its text in both target instances with:

module ModuleSecond
#if FOO
let addedFoo = 1
#endif

Searching addedFoo in the FOO instance first, then the plain owner, returns no result; repeating finds it. Roslyn prioritizes the active project, so this order occurs in normal searches. Validate the cached flag against the current text version before skipping.

[xperiandri]

Covered by the same rework as the other thread. The cache no longer keeps a directives flag that can outlive the text it was read from: an entry carries the text version it was parsed at, so the edit that puts addedFoo behind #if FOO produces a new entry rather than a skip decided by the parse before it.

The repro is a declaration an edit puts behind a directive is reported by the instance that defines it in MultiTargetNavigateToSearchTests.fs — warm the shared file without directives, replace its text in both instances, search the FOO instance first.

[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]

🔍 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.

[xperiandri]

Fixed in 6e68aace9e. The key carries the language version beside the defines now, and both come from TryGetCompilationDefinesAndLangVersionForEditingDocument, which answers nothing where the project has not produced its options — so cross-project reuse is off exactly while the identity is unavailable, and such a document parses for itself.

The fallback was worse than incomplete: the editing defaults are not a neutral placeholder but the very define set a project that defines nothing of its own really has, so a document keyed under them read that project's parse, with the branch of a #if that project chose.

The other parsing options cannot differ for one path: ApplyLineDirectives is false for every document the editor parses, and IsInteractive follows from the file's extension.

New test, a project whose options have not arrived does not answer from another project's parse: two projects hold the same file behind one TextLoader — a linked file open in the editor, which is what makes the two documents report the same text version and lets one read the other's entry at all — one with options that define nothing, the other with none. Before the fix the second answered from the first's parse; now it asks for the parse it cannot have and is cancelled. Twelve *NavigateTo* tests pass.

[xperiandri]

Fixed in 840c454ce5. The two bullets are the two designs this PR went through: the first describes the instance that skipped files, which the rework replaced with sharing the parse, and it should have gone with it. Only the second remains.