Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.VisualStudio/18.vNext.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

* Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482))
* Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503))
* Reuse the in-memory PE reference of a referenced C# project while its dependent semantic version is unchanged, instead of emitting a PE image and constructing a metadata reader for every `Compilation` Roslyn recreates. A project that has analyzer references, and so may run source generators, is reused only until its next edit. ([PR #20460](https://github.com/dotnet/fsharp/pull/20460))
* Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128), [PR #20695](https://github.com/dotnet/fsharp/pull/20695))
* Fix record-type Rename corrupting copy-and-update expressions, including from abstract signatures, while retaining Find All References results. ([Issue #20791](https://github.com/dotnet/fsharp/issues/20791), [PR #20797](https://github.com/dotnet/fsharp/pull/20797))
* Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
Expand Down
1 change: 1 addition & 0 deletions eng/Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<PackageVersion Include="Microsoft.Build.Tasks.Core" Version="$(MicrosoftBuildTasksCoreVersion)" />
<PackageVersion Include="Microsoft.Build.Utilities.Core" Version="$(MicrosoftBuildUtilitiesCoreVersion)" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisCSharpVersion)" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisCSharpVersion)" />
<PackageVersion Include="Microsoft.CodeAnalysis.EditorFeatures" Version="$(MicrosoftCodeAnalysisEditorFeaturesTextVersion)" />
<PackageVersion Include="Microsoft.CodeAnalysis.EditorFeatures.Text" Version="$(MicrosoftCodeAnalysisEditorFeaturesTextVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.LanguageServices.ExternalAccess" Version="$(MicrosoftVisualStudioLanguageServicesExternalAccessVersion)" />
Expand Down
82 changes: 82 additions & 0 deletions tests/fsharp/Compiler/Service/MultiProjectTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ namespace FSharp.Compiler.UnitTests

open System
open System.IO
open System.Threading
open FSharp.Compiler.Diagnostics
open Xunit
open FSharp.Test
Expand Down Expand Up @@ -209,3 +210,84 @@ let y = 1




let private mkCountedCSharpPEReference (stamp: DateTime) =
let csSrc =
"""
namespace CSharpTest
{
public class CSharpClass
{
}
}
"""

let csOptions = CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)
let csSyntax = CSharpSyntaxTree.ParseText(csSrc)
let csReferences = TargetFrameworkUtil.getReferences TargetFramework.NetStandard20
let cs = CSharpCompilation.Create("csharp_test.dll", references = csReferences.As<MetadataReference>(), syntaxTrees = [csSyntax], options = csOptions)

let mutable emitCount = 0

let getStream ct =
Interlocked.Increment(&emitCount) |> ignore
let ms = new MemoryStream()
cs.Emit(ms, cancellationToken = ct) |> ignore
ms.Position <- 0L
ms :> Stream |> Some

let csRefProj = FSharpReferencedProject.PEReference((fun () -> stamp), DelayedILModuleReader("""Z:\csharp_test.dll""", getStream))

csRefProj, (fun () -> emitCount)

let private projectReferencing (csRefProj: FSharpReferencedProject) =
let fsOptions = CompilerAssert.DefaultProjectOptions TargetFramework.Current

{ fsOptions with
ProjectId = Some(Guid.NewGuid().ToString())
OtherOptions = Array.append fsOptions.OtherOptions [|"""-r:Z:\csharp_test.dll"""|]
ReferencedProjects = [|csRefProj|] }

let private checkUsesCSharpClass (options: FSharpProjectOptions) =
let fsText =
"""
module FSharpTest

open CSharpTest

let test() =
CSharpClass()
"""
|> SourceText.ofString

match
CompilerAssert.Checker.ParseAndCheckFileInProject("test.fs", 0, fsText, options)
|> Async.RunSynchronouslyImmediate
|> snd
with
| FSharpCheckFileAnswer.Aborted -> failwith "check file aborted"
| FSharpCheckFileAnswer.Succeeded checkResults -> Assert.shouldBeEmpty checkResults.Diagnostics

[<Fact>]
let ``Reusing a CSharp reference's stamp avoids re-emitting a recreated Compilation``() =
let stamp = DateTime(2024, 1, 1)
let csRefProj, emitCount = mkCountedCSharpPEReference stamp
let recreatedRefProj, recreatedEmitCount = mkCountedCSharpPEReference stamp

let fsOptions = projectReferencing csRefProj
checkUsesCSharpClass fsOptions
Assert.Equal(1, emitCount())

checkUsesCSharpClass { fsOptions with ReferencedProjects = [|recreatedRefProj|] }
Assert.Equal(0, recreatedEmitCount())

[<Fact>]
let ``Changing a CSharp reference's stamp does re-emit the new Compilation``() =
let csRefProj, _ = mkCountedCSharpPEReference (DateTime(2024, 1, 1))
let changedRefProj, changedEmitCount = mkCountedCSharpPEReference (DateTime(2024, 1, 2))

let fsOptions = projectReferencing csRefProj
checkUsesCSharpClass fsOptions

checkUsesCSharpClass { fsOptions with ReferencedProjects = [|changedRefProj|] }
Assert.True(changedEmitCount() >= 1)
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ open FSharp.Compiler.CodeAnalysis
open Microsoft.VisualStudio.FSharp.Editor
open System.Threading
open Microsoft.VisualStudio.FSharp.Interactive.Session
open System.Runtime.CompilerServices
open CancellableTasks
open Microsoft.VisualStudio.FSharp.Editor.Extensions
open System.Windows
Expand Down Expand Up @@ -110,6 +109,50 @@ type private SingleFileCacheEntry =
Subscription: ConnectionPointSubscription
}

