diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index cffa42edc9c..7ab7c786e82 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -7,6 +7,8 @@ * 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)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) +* Reduce excessive metadata-only `Compilation.Emit` churn for C# project references in the IDE. ([Issue #20118](https://github.com/dotnet/fsharp/issues/20118), [PR #20119](https://github.com/dotnet/fsharp/pull/20119)) +* Avoid blocking the VS project-options mailbox and unnecessary script option recomputation while editing by making invalidation async and debouncing caret-driven updates. ([Issue #20124](https://github.com/dotnet/fsharp/issues/20124), [Issue #20125](https://github.com/dotnet/fsharp/issues/20125), [PR #20126](https://github.com/dotnet/fsharp/pull/20126)) * Find All References for external DLL symbols now only searches projects that reference the specific assembly. ([Issue #10227](https://github.com/dotnet/fsharp/issues/10227), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * 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)) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 08bfbbddaa8..36ab7c7b8e7 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. namespace Microsoft.VisualStudio.FSharp.Editor @@ -8,19 +8,17 @@ open System.Collections.Concurrent open System.Collections.Immutable open System.IO open System.Linq +open System.Runtime.CompilerServices +open System.Threading +open System.Threading.Tasks open Microsoft.CodeAnalysis open FSharp.Compiler open FSharp.Compiler.CodeAnalysis +open FSharp.Compiler.Text 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 -open Microsoft.VisualStudio -open FSharp.Compiler.Text open Microsoft.VisualStudio.TextManager.Interop +open CancellableTasks #nowarn "57" @@ -40,7 +38,7 @@ module private FSharpProjectOptionsHelpers = member _.CompilationSourceFiles = sourcePaths member _.CompilationOptions = - Array.concat [ options; referencePaths |> Array.map (fun r -> "-r:" + r) ] + [| yield! options; yield! referencePaths |> Seq.map (fun r -> "-r:" + r) |] member _.CompilationReferences = referencePaths @@ -66,40 +64,74 @@ module private FSharpProjectOptionsHelpers = let inline hasProjectVersionChanged (oldProject: Project) (newProject: Project) = oldProject.Version <> newProject.Version - let hasDependentVersionChanged (oldProject: Project) (newProject: Project) (ct: CancellationToken) = - let oldProjectMetadataRefs = oldProject.MetadataReferences - let newProjectMetadataRefs = newProject.MetadataReferences + let hasDependentVersionChanged (oldProject: Project) (newProject: Project) = + cancellableTask { + let! ct = CancellableTask.getCancellationToken () + let oldProjectMetadataRefs = oldProject.MetadataReferences + let newProjectMetadataRefs = newProject.MetadataReferences - if oldProjectMetadataRefs.Count <> newProjectMetadataRefs.Count then - true - else + if oldProjectMetadataRefs.Count <> newProjectMetadataRefs.Count then + return true + else - let oldProjectRefs = oldProject.ProjectReferences - let newProjectRefs = newProject.ProjectReferences - - oldProjectRefs.Count() <> newProjectRefs.Count() - || (oldProjectRefs, newProjectRefs) - ||> Seq.exists2 (fun p1 p2 -> - ct.ThrowIfCancellationRequested() - let doesProjectIdDiffer = p1.ProjectId <> p2.ProjectId - let p1 = oldProject.Solution.GetProject(p1.ProjectId) - let p2 = newProject.Solution.GetProject(p2.ProjectId) - - doesProjectIdDiffer - || (if p1.IsFSharp then - p1.Version <> p2.Version - else - let v1 = p1.GetDependentVersionAsync(ct).Result - let v2 = p2.GetDependentVersionAsync(ct).Result - v1 <> v2)) - - let isProjectInvalidated (oldProject: Project) (newProject: Project) ct = - let hasProjectVersionChanged = hasProjectVersionChanged oldProject newProject - - if newProject.AreFSharpInMemoryCrossProjectReferencesEnabled then - hasProjectVersionChanged || hasDependentVersionChanged oldProject newProject ct - else - hasProjectVersionChanged + let oldProjectRefs = oldProject.ProjectReferences + let newProjectRefs = newProject.ProjectReferences + + if oldProjectRefs.Count() <> newProjectRefs.Count() then + return true + else + let mutable result = false + let mutable enum1 = oldProjectRefs.GetEnumerator() + let mutable enum2 = newProjectRefs.GetEnumerator() + + while not result && enum1.MoveNext() && enum2.MoveNext() do + ct.ThrowIfCancellationRequested() + let p1 = enum1.Current + let p2 = enum2.Current + let doesProjectIdDiffer = p1.ProjectId <> p2.ProjectId + let p1 = oldProject.Solution.GetProject(p1.ProjectId) + let p2 = newProject.Solution.GetProject(p2.ProjectId) + + if doesProjectIdDiffer then + result <- true + elif p1.IsFSharp then + result <- p1.Version <> p2.Version + else + let! v1 = p1.GetDependentVersionAsync(ct) + let! v2 = p2.GetDependentVersionAsync(ct) + result <- v1 <> v2 + + return result + } + + let isProjectInvalidated (oldProject: Project) (newProject: Project) = + cancellableTask { + let hasProjectVersionChanged = hasProjectVersionChanged oldProject newProject + + if newProject.AreFSharpInMemoryCrossProjectReferencesEnabled then + if hasProjectVersionChanged then + return true + else + return! hasDependentVersionChanged oldProject newProject + else + return hasProjectVersionChanged + } + +type SingleFileCacheEntry = + { + Project: Project + FileStamp: VersionStamp + ParsingOptions: FSharpParsingOptions + ProjectOptions: FSharpProjectOptions + Subscription: ConnectionPointSubscription + } + +type PEReferenceCacheEntry = + { + Stamp: VersionStamp + Compilation: Compilation + Reference: FSharpReferencedProject + } [] type private FSharpProjectOptionsMessage = @@ -124,77 +156,103 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let cache = ConcurrentDictionary() - let singleFileCache = - ConcurrentDictionary() + let singleFileCache = ConcurrentDictionary() - // This is used to not constantly emit the same compilation. + let emitCache = ConcurrentDictionary() let weakPEReferences = ConditionalWeakTable() let lastSuccessfulCompilations = ConcurrentDictionary() let scriptUpdatedEvent = Event() - let createPEReference (referencedProject: Project) (comp: Compilation) = - let projectId = referencedProject.Id + let disposeSingleFileCacheEntry ({ Subscription = subscription }: SingleFileCacheEntry) = + subscription |> Option.iter (fun subscription -> subscription.Dispose()) + + let tryGetCachedPEReference projectId stamp comp = + match emitCache.TryGetValue(projectId) with + | true, + { + Stamp = cachedStamp + Compilation = cachedCompilation + Reference = fsRefProj + } when cachedStamp = stamp && obj.ReferenceEquals(cachedCompilation, comp) -> ValueSome fsRefProj + | _ -> ValueNone + + let cachePEReference projectId stamp comp fsRefProj = + emitCache.[projectId] <- + { + Stamp = stamp + Compilation = comp + Reference = fsRefProj + } - match weakPEReferences.TryGetValue comp with - | true, fsRefProj -> fsRefProj - | _ -> - let mutable strongComp = comp - let weakComp = WeakReference(comp) - let mutable stamp = DateTime.UtcNow + fsRefProj - // 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 createNewPEReference projectId (referencedProject: Project) (comp: Compilation) = + let mutable strongComp = comp + let weakComp = WeakReference(comp) + let mutable stampTime = DateTime.UtcNow - let emitOptions = - Emit.EmitOptions(metadataOnly = true, includePrivateMembers = false, tolerateErrors = true) + let getStream = + fun ct -> + let tryStream (comp: Compilation) = + let ms = new MemoryStream() - try - let result = comp.Emit(ms, options = emitOptions, cancellationToken = ct) + let emitOptions = + Emit.EmitOptions(metadataOnly = true, includePrivateMembers = false, tolerateErrors = true) - 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 + try + let result = comp.Emit(ms, options = emitOptions, cancellationToken = ct) + + if result.Success then + strongComp <- Unchecked.defaultof<_> + lastSuccessfulCompilations.[projectId] <- comp + ms.Position <- 0L + ms :> Stream |> Some + else + strongComp <- Unchecked.defaultof<_> ms.Dispose() None - | _ -> - strongComp <- Unchecked.defaultof<_> // Stop strongly holding the compilation since we have a result. - ms.Dispose() // it failed, dispose of stream - None - - let resultOpt = - match weakComp.TryGetTarget() with - | true, comp -> tryStream comp - | _ -> None - - match resultOpt with - | Some _ -> resultOpt + with + | :? OperationCanceledException -> + stampTime <- DateTime.UtcNow + ms.Dispose() + None | _ -> - match lastSuccessfulCompilations.TryGetValue(projectId) with - | true, comp -> tryStream comp - | _ -> None + strongComp <- Unchecked.defaultof<_> + ms.Dispose() + None + + let resultOpt = + match weakComp.TryGetTarget() with + | true, comp -> tryStream comp + | _ -> None - let getStamp = fun () -> stamp + match resultOpt with + | Some _ -> resultOpt + | _ -> + match lastSuccessfulCompilations.TryGetValue(projectId) with + | true, comp -> tryStream comp + | _ -> None - let fsRefProj = - FSharpReferencedProject.PEReference(getStamp, DelayedILModuleReader(referencedProject.OutputFilePath, getStream)) + let getStampTime () = stampTime + FSharpReferencedProject.PEReference(getStampTime, DelayedILModuleReader(referencedProject.OutputFilePath, getStream)) - weakPEReferences.Add(comp, fsRefProj) - fsRefProj + let createPEReference (referencedProject: Project) (comp: Compilation) = + cancellableTask { + let! ct = CancellableTask.getCancellationToken () + let projectId = referencedProject.Id + let! stamp = referencedProject.GetDependentVersionAsync(ct) + + match tryGetCachedPEReference projectId stamp comp with + | ValueSome fsRefProj -> return fsRefProj + | ValueNone -> + match weakPEReferences.TryGetValue comp with + | true, fsRefProj -> return cachePEReference projectId stamp comp fsRefProj + | _ -> + let fsRefProj = createNewPEReference projectId referencedProject comp + weakPEReferences.Add(comp, fsRefProj) + return cachePEReference projectId stamp comp fsRefProj + } let rec tryComputeOptionsBySingleScriptOrFile (document: Document) userOpName = cancellableTask { @@ -234,14 +292,13 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let otherOptions = if project.IsFSharpMetadata then - project.ProjectReferences - |> Seq.map (fun x -> "-r:" + project.Solution.GetProject(x.ProjectId).OutputFilePath) - |> Array.ofSeq - |> Array.append ( - project.MetadataReferences.OfType() - |> Seq.map (fun x -> "-r:" + x.FilePath) - |> Array.ofSeq - ) + [| + for projectReference in project.ProjectReferences do + "-r:" + project.Solution.GetProject(projectReference.ProjectId).OutputFilePath + + for metadataReference in project.MetadataReferences.OfType() do + "-r:" + metadataReference.FilePath + |] else [||] @@ -266,44 +323,107 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let parsingOptions, _ = checker.GetParsingOptionsFromProjectOptions(projectOptions) + let mutable debounceCts: CancellationTokenSource | null = null + + let disposeDebounceCts () = + match Interlocked.Exchange(&debounceCts, null) with + | null -> () + | cts -> + cts.Cancel() + cts.Dispose() + let updateProjectOptions () = - async { - let! scriptProjectOptions, _ = getProjectOptionsFromScript textViewAndCaret + let cts = new CancellationTokenSource() + let debounceToken = cts.Token + + match Interlocked.Exchange(&debounceCts, cts) with + | null -> () + | previousCts -> + previousCts.Cancel() + previousCts.Dispose() + + task { + try + do! Task.Delay(500, debounceToken) - checker.NotifyFileChanged(document.FilePath, scriptProjectOptions) - |> Async.Start + let! scriptProjectOptions, _ = getProjectOptionsFromScript textViewAndCaret + + do! checker.NotifyFileChanged(document.FilePath, scriptProjectOptions) + with + | :? OperationCanceledException + | :? TaskCanceledException -> () } - |> Async.Start + |> ignore let onChangeCaretHandler (_, _newline: int, _oldline: int) = updateProjectOptions () let onKillFocus (_) = updateProjectOptions () let onSetFocus (_) = updateProjectOptions () - let addToCacheAndSubscribe value = - match value with - | projectId, fileStamp, parsingOptions, projectOptions, _ -> - let subscription = - match textViewAndCaret () with - | Some(textView, _) -> - subscribeToTextViewEvents (textView, (Some onChangeCaretHandler), (Some onKillFocus), (Some onSetFocus)) - | None -> None + let addToCacheAndSubscribe + ({ + Project = _ + FileStamp = fileStamp + ParsingOptions = parsingOptions + ProjectOptions = projectOptions + Subscription = _ + }: SingleFileCacheEntry) + = + let textViewSubscription = + match textViewAndCaret () with + | Some(textView, _) -> + subscribeToTextViewEvents (textView, (Some onChangeCaretHandler), (Some onKillFocus), (Some onSetFocus)) + | None -> None + + let subscription = + Some + { new IDisposable with + member _.Dispose() = + textViewSubscription |> Option.iter (fun subscription -> subscription.Dispose()) + disposeDebounceCts () + } - (projectId, fileStamp, parsingOptions, projectOptions, subscription) + { + Project = document.Project + FileStamp = fileStamp + ParsingOptions = parsingOptions + ProjectOptions = projectOptions + Subscription = subscription + } singleFileCache.AddOrUpdate( - document.Id, // The key to the cache - (fun _ value -> addToCacheAndSubscribe value), // Function to add the cached value if the key does not exist - (fun _ _ value -> value), // Function to update the value if the key exists - (document.Project, fileStamp, parsingOptions, projectOptions, None) // The value to add or update + document.Id, + (fun _ -> + addToCacheAndSubscribe + { + Project = document.Project + FileStamp = fileStamp + ParsingOptions = parsingOptions + ProjectOptions = projectOptions + Subscription = None + }), + (fun _ existing -> addToCacheAndSubscribe existing) ) |> ignore return ValueSome(parsingOptions, projectOptions) - | true, (oldProject, oldFileStamp, parsingOptions, projectOptions, _) -> - if fileStamp <> oldFileStamp || isProjectInvalidated document.Project oldProject ct then + | true, + { + Project = oldProject + FileStamp = oldFileStamp + ParsingOptions = parsingOptions + ProjectOptions = projectOptions + Subscription = _ + } -> + let! isInvalidated = + if fileStamp <> oldFileStamp then + CancellableTask.singleton true + else + isProjectInvalidated document.Project oldProject + + if isInvalidated then match singleFileCache.TryRemove(document.Id) with - | true, (_, _, _, _, Some subscription) -> subscription.Dispose() + | true, cacheEntry -> disposeSingleFileCacheEntry cacheEntry | _ -> () return! tryComputeOptionsBySingleScriptOrFile document userOpName @@ -348,7 +468,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = ) elif referencedProject.SupportsCompilation then let! comp = referencedProject.GetCompilationAsync(ct) - let peRef = createPEReference referencedProject comp + let! peRef = createPEReference referencedProject comp referencedProjects.Add(peRef) if canBail then @@ -418,10 +538,10 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = checker.ClearCache(options, userOpName = "tryComputeOptions") - lastSuccessfulCompilations.ToArray() - |> Array.iter (fun pair -> + for pair in lastSuccessfulCompilations.ToArray() do if not (currentSolution.ContainsProject(pair.Key)) then - lastSuccessfulCompilations.TryRemove(pair.Key) |> ignore) + lastSuccessfulCompilations.TryRemove(pair.Key) |> ignore + emitCache.TryRemove(pair.Key) |> ignore checker.InvalidateConfiguration(projectOptions, userOpName = "tryComputeOptions") @@ -432,9 +552,11 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = return ValueSome(parsingOptions, projectOptions) | true, (oldProject, parsingOptions, projectOptions) -> - if isProjectInvalidated oldProject project ct then + let! isInvalidated = isProjectInvalidated oldProject project + + if isInvalidated then cache.TryRemove(projectId) |> ignore - return! tryComputeOptions project ct + return! tryComputeOptions project else return ValueSome(parsingOptions, projectOptions) } @@ -510,17 +632,20 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = match cache.TryRemove(projectId) with | true, (_, _, projectOptions) -> lastSuccessfulCompilations.TryRemove(projectId) |> ignore + emitCache.TryRemove(projectId) |> ignore checker.ClearCache([ projectOptions ]) | _ -> () legacyProjectSites.TryRemove(projectId) |> ignore | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with - | true, (_, _, _, projectOptions, subscription) -> + | true, ({ ProjectOptions = projectOptions } as cacheEntry) -> lastSuccessfulCompilations.TryRemove(documentId.ProjectId) |> ignore + emitCache.TryRemove(documentId.ProjectId) |> ignore checker.ClearCache([ projectOptions ]) - subscription |> Option.iter (fun handler -> handler.Dispose()) + disposeSingleFileCacheEntry cacheEntry | _ -> () + } let agent = @@ -553,13 +678,16 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = commandLineOptions.Clear() legacyProjectSites.Clear() cache.Clear() + singleFileCache.Values |> Seq.iter disposeSingleFileCacheEntry singleFileCache.Clear() lastSuccessfulCompilations.Clear() + emitCache.Clear() member _.ScriptUpdated = scriptUpdatedEvent.Publish interface IDisposable with - member _.Dispose() = + member this.Dispose() = + this.ClearAllCaches() cancellationTokenSource.Cancel() cancellationTokenSource.Dispose() (agent :> IDisposable).Dispose()