Release project snapshots when their project leaves the workspace

docs/release-notes/.VisualStudio/18.vNext.md

@@ -21,6 +21,7 @@
 * Improve static compilation of state machines. ([PR #19297](https://github.com/dotnet/fsharp/pull/19297))
 * Make Alt+F1 (momentary toggle) work for inlay hints. ([PR #19421](https://github.com/dotnet/fsharp/pull/19421))
 * Fix doubled F# diagnostics in tooltips. ([Issue #16360](https://github.com/dotnet/fsharp/issues/16360))
+* Release the project snapshots the editor keeps for the transparent compiler when their project is removed or the solution is closed. Each one held the Roslyn solution it was built from, so every solution opened in a session stayed in memory until Visual Studio exited. ([PR #20762](https://github.com/dotnet/fsharp/pull/20762))
 * Fix `NotSupportedException` in the memory-mapped-file optimization when copying `ReadOnlyMemory` into `MemoryMappedFileViewStream`. ([Issue #20263](https://github.com/dotnet/fsharp/issues/20263))
 * Reduce allocations in the VS project options reactor: the command-line options and project options caches and the mailbox reply payloads now hold struct tuples, and `IProjectSite.CompilationBinOutputPath` returns `string voption` picked with a new `Array.tryPickV`. ([PR #20413](https://github.com/dotnet/fsharp/pull/20413))
 * Build a single-file project's `OtherOptions` reference flags with one array comprehension instead of two `Array.ofSeq` calls and an `Array.append`. ([PR #20499](https://github.com/dotnet/fsharp/pull/20499))

vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs

@@ -220,6 +220,16 @@ type internal FSharpWorkspaceServiceFactory
 
             let optionsManager = FSharpProjectOptionsManager(checker, workspace)
 
+            workspace.WorkspaceChanged.Add(fun args ->
+                match args.Kind with
+                | WorkspaceChangeKind.ProjectRemoved -> ProjectSnapshotCache.removeProject args.ProjectId
+                | WorkspaceChangeKind.SolutionRemoved
+                | WorkspaceChangeKind.SolutionCleared
+                | WorkspaceChangeKind.SolutionReloaded ->
+                    for projectId in args.OldSolution.ProjectIds do
+                        ProjectSnapshotCache.removeProject projectId
+                | _ -> ())
+
             { new IFSharpWorkspaceService with
                 member _.Checker = checker
                 member _.FSharpProjectOptionsManager = optionsManager
@@ -235,6 +245,7 @@ type private FSharpSolutionEvents(projectManager: FSharpProjectOptionsManager, m
             projectManager.Checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients()
             metadataAsSource.ClearGeneratedFiles()
             projectManager.ClearAllCaches()
+            ProjectSnapshotCache.clear ()
             VSConstants.S_OK
 
         member _.OnAfterLoadProject(_, _) = VSConstants.E_NOTIMPL

vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs

@@ -161,12 +161,40 @@ module internal FSharpProjectSnapshotSerialization =
 open FSharpProjectSnapshotSerialization
 open System.Collections.Concurrent
 
-[<AutoOpen>]
-module private CheckerExtensions =
+/// Every snapshot roots the `Project`, and through it the `Solution`, it was built from: the entries follow the workspace.
+[<RequireQualifiedAccess>]
+module internal ProjectSnapshotCache =
+
+    let snapshots =
+        AsyncMemoize<ProjectId, Project, FSharpProjectSnapshot>(1000, 500, "SnapshotCache")
+
+    let latest =
+        ConcurrentDictionary<ProjectId, Project * VersionStamp * Set<string> * FSharpProjectOptions * FSharpProjectSnapshot>()
+
+    let mutable private removals = 0
+
+    let removalsSoFar () =
+        System.Threading.Volatile.Read &removals
+
+    let forget (projectId: ProjectId) =
+        snapshots.Clear(fun key -> key = projectId)
+        latest.TryRemove projectId |> ignore
+
+    let removeProject (projectId: ProjectId) =
+        System.Threading.Interlocked.Increment &removals |> ignore
+        forget projectId
 
-    let snapshotCache = AsyncMemoize(1000, 500, "SnapshotCache")
+    let clear () =
+        System.Threading.Interlocked.Increment &removals |> ignore
+        snapshots.Clear()
+        latest.Clear()
 
-    let latestSnapshots = ConcurrentDictionary<_, _>()
+    let isGone (project: Project) removalsAtStart =
+        removalsSoFar () <> removalsAtStart
+        || not (project.Solution.Workspace.CurrentSolution.ContainsProject project.Id)
+
+[<AutoOpen>]
+module private CheckerExtensions =
 
     let exist xs = xs |> Seq.isEmpty |> not
 
@@ -224,7 +252,7 @@ module private CheckerExtensions =
             })
         |> Seq.toList
 
-    let createProjectSnapshot (snapshotAccumulatorOpt) (project: Project) (options: FSharpProjectOptions option) =
+    let createProjectSnapshot removalsAtStart (snapshotAccumulatorOpt) (project: Project) (options: FSharpProjectOptions option) =
         cancellableTask {
 
             let! options =
@@ -241,7 +269,7 @@ module private CheckerExtensions =
             let! referenceVersions = getReferencedProjectVersions project
 
             let updatedSnapshot =
-                match project.IsTransparentCompilerSnapshotReuseEnabled, latestSnapshots.TryGetValue project.Id with
+                match project.IsTransparentCompilerSnapshotReuseEnabled, ProjectSnapshotCache.latest.TryGetValue project.Id with
                 | true, (true, (_, _, oldReferenceVersions, _, _)) when referenceVersions <> oldReferenceVersions ->
                     System.Diagnostics.Trace.TraceWarning "Reference versions changed"
                     None
@@ -362,8 +390,9 @@ module private CheckerExtensions =
             let latestSnapshotData =
                 project, projectVersion, referenceVersions, options, newSnapshot
 
-            latestSnapshots.AddOrUpdate(project.Id, latestSnapshotData, (fun _ _ -> latestSnapshotData))
-            |> ignore
+            if not (ProjectSnapshotCache.isGone project removalsAtStart) then
+                ProjectSnapshotCache.latest.AddOrUpdate(project.Id, latestSnapshotData, (fun _ _ -> latestSnapshotData))
+                |> ignore
 
             return newSnapshot
         }
@@ -377,16 +406,25 @@ module private CheckerExtensions =
                 member _.GetLabel() = project.FilePath
             }
 
-        snapshotCache.Get(
-            key,
-            async {
-                let! ct = Async.CancellationToken
+        async {
+            let removalsAtStart = ProjectSnapshotCache.removalsSoFar ()
 
+            try
                 return!
-                    createProjectSnapshot snapshotAccumulatorOpt project options ct
-                    |> Async.AwaitTask
-            }
-        )
+                    ProjectSnapshotCache.snapshots.Get(
+                        key,
+                        async {
+                            let! ct = Async.CancellationToken
+
+                            return!
+                                createProjectSnapshot removalsAtStart snapshotAccumulatorOpt project options ct
+                                |> Async.AwaitTask
+                        }
+                    )
+            finally
+                if ProjectSnapshotCache.isGone project removalsAtStart then
+                    ProjectSnapshotCache.forget project.Id
+        }
 
     let getProjectSnapshotForDocument (document: Document, options: FSharpProjectOptions) =
         getOrCreateSnapshotForProject document.Project (Some options) None

vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj

@@ -89,6 +89,7 @@
     <Compile Include="RoslynSourceTextTests.fs" />
     <Compile Include="SemanticClassificationServiceTests.fs" />
     <Compile Include="SyntacticColorizationServiceTests.fs" />
+    <Compile Include="ProjectSnapshotCacheTests.fs" />
     <Compile Include="DocumentHighlightsServiceTests.fs" />
   </ItemGroup>
 

vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs

@@ -0,0 +1,127 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+module FSharp.Editor.Tests.ProjectSnapshotCacheTests
+
+open System
+open System.Threading
+open System.Threading.Tasks
+open Xunit
+open Microsoft.CodeAnalysis
+open Microsoft.CodeAnalysis.Text
+open Microsoft.VisualStudio.FSharp.Editor
+open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks
+open FSharp.Editor.Tests.Helpers
+
+type private GatedTextLoader(text: string, release: Task) =
+    inherit TextLoader()
+
+    let asked =
+        TaskCompletionSource<unit>(TaskCreationOptions.RunContinuationsAsynchronously)
+
+    member _.Asked = asked.Task
+
+    override _.LoadTextAndVersionAsync(_options: LoadTextOptions, _cancellationToken: CancellationToken) : Task<TextAndVersion> =
+        task {
+            asked.TrySetResult() |> ignore
+            do! release
+            return TextAndVersion.Create(SourceText.From text, VersionStamp.Create())
+        }
+
+let private createProject () =
+    RoslynTestHelpers.CreateSolution("module Test").Projects |> Seq.exactlyOne
+
+let private createProjectWithGatedText (release: Task) =
+    let projectId = ProjectId.CreateNewId()
+    let loader = GatedTextLoader("module Test", release)
+
+    let document =
+        DocumentInfo.Create(DocumentId.CreateNewId projectId, "test.fs", loader = loader, filePath = "C:\\test.fs")
+
+    let projectInfo =
+        RoslynTestHelpers.CreateProjectInfo projectId "C:\\test.fsproj" [ document ]
+
+    let solution = RoslynTestHelpers.CreateSolution [ projectInfo ]
+    RoslynTestHelpers.SetProjectOptions projectId solution RoslynTestHelpers.DefaultProjectOptions
+    loader, projectInfo, solution.GetProject projectId
+
+let private snapshotOf (project: Project) =
+    project.GetFSharpProjectSnapshot()
+    |> CancellableTask.start CancellationToken.None
+
+let private waitForRemovalsAfter (removalsBefore: int) (projectId: ProjectId) =
+    let removed () =
+        ProjectSnapshotCache.removalsSoFar () <> removalsBefore
+        && not (ProjectSnapshotCache.latest.ContainsKey projectId)
+
+    Assert.True(SpinWait.SpinUntil(removed, TimeSpan.FromSeconds 60.))
+
+let private removeFromWorkspace (project: Project) =
+    let workspace = project.Solution.Workspace
+    let removalsBefore = ProjectSnapshotCache.removalsSoFar ()
+    Assert.True(workspace.TryApplyChanges(workspace.CurrentSolution.RemoveProject project.Id))
+    waitForRemovalsAfter removalsBefore project.Id
+
+let private reloadWorkspace (workspace: AdhocWorkspace) (projectInfo: ProjectInfo) =
+    let removalsBefore = ProjectSnapshotCache.removalsSoFar ()
+    workspace.ClearSolution()
+
+    workspace.AddSolution(SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create(), "test.sln", [ projectInfo ]))
+    |> ignore
+
+    waitForRemovalsAfter removalsBefore projectInfo.Id
+
+/// Runs `disturb` while the project's first snapshot request waits for the text of its document.
+let private snapshotBuiltWhile (disturb: Project -> ProjectInfo -> unit) =
+    task {
+        let release =
+            TaskCompletionSource<unit>(TaskCreationOptions.RunContinuationsAsynchronously)
+
+        let loader, projectInfo, project = createProjectWithGatedText release.Task
+        let pending = Task.Run(Func<Task<_>>(fun () -> snapshotOf project))
+        do! loader.Asked
+
+        disturb project projectInfo
+        release.SetResult()
+
+        let! built = pending
+        return project, box built
+    }
+
+let private assertForgotten (project: Project) (previous: obj) =
+    task {
+        Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id)
+
+        let! rebuilt = snapshotOf project
+        Assert.NotSame(previous, rebuilt)
+    }
+
+[<Fact>]
+let ``A project's snapshots are forgotten when it leaves the workspace, and a later request keeps none`` () : Task =
+    task {
+        let project = createProject ()
+
+        let! first = snapshotOf project
+        let! again = snapshotOf project
+        Assert.Same(first, again)
+
+        removeFromWorkspace project
+        do! assertForgotten project first
+        Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id)
+    }
+
+[<Fact>]
+let ``A snapshot being built when its project leaves the workspace is not kept`` () : Task =
+    task {
+        let! project, built = snapshotBuiltWhile (fun project _ -> removeFromWorkspace project)
+        do! assertForgotten project built
+    }
+
+[<Fact>]
+let ``A snapshot being built when the solution is reloaded under the same project ids is not kept`` () : Task =
+    task {
+        let! project, built =
+            snapshotBuiltWhile (fun project projectInfo -> reloadWorkspace (project.Solution.Workspace :?> AdhocWorkspace) projectInfo)
+
+        Assert.True(project.Solution.Workspace.CurrentSolution.ContainsProject project.Id)
+        do! assertForgotten project built
+    }