module internal PEReferenceVersion =

// A source generator can turn an edit below the declaration level into other declarations. Only Roslyn's
// out-of-process host knows which analyzer references carry one: asking here would load them into Visual Studio.
let private mayRunSourceGenerators (project: Project) =
let solution = project.Solution

let hasAnalyzerReferences (projectId: ProjectId) =
match solution.GetProject projectId with
| null -> false
| dependency -> dependency.AnalyzerReferences.Count > 0

project.AnalyzerReferences.Count > 0
|| solution.GetProjectDependencyGraph().GetProjectsThatThisProjectTransitivelyDependsOn(project.Id)
|> Seq.exists hasAnalyzerReferences

let getAsync (referencedProject: Project) (ct: CancellationToken) =
if mayRunSourceGenerators referencedProject then
referencedProject.GetDependentVersionAsync(ct)
else
referencedProject.GetDependentSemanticVersionAsync(ct)

/// <summary>
/// The in-memory PE reference of a referenced project, kept while its <c>PEReferenceVersion</c> is unchanged.
/// Roslyn recreates a <see cref="T:Microsoft.CodeAnalysis.Compilation"/> on every solution fork, and a reference per
/// instance has a fresh stamp, which invalidates every FCS cache keyed on it.
/// </summary>
[<Sealed>]
type private PEReferenceCacheEntry(version: VersionStamp, compilation: Compilation) =
let mutable pinnedUntilEmitted = compilation
let latest = WeakReference<Compilation>(compilation)

member _.Version = version

member _.TryGetCompilation() =
match pinnedUntilEmitted with
| null ->
match latest.TryGetTarget() with
| true, compilation -> ValueSome compilation
| _ -> ValueNone
| pinned -> ValueSome pinned

member _.Emitted() = pinnedUntilEmitted <- null

[<RequireQualifiedAccess>]
type private FSharpProjectOptionsMessage =
| TryGetOptionsByDocument of
Expand Down Expand Up @@ -139,74 +182,75 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) =

let singleFileCache = ConcurrentDictionary<DocumentId, SingleFileCacheEntry>()

// This is used to not constantly emit the same compilation.
let weakPEReferences = ConditionalWeakTable<Compilation, FSharpReferencedProject>()
let peReferences =
ConcurrentDictionary<ProjectId, PEReferenceCacheEntry * FSharpReferencedProject>()

let lastSuccessfulCompilations = ConcurrentDictionary<ProjectId, Compilation>()

let scriptUpdatedEvent = Event<FSharpProjectOptions>()

let createPEReference (referencedProject: Project) (comp: Compilation) =
let buildPEReference (referencedProject: Project) (entry: PEReferenceCacheEntry) =
let projectId = referencedProject.Id
let mutable stamp = DateTime.UtcNow

match weakPEReferences.TryGetValue comp with
| true, fsRefProj -> fsRefProj
| _ ->
let mutable strongComp = comp
let weakComp = WeakReference<Compilation>(comp)
let mutable stamp = DateTime.UtcNow
// Emitting can fail on unresolvable compilation errors: fall back to the project's last successful compilation.
let getStream =
fun ct ->
let tryStream (comp: Compilation) =
let ms = new MemoryStream() // do not dispose the stream as it will be owned on the reference.

