From 5f2ad8064c4b4b4794dfd58d26faef104b242480 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 3 Aug 2026 01:20:17 +0200 Subject: [PATCH 1/5] Add version-stamp cache to FSharpDocumentDiagnosticAnalyzer to avoid recomputing diagnostics when document/project version is unchanged --- .../Diagnostics/DocumentDiagnosticAnalyzer.fs | 77 +++++++++++++------ 1 file changed, 53 insertions(+), 24 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs index a1a2bbe9f90..3cb80f18e26 100644 --- a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs +++ b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs @@ -1,8 +1,9 @@ -// 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 open System.Composition +open System.Collections.Concurrent open System.Collections.Immutable open System.Collections.Generic open System.Threading @@ -27,6 +28,9 @@ type internal FSharpDocumentDiagnosticAnalyzer [] () = let shouldProduceDiagnostics (document: Document) = document.Project.Solution.GetFSharpExtensionConfig().ShouldProduceDiagnostics() + static let cache = + ConcurrentDictionary>() + static let diagnosticEqualityComparer = { new IEqualityComparer with @@ -72,6 +76,27 @@ type internal FSharpDocumentDiagnosticAnalyzer [] () = let! ct = CancellableTask.getCancellationToken () + let! textVersion = document.GetTextVersionAsync(ct) + + let! projectVersion = + match diagnosticType with + | DiagnosticsType.Syntax -> CancellableTask.singleton VersionStamp.Default + | DiagnosticsType.Semantic -> (fun ct -> document.Project.GetDependentVersionAsync(ct)) + + let cacheKey = struct (document.Id, diagnosticType) + + let cached = + match cache.TryGetValue(cacheKey) with + | true, (cachedTextVersion, cachedProjectVersion, cachedDiagnostics) when + cachedTextVersion = textVersion && cachedProjectVersion = projectVersion + -> + ValueSome cachedDiagnostics + | _ -> ValueNone + + match cached with + | ValueSome cachedDiagnostics -> return cachedDiagnostics + | ValueNone -> + let! sourceText = document.GetTextAsync(ct) let filePath = document.FilePath @@ -98,35 +123,39 @@ type internal FSharpDocumentDiagnosticAnalyzer [] () = UnnecessaryParenthesesDiagnosticAnalyzer.GetDiagnostics document | _ -> CancellableTask.singleton ImmutableArray.Empty - if errors.Count = 0 && unnecessaryParentheses.IsEmpty then - return ImmutableArray.Empty - else - let iab = ImmutableArray.CreateBuilder(errors.Count + unnecessaryParentheses.Length) + let result = + if errors.Count = 0 && unnecessaryParentheses.IsEmpty then + ImmutableArray.Empty + else + let iab = ImmutableArray.CreateBuilder(errors.Count + unnecessaryParentheses.Length) + + for diagnostic in errors do + if diagnostic.StartLine <> 0 && diagnostic.EndLine <> 0 then + let linePositionSpan = + LinePositionSpan( + LinePosition(diagnostic.StartLine - 1, diagnostic.StartColumn), + LinePosition(diagnostic.EndLine - 1, diagnostic.EndColumn) + ) - for diagnostic in errors do - if diagnostic.StartLine <> 0 && diagnostic.EndLine <> 0 then - let linePositionSpan = - LinePositionSpan( - LinePosition(diagnostic.StartLine - 1, diagnostic.StartColumn), - LinePosition(diagnostic.EndLine - 1, diagnostic.EndColumn) - ) + let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan) - let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan) + // F# compiler report errors at end of file if parsing fails. It should be corrected to match Roslyn boundaries + let correctedTextSpan = + if textSpan.End <= sourceText.Length then + textSpan + else + let start = min textSpan.Start (sourceText.Length - 1) |> max 0 - // F# compiler report errors at end of file if parsing fails. It should be corrected to match Roslyn boundaries - let correctedTextSpan = - if textSpan.End <= sourceText.Length then - textSpan - else - let start = min textSpan.Start (sourceText.Length - 1) |> max 0 + TextSpan.FromBounds(start, sourceText.Length) - TextSpan.FromBounds(start, sourceText.Length) + let location = Location.Create(filePath, correctedTextSpan, linePositionSpan) + iab.Add(RoslynHelpers.ConvertError(diagnostic, location)) - let location = Location.Create(filePath, correctedTextSpan, linePositionSpan) - iab.Add(RoslynHelpers.ConvertError(diagnostic, location)) + iab.AddRange unnecessaryParentheses + iab.ToImmutable() - iab.AddRange unnecessaryParentheses - return iab.ToImmutable() + cache.[cacheKey] <- (textVersion, projectVersion, result) + return result } interface IFSharpDocumentDiagnosticAnalyzer with From fed57cb0598d1e1397f2b36ea977e08c0f8c9e2d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 13 Aug 2026 23:05:23 +0200 Subject: [PATCH 2/5] Perf: Refactor diagnostics cache with metadata record * Refactored the diagnostics cache in `FSharpDocumentDiagnosticAnalyzer` to use a new `CachedDiagnosticsEntry` record, storing `TextVersion`, `ProjectVersion`, `FilePath`, `IsRemoveParensEnabled`, and cached `Diagnostics`. * The cache key remains `(DocumentId * DiagnosticsType)`, but the value is now the new record. Cache lookup now checks all relevant fields for equality, ensuring diagnostics are reused only when context matches. * Added `evictRemovedDocuments` to remove cache entries for deleted documents. * Updated logic for "Remove Parentheses" diagnostics to use the cached flag, and updated cache storage to the new structure. --- .../Diagnostics/DocumentDiagnosticAnalyzer.fs | 146 +++++++++++------- 1 file changed, 90 insertions(+), 56 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs index 3cb80f18e26..9d0c8ca66f5 100644 --- a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs +++ b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs @@ -22,6 +22,15 @@ type internal DiagnosticsType = | Syntax | Semantic +type private CachedDiagnosticsEntry = + { + TextVersion: VersionStamp + ProjectVersion: VersionStamp + FilePath: string + IsRemoveParensEnabled: bool + Diagnostics: ImmutableArray + } + [)>] type internal FSharpDocumentDiagnosticAnalyzer [] () = @@ -29,7 +38,14 @@ type internal FSharpDocumentDiagnosticAnalyzer [] () = document.Project.Solution.GetFSharpExtensionConfig().ShouldProduceDiagnostics() static let cache = - ConcurrentDictionary>() + ConcurrentDictionary() + + static let evictRemovedDocuments (solution: Solution) = + for entry in cache do + let struct (documentId, _) = entry.Key + + if isNull (solution.GetDocument(documentId)) then + cache.TryRemove(entry.Key) |> ignore static let diagnosticEqualityComparer = { new IEqualityComparer with @@ -83,79 +99,97 @@ type internal FSharpDocumentDiagnosticAnalyzer [] () = | DiagnosticsType.Syntax -> CancellableTask.singleton VersionStamp.Default | DiagnosticsType.Semantic -> (fun ct -> document.Project.GetDependentVersionAsync(ct)) + let filePath = document.FilePath + + let isRemoveParensEnabled = + match diagnosticType with + | DiagnosticsType.Syntax -> document.Project.IsFsharpRemoveParensEnabled + | DiagnosticsType.Semantic -> false + + evictRemovedDocuments document.Project.Solution + let cacheKey = struct (document.Id, diagnosticType) let cached = match cache.TryGetValue(cacheKey) with - | true, (cachedTextVersion, cachedProjectVersion, cachedDiagnostics) when - cachedTextVersion = textVersion && cachedProjectVersion = projectVersion + | true, cachedEntry when + cachedEntry.TextVersion = textVersion + && cachedEntry.ProjectVersion = projectVersion + && cachedEntry.FilePath = filePath + && cachedEntry.IsRemoveParensEnabled = isRemoveParensEnabled -> - ValueSome cachedDiagnostics + ValueSome cachedEntry.Diagnostics | _ -> ValueNone match cached with | ValueSome cachedDiagnostics -> return cachedDiagnostics | ValueNone -> - let! sourceText = document.GetTextAsync(ct) - let filePath = document.FilePath + let! sourceText = document.GetTextAsync(ct) - let errors = HashSet(diagnosticEqualityComparer) + let errors = HashSet(diagnosticEqualityComparer) - let! parseResults = document.GetFSharpParseResultsAsync("GetDiagnostics") + let! parseResults = document.GetFSharpParseResultsAsync("GetDiagnostics") - match diagnosticType with - | DiagnosticsType.Syntax -> - for diagnostic in parseResults.Diagnostics do - errors.Add(diagnostic) |> ignore + match diagnosticType with + | DiagnosticsType.Syntax -> + for diagnostic in parseResults.Diagnostics do + errors.Add(diagnostic) |> ignore - | DiagnosticsType.Semantic -> - let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync("GetDiagnostics") + | DiagnosticsType.Semantic -> + let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync("GetDiagnostics") - for diagnostic in checkResults.Diagnostics do - errors.Add(diagnostic) |> ignore + for diagnostic in checkResults.Diagnostics do + errors.Add(diagnostic) |> ignore - errors.ExceptWith(parseResults.Diagnostics) + errors.ExceptWith(parseResults.Diagnostics) - let! unnecessaryParentheses = - match diagnosticType with - | DiagnosticsType.Syntax when document.Project.IsFsharpRemoveParensEnabled -> - UnnecessaryParenthesesDiagnosticAnalyzer.GetDiagnostics document - | _ -> CancellableTask.singleton ImmutableArray.Empty - - let result = - if errors.Count = 0 && unnecessaryParentheses.IsEmpty then - ImmutableArray.Empty - else - let iab = ImmutableArray.CreateBuilder(errors.Count + unnecessaryParentheses.Length) - - for diagnostic in errors do - if diagnostic.StartLine <> 0 && diagnostic.EndLine <> 0 then - let linePositionSpan = - LinePositionSpan( - LinePosition(diagnostic.StartLine - 1, diagnostic.StartColumn), - LinePosition(diagnostic.EndLine - 1, diagnostic.EndColumn) - ) - - let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan) - - // F# compiler report errors at end of file if parsing fails. It should be corrected to match Roslyn boundaries - let correctedTextSpan = - if textSpan.End <= sourceText.Length then - textSpan - else - let start = min textSpan.Start (sourceText.Length - 1) |> max 0 - - TextSpan.FromBounds(start, sourceText.Length) - - let location = Location.Create(filePath, correctedTextSpan, linePositionSpan) - iab.Add(RoslynHelpers.ConvertError(diagnostic, location)) - - iab.AddRange unnecessaryParentheses - iab.ToImmutable() - - cache.[cacheKey] <- (textVersion, projectVersion, result) - return result + let! unnecessaryParentheses = + match diagnosticType with + | DiagnosticsType.Syntax when isRemoveParensEnabled -> UnnecessaryParenthesesDiagnosticAnalyzer.GetDiagnostics document + | _ -> CancellableTask.singleton ImmutableArray.Empty + + let result = + if errors.Count = 0 && unnecessaryParentheses.IsEmpty then + ImmutableArray.Empty + else + let iab = ImmutableArray.CreateBuilder(errors.Count + unnecessaryParentheses.Length) + + for diagnostic in errors do + if diagnostic.StartLine <> 0 && diagnostic.EndLine <> 0 then + let linePositionSpan = + LinePositionSpan( + LinePosition(diagnostic.StartLine - 1, diagnostic.StartColumn), + LinePosition(diagnostic.EndLine - 1, diagnostic.EndColumn) + ) + + let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan) + + // F# compiler report errors at end of file if parsing fails. It should be corrected to match Roslyn boundaries + let correctedTextSpan = + if textSpan.End <= sourceText.Length then + textSpan + else + let start = min textSpan.Start (sourceText.Length - 1) |> max 0 + + TextSpan.FromBounds(start, sourceText.Length) + + let location = Location.Create(filePath, correctedTextSpan, linePositionSpan) + iab.Add(RoslynHelpers.ConvertError(diagnostic, location)) + + iab.AddRange unnecessaryParentheses + iab.ToImmutable() + + cache.[cacheKey] <- + { + TextVersion = textVersion + ProjectVersion = projectVersion + FilePath = filePath + IsRemoveParensEnabled = isRemoveParensEnabled + Diagnostics = result + } + + return result } interface IFSharpDocumentDiagnosticAnalyzer with From 22fc6109e62533e43293101238b696ac72e4099f Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 13 Aug 2026 23:06:23 +0200 Subject: [PATCH 3/5] Perf: Add release notes (#20121) --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index cffa42edc9c..0252668b490 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -7,6 +7,7 @@ * 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)) +* Prevent stale and leaked per-document diagnostics by tightening cache validity and evicting entries for removed documents. ([PR #20121](https://github.com/dotnet/fsharp/pull/20121)) * 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)) From 51edeb37a9036c55a9f28137ddbb6e28f3e6c9e2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 20 Aug 2026 17:41:19 +0200 Subject: [PATCH 4/5] evict diagnostics cache on workspace events * `FSharpDocumentDiagnosticAnalyzer` now evicts cached diagnostics when documents/projects/solution are removed, using `WorkspaceChanged` events. * Replaces `evictRemovedDocuments` with targeted evictDocument/evictProject. * Cache eviction is now event-driven, not on-demand. * Also injects optional VisualStudioWorkspace and marks analyzer `[Shared]` for MEF. --- .../Diagnostics/DocumentDiagnosticAnalyzer.fs | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs index 9d0c8ca66f5..3b24e4f9f68 100644 --- a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs +++ b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs @@ -12,6 +12,7 @@ open System.Threading.Tasks open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Diagnostics +open Microsoft.VisualStudio.LanguageServices open FSharp.Compiler.Diagnostics open CancellableTasks @@ -31,8 +32,8 @@ type private CachedDiagnosticsEntry = Diagnostics: ImmutableArray } -[)>] -type internal FSharpDocumentDiagnosticAnalyzer [] () = +[); Shared>] +type internal FSharpDocumentDiagnosticAnalyzer [] ([] workspace: VisualStudioWorkspace | null) = let shouldProduceDiagnostics (document: Document) = document.Project.Solution.GetFSharpExtensionConfig().ShouldProduceDiagnostics() @@ -40,13 +41,30 @@ type internal FSharpDocumentDiagnosticAnalyzer [] () = static let cache = ConcurrentDictionary() - static let evictRemovedDocuments (solution: Solution) = + static let evictDocument (documentId: DocumentId) = + cache.TryRemove(struct (documentId, DiagnosticsType.Syntax)) |> ignore + cache.TryRemove(struct (documentId, DiagnosticsType.Semantic)) |> ignore + + static let evictProject (projectId: ProjectId) = for entry in cache do let struct (documentId, _) = entry.Key - if isNull (solution.GetDocument(documentId)) then + if documentId.ProjectId = projectId then cache.TryRemove(entry.Key) |> ignore + do + match workspace with + | null -> () + | workspace -> + workspace.WorkspaceChanged.Add(fun args -> + match args.Kind with + | WorkspaceChangeKind.DocumentRemoved when not (isNull args.DocumentId) -> evictDocument args.DocumentId + | WorkspaceChangeKind.ProjectRemoved when not (isNull args.ProjectId) -> evictProject args.ProjectId + | WorkspaceChangeKind.SolutionCleared + | WorkspaceChangeKind.SolutionRemoved -> cache.Clear() + | _ -> () + ) + static let diagnosticEqualityComparer = { new IEqualityComparer with @@ -106,8 +124,6 @@ type internal FSharpDocumentDiagnosticAnalyzer [] () = | DiagnosticsType.Syntax -> document.Project.IsFsharpRemoveParensEnabled | DiagnosticsType.Semantic -> false - evictRemovedDocuments document.Project.Solution - let cacheKey = struct (document.Id, diagnosticType) let cached = From 825f0453908df2b26b730c6fbc24ffad268c4fed Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 20 Aug 2026 19:36:13 +0200 Subject: [PATCH 5/5] Use ConditionalWeakTable for DocumentDiagnosticAnalyzer caches Replace ConcurrentDictionary + WorkspaceChanged eviction with two ConditionalWeakTable<Document, CachedDiagnosticsEntry> (syntax/semantic), keyed by Document snapshot identity matching ProjectCache.Projects idiom. Drop TextVersion/FilePath/GetTextVersionAsync. Keep ProjectVersion for Semantic. Builds/tests clean, no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Diagnostics/DocumentDiagnosticAnalyzer.fs | 56 +++++-------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs index 3b24e4f9f68..d1663995bf9 100644 --- a/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs +++ b/vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs @@ -3,16 +3,15 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Collections.Concurrent open System.Collections.Immutable open System.Collections.Generic +open System.Runtime.CompilerServices open System.Threading open System.Threading.Tasks open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Diagnostics -open Microsoft.VisualStudio.LanguageServices open FSharp.Compiler.Diagnostics open CancellableTasks @@ -25,45 +24,19 @@ type internal DiagnosticsType = type private CachedDiagnosticsEntry = { - TextVersion: VersionStamp ProjectVersion: VersionStamp - FilePath: string IsRemoveParensEnabled: bool Diagnostics: ImmutableArray } [); Shared>] -type internal FSharpDocumentDiagnosticAnalyzer [] ([] workspace: VisualStudioWorkspace | null) = +type internal FSharpDocumentDiagnosticAnalyzer [] () = let shouldProduceDiagnostics (document: Document) = document.Project.Solution.GetFSharpExtensionConfig().ShouldProduceDiagnostics() - static let cache = - ConcurrentDictionary() - - static let evictDocument (documentId: DocumentId) = - cache.TryRemove(struct (documentId, DiagnosticsType.Syntax)) |> ignore - cache.TryRemove(struct (documentId, DiagnosticsType.Semantic)) |> ignore - - static let evictProject (projectId: ProjectId) = - for entry in cache do - let struct (documentId, _) = entry.Key - - if documentId.ProjectId = projectId then - cache.TryRemove(entry.Key) |> ignore - - do - match workspace with - | null -> () - | workspace -> - workspace.WorkspaceChanged.Add(fun args -> - match args.Kind with - | WorkspaceChangeKind.DocumentRemoved when not (isNull args.DocumentId) -> evictDocument args.DocumentId - | WorkspaceChangeKind.ProjectRemoved when not (isNull args.ProjectId) -> evictProject args.ProjectId - | WorkspaceChangeKind.SolutionCleared - | WorkspaceChangeKind.SolutionRemoved -> cache.Clear() - | _ -> () - ) + static let syntaxCache = ConditionalWeakTable() + static let semanticCache = ConditionalWeakTable() static let diagnosticEqualityComparer = { new IEqualityComparer with @@ -110,8 +83,6 @@ type internal FSharpDocumentDiagnosticAnalyzer [] ([ CancellableTask.singleton VersionStamp.Default @@ -124,14 +95,15 @@ type internal FSharpDocumentDiagnosticAnalyzer [] ([ document.Project.IsFsharpRemoveParensEnabled | DiagnosticsType.Semantic -> false - let cacheKey = struct (document.Id, diagnosticType) + let cache = + match diagnosticType with + | DiagnosticsType.Syntax -> syntaxCache + | DiagnosticsType.Semantic -> semanticCache let cached = - match cache.TryGetValue(cacheKey) with + match cache.TryGetValue document with | true, cachedEntry when - cachedEntry.TextVersion = textVersion - && cachedEntry.ProjectVersion = projectVersion - && cachedEntry.FilePath = filePath + cachedEntry.ProjectVersion = projectVersion && cachedEntry.IsRemoveParensEnabled = isRemoveParensEnabled -> ValueSome cachedEntry.Diagnostics @@ -196,14 +168,16 @@ type internal FSharpDocumentDiagnosticAnalyzer [] ([ ignore + + cache.Add( + document, { - TextVersion = textVersion ProjectVersion = projectVersion - FilePath = filePath IsRemoveParensEnabled = isRemoveParensEnabled Diagnostics = result } + ) return result }