From edad119de889a3211dfa2f73e32c572f4031e95b Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 06:48:53 +0200 Subject: [PATCH 1/5] Reuse a C# project's in-memory PE reference while its semantic version holds createPEReference keyed the reference on the identity of the Roslyn Compilation and stamped it with DateTime.UtcNow at creation. Roslyn recreates Compilation instances freely - on every solution fork, and under memory pressure because it holds the final compilation weakly - so a dependent F# project kept receiving a reference with a fresh stamp. That stamp feeds the project snapshot's base version, so BootstrapInfo was invalidated, every reference re-imported and every file re-checked, even though the referenced assembly's metadata had not changed. The reference is now cached per referenced project and reused while the project's dependent semantic version is unchanged; a newer Compilation only refreshes the source the delayed reader emits from. Metadata-only emit depends on the public surface, which is what that version tracks, so C# edits below the declaration level no longer invalidate F# checking either. The ConditionalWeakTable is gone with it: its value pinned the compilation until the first emit, which kept the key alive, so entries were never collected. Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../FSharpProjectOptionsManager.fs | 159 +++++++++++------- 2 files changed, 99 insertions(+), 61 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index ba03f663967..2a80b9ec239 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -5,6 +5,7 @@ ### Fixed +* Reuse the in-memory PE reference of a referenced C# project while its dependent semantic version is unchanged, instead of creating one per Roslyn `Compilation` instance. A recreated `Compilation` produced a reference with a fresh stamp, which invalidated the FCS bootstrap of every dependent F# project and made it re-import all of its references before re-checking every file. * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * 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)) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 7cd53631893..86ba5b8019f 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -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 @@ -101,6 +100,33 @@ module private FSharpProjectOptionsHelpers = else hasProjectVersionChanged +/// The in-memory PE reference of a referenced project, kept while the project's dependent +/// semantic version is unchanged. Roslyn recreates `Compilation` instances freely - on every +/// solution fork, and under memory pressure because it holds the final compilation weakly - and +/// a reference created per instance carries a fresh stamp that invalidates every FCS cache +/// keyed on it. +[] +type private PEReferenceCacheEntry(version: VersionStamp, compilation: Compilation) = + // Pinned until the first emit result, so the reader can always be materialised. + let mutable pinned = compilation + let latest = WeakReference(compilation) + + member _.Version = version + + member _.TryGetCompilation() = + match latest.TryGetTarget() with + | true, compilation -> ValueSome compilation + | _ -> ValueNone + + member _.Refresh(compilation: Compilation) = + latest.SetTarget compilation + + match pinned with + | null -> () + | _ -> pinned <- compilation + + member _.Emitted() = pinned <- Unchecked.defaultof<_> + [] type private FSharpProjectOptionsMessage = | TryGetOptionsByDocument of @@ -132,72 +158,75 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = ConcurrentDictionary() // This is used to not constantly emit the same compilation. - let weakPEReferences = ConditionalWeakTable() + let peReferences = + ConcurrentDictionary() + let lastSuccessfulCompilations = ConcurrentDictionary() let scriptUpdatedEvent = Event() - let createPEReference (referencedProject: Project) (comp: Compilation) = + let createNewPEReference (referencedProject: Project) (entry: PEReferenceCacheEntry) = let projectId = referencedProject.Id - - match weakPEReferences.TryGetValue comp with - | true, fsRefProj -> fsRefProj - | _ -> - let mutable strongComp = comp - let weakComp = WeakReference(comp) - let mutable stamp = DateTime.UtcNow - - // 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) - - 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. + let mutable stamp = DateTime.UtcNow + + // 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) + + try + let result = comp.Emit(ms, options = emitOptions, cancellationToken = ct) + + if result.Success then + entry.Emitted() // Stop strongly holding the compilation since we have a result. + lastSuccessfulCompilations.[projectId] <- comp + ms.Position <- 0L + ms :> Stream |> Some + else + entry.Emitted() // 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 -> + // Since we cancelled, keep the compilation pinned and update the stamp. + stamp <- DateTime.UtcNow + ms.Dispose() + None | _ -> - match lastSuccessfulCompilations.TryGetValue(projectId) with - | true, comp -> tryStream comp - | _ -> None - - let getStamp = fun () -> stamp - - let fsRefProj = - FSharpReferencedProject.PEReference(getStamp, DelayedILModuleReader(referencedProject.OutputFilePath, getStream)) - - weakPEReferences.Add(comp, fsRefProj) + entry.Emitted() // Stop strongly holding the compilation since we have a result. + ms.Dispose() // it failed, dispose of stream + 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 + + let getStamp = fun () -> stamp + + FSharpReferencedProject.PEReference(getStamp, DelayedILModuleReader(referencedProject.OutputFilePath, getStream)) + + let createPEReference (referencedProject: Project) (version: VersionStamp) (comp: Compilation) = + match peReferences.TryGetValue referencedProject.Id with + | true, (entry, fsRefProj) when entry.Version = version -> + entry.Refresh comp + fsRefProj + | _ -> + let entry = PEReferenceCacheEntry(version, comp) + let fsRefProj = createNewPEReference referencedProject entry + peReferences.[referencedProject.Id] <- (entry, fsRefProj) fsRefProj let rec tryComputeOptionsBySingleScriptOrFile (document: Document) userOpName = @@ -352,8 +381,8 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = ) elif referencedProject.SupportsCompilation then let! comp = referencedProject.GetCompilationAsync(ct) - let peRef = createPEReference referencedProject comp - referencedProjects.Add(peRef) + and! version = referencedProject.GetDependentSemanticVersionAsync(ct) + referencedProjects.Add(createPEReference referencedProject version comp) if canBail then return ValueNone @@ -427,6 +456,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) @@ -514,6 +548,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = match cache.TryRemove(projectId) with | true, struct (_, _, projectOptions) -> lastSuccessfulCompilations.TryRemove(projectId) |> ignore + peReferences.TryRemove(projectId) |> ignore checker.ClearCache([ projectOptions ]) | _ -> () @@ -522,6 +557,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> lastSuccessfulCompilations.TryRemove(documentId.ProjectId) |> ignore + peReferences.TryRemove(documentId.ProjectId) |> ignore checker.ClearCache([ projectOptions ]) subscription |> Option.iter (fun handler -> handler.Dispose()) | _ -> () @@ -559,6 +595,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = cache.Clear() singleFileCache.Clear() lastSuccessfulCompilations.Clear() + peReferences.Clear() member _.ScriptUpdated = scriptUpdatedEvent.Publish From ee746c22f5ea9675a13a1682200f0227634dac98 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 06:49:41 +0200 Subject: [PATCH 2/5] Link the PE reference release note to PR #20460 Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2a80b9ec239..8c057e18ea7 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -5,7 +5,7 @@ ### Fixed -* Reuse the in-memory PE reference of a referenced C# project while its dependent semantic version is unchanged, instead of creating one per Roslyn `Compilation` instance. A recreated `Compilation` produced a reference with a fresh stamp, which invalidated the FCS bootstrap of every dependent F# project and made it re-import all of its references before re-checking every file. +* Reuse the in-memory PE reference of a referenced C# project while its dependent semantic version is unchanged, instead of creating one per Roslyn `Compilation` instance. A recreated `Compilation` produced a reference with a fresh stamp, which invalidated the FCS bootstrap of every dependent F# project and made it re-import all of its references before re-checking every file. ([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)) * 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)) From 4022458b07c0efac08a9c6f69862c66fdc3e4d07 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 06:56:42 +0200 Subject: [PATCH 3/5] Wrap the PE reference cache doc comment in and reference Compilation with Co-Authored-By: Claude Opus 5 (1M context) --- .../LanguageService/FSharpProjectOptionsManager.fs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 86ba5b8019f..0f18820bf29 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -100,11 +100,13 @@ module private FSharpProjectOptionsHelpers = else hasProjectVersionChanged +/// /// The in-memory PE reference of a referenced project, kept while the project's dependent -/// semantic version is unchanged. Roslyn recreates `Compilation` instances freely - on every -/// solution fork, and under memory pressure because it holds the final compilation weakly - and -/// a reference created per instance carries a fresh stamp that invalidates every FCS cache -/// keyed on it. +/// semantic version is unchanged. Roslyn recreates +/// instances freely - on every solution fork, +/// and under memory pressure because it holds the final compilation weakly - and a reference +/// created per instance carries a fresh stamp that invalidates every FCS cache keyed on it. +/// [] type private PEReferenceCacheEntry(version: VersionStamp, compilation: Compilation) = // Pinned until the first emit result, so the reader can always be materialised. From ee693bc653f7b3499f6fbb5ccc8e835f4e072549 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 07:39:20 +0200 Subject: [PATCH 4/5] Address review: version-first lookup, no Refresh race, dead removals dropped - Look the reference up by dependent semantic version before asking for a Compilation, so a cache hit no longer rebuilds and re-pins one Roslyn dropped under memory pressure (matching SolutionCompilationState.SkeletonReferenceCache). This removes Refresh, and with it the reactor/emit-thread race that could re-pin a compilation after a successful emit, so no lock is needed; and! is gone with it. - TryGetCompilation prefers the pinned compilation and falls back to the weak reference, instead of assuming the two agree. - Split tryGetPEReference from createPEReference and rename createNewPEReference to buildPEReference. - ClearOptions: hoist the lastSuccessfulCompilations and peReferences removals out of the cache.TryRemove arm - cache holds F# project ids, both dictionaries hold referenced C# ids, so the removals never ran. Drop their ClearSingleFileOptionsCache twins, where the id is the miscellaneous-files project and can never be a key. - Drop comments the code already states, and trim the release note to the user-visible effect. Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- .../FSharpProjectOptionsManager.fs | 62 +++++++++---------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 8c057e18ea7..9c1655fbb72 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -5,7 +5,7 @@ ### Fixed -* Reuse the in-memory PE reference of a referenced C# project while its dependent semantic version is unchanged, instead of creating one per Roslyn `Compilation` instance. A recreated `Compilation` produced a reference with a fresh stamp, which invalidated the FCS bootstrap of every dependent F# project and made it re-import all of its references before re-checking every file. ([PR #20460](https://github.com/dotnet/fsharp/pull/20460)) +* Reuse the in-memory PE reference of a referenced C# project while its dependent semantic version is unchanged, so dependent F# projects no longer re-import every reference and re-check every file each time Roslyn recreates the `Compilation`. ([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)) * 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)) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 0f18820bf29..42eb821b041 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -116,18 +116,14 @@ type private PEReferenceCacheEntry(version: VersionStamp, compilation: Compilati member _.Version = version member _.TryGetCompilation() = - match latest.TryGetTarget() with - | true, compilation -> ValueSome compilation - | _ -> ValueNone - - member _.Refresh(compilation: Compilation) = - latest.SetTarget compilation - match pinned with - | null -> () - | _ -> pinned <- compilation + | null -> + match latest.TryGetTarget() with + | true, compilation -> ValueSome compilation + | _ -> ValueNone + | pinned -> ValueSome pinned - member _.Emitted() = pinned <- Unchecked.defaultof<_> + member _.Emitted() = pinned <- null [] type private FSharpProjectOptionsMessage = @@ -159,7 +155,6 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let singleFileCache = ConcurrentDictionary() - // This is used to not constantly emit the same compilation. let peReferences = ConcurrentDictionary() @@ -167,7 +162,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let scriptUpdatedEvent = Event() - let createNewPEReference (referencedProject: Project) (entry: PEReferenceCacheEntry) = + let buildPEReference (referencedProject: Project) (entry: PEReferenceCacheEntry) = let projectId = referencedProject.Id let mutable stamp = DateTime.UtcNow @@ -185,12 +180,12 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let result = comp.Emit(ms, options = emitOptions, cancellationToken = ct) if result.Success then - entry.Emitted() // Stop strongly holding the compilation since we have a result. + entry.Emitted() lastSuccessfulCompilations.[projectId] <- comp ms.Position <- 0L ms :> Stream |> Some else - entry.Emitted() // Stop strongly holding the compilation since we have a result. + entry.Emitted() ms.Dispose() // it failed, dispose of stream None with @@ -200,7 +195,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = ms.Dispose() None | _ -> - entry.Emitted() // Stop strongly holding the compilation since we have a result. + entry.Emitted() ms.Dispose() // it failed, dispose of stream None @@ -220,16 +215,16 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = FSharpReferencedProject.PEReference(getStamp, DelayedILModuleReader(referencedProject.OutputFilePath, getStream)) - let createPEReference (referencedProject: Project) (version: VersionStamp) (comp: Compilation) = + let tryGetPEReference (referencedProject: Project) (version: VersionStamp) = match peReferences.TryGetValue referencedProject.Id with - | true, (entry, fsRefProj) when entry.Version = version -> - entry.Refresh comp - fsRefProj - | _ -> - let entry = PEReferenceCacheEntry(version, comp) - let fsRefProj = createNewPEReference referencedProject entry - peReferences.[referencedProject.Id] <- (entry, fsRefProj) - fsRefProj + | 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 { @@ -382,9 +377,13 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = FSharpReferencedProject.FSharpReference(referencedProject.OutputFilePath, projectOptions) ) elif referencedProject.SupportsCompilation then - let! comp = referencedProject.GetCompilationAsync(ct) - and! version = referencedProject.GetDependentSemanticVersionAsync(ct) - referencedProjects.Add(createPEReference referencedProject version comp) + let! version = referencedProject.GetDependentSemanticVersionAsync(ct) + + match tryGetPEReference referencedProject version with + | ValueSome peRef -> referencedProjects.Add peRef + | ValueNone -> + let! comp = referencedProject.GetCompilationAsync(ct) + referencedProjects.Add(createPEReference referencedProject version comp) if canBail then return ValueNone @@ -548,18 +547,15 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = | FSharpProjectOptionsMessage.ClearOptions(projectId) -> match cache.TryRemove(projectId) with - | true, struct (_, _, projectOptions) -> - lastSuccessfulCompilations.TryRemove(projectId) |> ignore - peReferences.TryRemove(projectId) |> ignore - checker.ClearCache([ projectOptions ]) + | true, struct (_, _, projectOptions) -> checker.ClearCache([ projectOptions ]) | _ -> () + lastSuccessfulCompilations.TryRemove(projectId) |> ignore + peReferences.TryRemove(projectId) |> ignore legacyProjectSites.TryRemove(projectId) |> ignore | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> - lastSuccessfulCompilations.TryRemove(documentId.ProjectId) |> ignore - peReferences.TryRemove(documentId.ProjectId) |> ignore checker.ClearCache([ projectOptions ]) subscription |> Option.iter (fun handler -> handler.Dispose()) | _ -> () From 69cb60add36c9cb41ccec222ed6b49b887881367 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 9 Sep 2026 23:51:32 +0200 Subject: [PATCH 5/5] Add a focused test counting PE emits across unchanged/changed reference stamps Addresses https://github.com/dotnet/fsharp/pull/20460#discussion_r3965947159 by proving the specific claim the fix depends on, at the FCS level rather than through FSharpProjectOptionsManager (which the editor test host cannot exercise with a real C# project - see the review's earlier note on that gap). mkCountedCSharpPEReference builds a PEReference backed by an independent CSharpCompilation each time, with a counted getStream (the PE emit / metadata-import entry point DelayedILModuleReader lazily invokes). Two facts: - Reusing a CSharp reference's stamp avoids re-emitting a recreated Compilation: checking the same F# file first against one reference, then against a second reference for a different Compilation but the SAME stamp, leaves the second reference's emit count at 0 - FSharpProjectOptions.AreSameForChecking's structural fallback (both Stamp fields are None here) treats the two ReferencedProjects arrays as equal via PEReference's custom Equals (OutputFile + getStamp()), so the checker's incrementalBuildersCache reuses the existing build and never touches the new reference. - Changing a CSharp reference's stamp does re-emit the new Compilation: the same setup with a different stamp does invoke the second reference's getStream at least once - the control that rules out the first test passing by some unrelated always-cached path. Both verified locally (FSharpSuite.Tests.fsproj, filter-method "*CSharp reference's stamp*"): 2/2 pass. The full MultiProjectTests class (5 facts) still passes. Co-Authored-By: Claude Sonnet 5 --- .../Compiler/Service/MultiProjectTests.fs | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tests/fsharp/Compiler/Service/MultiProjectTests.fs b/tests/fsharp/Compiler/Service/MultiProjectTests.fs index 2633494f3e4..5a8d22073a3 100644 --- a/tests/fsharp/Compiler/Service/MultiProjectTests.fs +++ b/tests/fsharp/Compiler/Service/MultiProjectTests.fs @@ -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 @@ -209,3 +210,91 @@ let y = 1 + + // Focused counters for https://github.com/dotnet/fsharp/pull/20460#discussion_r3965947159: + // a referenced C# project's `Compilation` is recreated on every solution fork, but its + // in-memory PE reference is only re-emitted when the project's dependent semantic version + // (here, the reference's stamp) actually changes. + 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(), 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 + + [] + let ``Reusing a CSharp reference's stamp avoids re-emitting a recreated Compilation``() = + let stamp = DateTime(2024, 1, 1) + let csRefProj1, emitCount1 = mkCountedCSharpPEReference stamp + let csRefProj2, emitCount2 = mkCountedCSharpPEReference stamp + + let fsOptions = projectReferencing csRefProj1 + checkUsesCSharpClass fsOptions + Assert.Equal(1, emitCount1()) + + // Same dependent semantic version (stamp unchanged): the checker must reuse its cached + // project build and never touch the recreated Compilation behind the new reference. + checkUsesCSharpClass { fsOptions with ReferencedProjects = [|csRefProj2|] } + Assert.Equal(0, emitCount2()) + + [] + let ``Changing a CSharp reference's stamp does re-emit the new Compilation``() = + let csRefProj1, _ = mkCountedCSharpPEReference (DateTime(2024, 1, 1)) + let csRefProj2, emitCount2 = mkCountedCSharpPEReference (DateTime(2024, 1, 2)) + + let fsOptions = projectReferencing csRefProj1 + checkUsesCSharpClass fsOptions + + // Different dependent semantic version (stamp changed): the checker must pick up the new reference. + checkUsesCSharpClass { fsOptions with ReferencedProjects = [|csRefProj2|] } + Assert.True(emitCount2() >= 1)