// Getting a C# reference assembly can fail if there are compilation errors that cannot be resolved.
// To mitigate this, we store the last successful compilation of a C# project and re-use it until we get a new successful compilation.
let getStream =
fun ct ->
let tryStream (comp: Compilation) =
let ms = new MemoryStream() // do not dispose the stream as it will be owned on the reference.
let emitOptions =
Emit.EmitOptions(metadataOnly = true, includePrivateMembers = false, tolerateErrors = true)

let emitOptions =
Emit.EmitOptions(metadataOnly = true, includePrivateMembers = false, tolerateErrors = true)
try
let result = comp.Emit(ms, options = emitOptions, cancellationToken = ct)

try
let result = comp.Emit(ms, options = emitOptions, cancellationToken = ct)

if result.Success then
strongComp <- Unchecked.defaultof<_> // Stop strongly holding the compilation since we have a result.
lastSuccessfulCompilations.[projectId] <- comp
ms.Position <- 0L
ms :> Stream |> Some
else
strongComp <- Unchecked.defaultof<_> // Stop strongly holding the compilation since we have a result.
ms.Dispose() // it failed, dispose of stream
None
with
| :? OperationCanceledException ->
// Since we cancelled, do not null out the strong compilation ref and update the stamp.
stamp <- DateTime.UtcNow
ms.Dispose()
None
| _ ->
strongComp <- Unchecked.defaultof<_> // Stop strongly holding the compilation since we have a result.
if result.Success then
entry.Emitted()
lastSuccessfulCompilations.[projectId] <- comp
ms.Position <- 0L
ms :> Stream |> Some
else
entry.Emitted()
ms.Dispose() // it failed, dispose of stream
None
with
| :? OperationCanceledException ->
// Since we cancelled, keep the compilation pinned and update the stamp.
stamp <- DateTime.UtcNow
ms.Dispose()
None
| _ ->
entry.Emitted()
ms.Dispose() // it failed, dispose of stream
None

let resultOpt =
match weakComp.TryGetTarget() with
| true, comp -> tryStream comp
| _ -> None
let resultOpt =
match entry.TryGetCompilation() with
| ValueSome comp -> tryStream comp
| ValueNone -> None

match resultOpt with
| Some _ -> resultOpt
| _ ->
match lastSuccessfulCompilations.TryGetValue(projectId) with
| true, comp -> tryStream comp
| _ -> None
match resultOpt with
| Some _ -> resultOpt
| _ ->
match lastSuccessfulCompilations.TryGetValue(projectId) with
| true, comp -> tryStream comp
| _ -> None

let getStamp = fun () -> stamp
let getStamp = fun () -> stamp

let fsRefProj =
FSharpReferencedProject.PEReference(getStamp, DelayedILModuleReader(referencedProject.OutputFilePath, getStream))
FSharpReferencedProject.PEReference(getStamp, DelayedILModuleReader(referencedProject.OutputFilePath, getStream))

weakPEReferences.Add(comp, fsRefProj)
fsRefProj
let tryGetPEReference (referencedProject: Project) (version: VersionStamp) =
match peReferences.TryGetValue referencedProject.Id with
| true, (entry, fsRefProj) when entry.Version = version -> ValueSome fsRefProj
| _ -> ValueNone

let createPEReference (referencedProject: Project) (version: VersionStamp) (comp: Compilation) =
let entry = PEReferenceCacheEntry(version, comp)
let fsRefProj = buildPEReference referencedProject entry
peReferences.[referencedProject.Id] <- (entry, fsRefProj)
fsRefProj

let rec tryComputeOptionsBySingleScriptOrFile (document: Document) userOpName =
cancellableTask {
Expand Down Expand Up @@ -377,9 +421,13 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) =
FSharpReferencedProject.FSharpReference(referencedProject.OutputFilePath, projectOptions)
)
elif referencedProject.SupportsCompilation then
let! comp = referencedProject.GetCompilationAsync(ct)
let peRef = createPEReference referencedProject comp
referencedProjects.Add(peRef)
let! version = PEReferenceVersion.getAsync referencedProject ct

match tryGetPEReference referencedProject version with
Comment thread
T-Gro marked this conversation as resolved.
Resolved
| ValueSome peRef -> referencedProjects.Add peRef

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.

🤖🕵️ Stale source-generated API after a C# body edit — unchanged dependent semantic version despite different public declarations. Extracted cache functions with deployed Roslyn 5.12.0-1.26428.6: merge-base succeeds, HEAD gives FS0039. Key generator-backed references on their generation inputs.

using System.Linq;
using Microsoft.CodeAnalysis;

[Generator]
public sealed class BodyGenerator : ISourceGenerator
{
    public void Initialize(GeneratorInitializationContext context) { }
    public void Execute(GeneratorExecutionContext context)
    {
        var name = context.Compilation.SyntaxTrees.First().GetText().ToString().Contains("=> 1;") ? "Before" : "After";
        context.AddSource("Generated.cs", $"public class Generated {{ public static int {name} => 42; }}");
    }
}
// Referenced C# project, with that generator installed:
public class Input { public int M() => 1; } // edit only 1 -> 2
let result = Generated.After // FS0039 on HEAD, no diagnostics at merge-base

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 cfaf397: a referenced project that has analyzer references, or depends on a project that has them, is versioned by GetDependentVersionAsync, so any edit replaces its PE reference; a project without them stays on the dependent semantic version.

Analyzer references stand in for "has a generator": only Roslyn's out-of-process host knows which of them carry one, and asking in-process would load the generators into Visual Studio. The dependency part matters too: a generator-free C# project that takes a constant from a generated one went stale the same way.

Test: only a declaration edit moves the version, unless a source generator may run in PEReferenceVersionTests.fs.

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.

The manager-level test a C# project with an analyzer reference gets a new PE reference on every edit (185332d) now covers this through FSharpProjectOptionsManager as well, next to PEReferenceVersionTests for the version itself.

| ValueNone ->
let! comp = referencedProject.GetCompilationAsync(ct)
referencedProjects.Add(createPEReference referencedProject version comp)

if canBail then
return ValueNone
Expand Down Expand Up @@ -455,6 +503,11 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) =
if not (currentSolution.ContainsProject(pair.Key)) then
lastSuccessfulCompilations.TryRemove(pair.Key) |> ignore)

peReferences.ToArray()
|> Array.iter (fun pair ->
if not (currentSolution.ContainsProject(pair.Key)) then
peReferences.TryRemove(pair.Key) |> ignore)

checker.InvalidateConfiguration(projectOptions, userOpName = "tryComputeOptions")

let parsingOptions, _ = checker.GetParsingOptionsFromProjectOptions(projectOptions)
Expand Down Expand Up @@ -540,11 +593,11 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) =

| FSharpProjectOptionsMessage.ClearOptions(projectId) ->
match cache.TryRemove(projectId) with
| true, struct (_, _, projectOptions) ->
lastSuccessfulCompilations.TryRemove(projectId) |> ignore
checker.ClearCache([ projectOptions ])
| true, struct (_, _, projectOptions) -> checker.ClearCache([ projectOptions ])
| _ -> ()

Comment thread
xperiandri marked this conversation as resolved.
Resolved
lastSuccessfulCompilations.TryRemove(projectId) |> ignore
peReferences.TryRemove(projectId) |> ignore
legacyProjectSites.TryRemove(projectId) |> ignore
| FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) ->
match singleFileCache.TryRemove(documentId) with
Expand All @@ -553,7 +606,6 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) =
ProjectOptions = projectOptions
Subscription = subscription
} ->
lastSuccessfulCompilations.TryRemove(documentId.ProjectId) |> ignore
checker.ClearCache([ projectOptions ])
subscription |> ValueOption.iter (fun handler -> handler.Dispose())
| _ -> ()
Expand Down Expand Up @@ -591,6 +643,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) =
cache.Clear()
singleFileCache.Clear()
lastSuccessfulCompilations.Clear()
peReferences.Clear()

member _.ScriptUpdated = scriptUpdatedEvent.Publish

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
<Compile Include="InlineRenameTests.fs" />
<Compile Include="GoToDefinitionServiceTests.fs" />
<Compile Include="PathMapNavigationTests.fs" />
<Compile Include="PEReferenceVersionTests.fs" />
<Compile Include="PEReferenceCacheTests.fs" />
<Compile Include="HelpContextServiceTests.fs" />
<Compile Include="QuickInfoTests.fs" />
<Compile Include="TaskListServiceTests.fs" />
Expand Down Expand Up @@ -99,6 +101,7 @@
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
<PackageReference Include="Microsoft.CodeAnalysis.EditorFeatures" />

<PackageReference Include="Microsoft.VisualStudio.LanguageServices.ExternalAccess" />
Expand Down
Loading
Loading