diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2357fbcdff4..e7685c41707 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/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)) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs index ab876679a77..c86c5ca27f1 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs +++ b/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 diff --git a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs index 2406f3a6e32..0ed5c8ef14c 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -161,12 +161,40 @@ module internal FSharpProjectSnapshotSerialization = open FSharpProjectSnapshotSerialization open System.Collections.Concurrent -[] -module private CheckerExtensions = +/// Every snapshot roots the `Project`, and through it the `Solution`, it was built from: the entries follow the workspace. +[] +module internal ProjectSnapshotCache = + + let snapshots = + AsyncMemoize(1000, 500, "SnapshotCache") + + let latest = + ConcurrentDictionary * 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) + +[] +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 diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index d5d1a09c79a..0e881ad7a77 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -89,6 +89,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs new file mode 100644 index 00000000000..1febfe81458 --- /dev/null +++ b/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(TaskCreationOptions.RunContinuationsAsynchronously) + + member _.Asked = asked.Task + + override _.LoadTextAndVersionAsync(_options: LoadTextOptions, _cancellationToken: CancellationToken) : Task = + 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(TaskCreationOptions.RunContinuationsAsynchronously) + + let loader, projectInfo, project = createProjectWithGatedText release.Task + let pending = Task.Run(Func>(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) + } + +[] +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) + } + +[] +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 + } + +[] +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 + }