Skip to content

Search each file of a multi-targeted F# project once in Find All References and Rename - #20464

Open
xperiandri wants to merge 5 commits into
dotnet:mainfrom
xperiandri:perf/find-references-multitarget
Open

Search each file of a multi-targeted F# project once in Find All References and Rename#20464
xperiandri wants to merge 5 commits into
dotnet:mainfrom
xperiandri:perf/find-references-multitarget

Conversation

@xperiandri

Copy link
Copy Markdown
Contributor

Description

Find All References and Rename on a symbol of a multi-targeted F# project did every piece of work once per target framework. Measured in the debugger on a solution with 135 project instances: a scope of two projects (the two instances of one .fsproj) grew to 86 instances of 26 project files after adding dependents, 1246 documents in all; the snapshots of all 86 were built one after another before the first search started; every file was type checked once per instance, and FCS never shares those checks between instances; Roslyn's window hides the duplicate results, so the user only sees the time. On top of that the "skip the files before the declaration" optimisation only applied in the instance owning the first DocumentId of the declaration, because Project.FindFSharpReferencesAsync compared document.Project = this by reference.

Change (SymbolHelpers.fs, WorkspaceExtensions.fs):

  • getSymbolUsesInProjects keeps F# projects only (which also stops getFSharpOptionsForProject from being asked about a C# project in the external-symbol branch) and, with EnableFastFindReferencesAndRename on (the default), groups the instances of one project file. The instance searched in full is the one of the current document, else one in its dependency closure, else the first; the other instances only search files compiled solely there and files whose parse tree has conditional compilation directives (ParsedInput.Trivia.ConditionalDirectives), since those are the files whose sources can differ between instances.
  • Searches start as soon as a project's snapshot is ready instead of after all snapshots; snapshots are built only for the transparent compiler (Document.FindFSharpReferencesAsync takes an FSharpProjectSnapshot voption and picks the FCS overload from it); one SemaphoreSlim(ProcessorCount) bounds the concurrent file checks across the whole search instead of per project.
  • Project.FindFSharpReferencesAsync resolves the declaration document inside its own instance, so the compile-order skip works in every instance.
  • The two dead arms of findSymbolUses (CurrentDocument/SignatureAndImplementation inside the | scope -> fallback) are gone; the outer match is exhaustive.

Behaviour change with the option off: every instance is still searched in full, but the concurrency bound now applies across the solution where the code used to run one project's documents sequentially and projects without a limit. Residual risk with the option on: an instance can resolve an overload differently without any #if (an extension member shadowed by a newer BCL intrinsic); the ranking covers the polyfill case within the current project's world, and turning the option off restores the full search.

Rename needs no change: getSymbolUsesInSolution now yields one DocumentId per file plus the instance-specific ones, and Roslyn merges the edits of linked documents.

Tests: MultiTargetFindReferencesTests loads one project as two instances (one without FOO and without the fourth file, one with both) through the new RoslynTestHelpers.CreateMultiTargetSolution and, starting from either instance, checks that Find All References reports the signature, the plain use, the use under #if FOO and the use in the instance-only file exactly once each, and that Rename gets one document per file, owned by an instance that compiles it. The first commit (test helpers) is shared with #20462; the second moves the IFSharpFindUsagesContext stub into RoslynTestHelpers.

No timings are claimed: on the solution above the search scope goes from 86 instances to 26, plus the conditional and instance-only files of the other 60.

Checklist

  • Test cases added

  • Performance benchmarks added in case of performance changes

  • Release notes entry updated:

    Please make sure to add an entry with short succinct description of the change as well as link to this pull request to the respective release notes file, if applicable.

    Release notes files:

    • If anything under src/Compiler has been changed, please make sure to make an entry in docs/release-notes/.FSharp.Compiler.Service/<version>.md, where <version> is usually "highest" one, e.g. 42.8.200
    • If language feature was added (i.e. LanguageFeatures.fsi was changed), please add it to docs/release-notes/.Language/preview.md
    • If a change to FSharp.Core was made, please make sure to edit docs/release-notes/.FSharp.Core/<version>.md where version is "highest" one, e.g. 8.0.200.

    Information about the release notes entries format can be found in the documentation.
    Example:

    If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

🤖 Generated with Claude Code

xperiandri and others added 4 commits September 6, 2026 11:18
…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>
The IFSharpFindUsagesContext stub of FindReferencesTests moves to
RoslynTestHelpers.CreateFindUsagesContext so other test files can collect the
definitions and references a search reports.

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

Find All References and Rename searched every target-framework instance of a
multi-targeted F# project in full, and built the snapshots of all projects in
scope before the first search started. Instances of one project file are now
grouped: the instance of the current document (or one in its dependency
closure) is searched in full, the others only for files compiled solely there
and for files with conditional compilation directives, whose sources can
differ between instances. Searches start as soon as a project's snapshot is
ready, snapshots are built only for the transparent compiler, and one
SemaphoreSlim bounds the concurrent file checks across the whole search
instead of per project.

With EnableFastFindReferencesAndRename off every instance is still searched in
full; the concurrency bound then replaces the previous per-project sequential
loop.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
One project loaded as two instances (one without FOO and without the fourth
file, one with both): every file is reported once, and Rename gets one
document per file, owned by an instance that compiles it.

The fixture lives in its own file: the modules of one file share a static
initializer, so a second fixture module makes the first module's values
observable before they are assigned when xunit runs the classes in parallel.

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

github-actions Bot commented Sep 6, 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

@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant