Reuse a C# project's in-memory PE reference while its semantic version holds

[xperiandri]

The in-memory PE reference FCS caches for a referenced C# project was stamped with the time it was created, and Roslyn hands back a new Compilation instance for that project on every solution fork and, under memory pressure, even when nothing changed — so FCS treated an unchanged C# project as a different reference each time and kept re-importing and re-checking it, continuously at solution scale.

The reference is now cached per referenced project and reused while the project's dependent semantic version is unchanged; a same-version Compilation only refreshes the source the delayed reader will emit from, when asked to. C# edits below the declaration level no longer invalidate F# checking either, since metadata-only emit depends on the public surface the dependent semantic version tracks. What is retained is bounded by the number of referenced C# projects — one compilation is pinned per project until its first emit result, then held weakly. Cancellation is unchanged: a cancelled emit keeps the compilation pinned and bumps the stamp so the reference is retried. The same stamp equality also lets the legacy (non-transparent-compiler) checker recognize two builds as the same project, so the reuse isn't specific to either checking path.

A focused test proves the mechanism directly — repeated emit counts across an unchanged vs. a changed stamp — at the FCS level; the editor test host cannot construct a C# project, so FSharpProjectOptionsManager itself is exercised only by building the VSIX and watching the Debug pane's FCS trace against a large solution.

🤖 Generated with Claude Code

[github-actions]

✅ No release notes required

[xperiandri]

Review from the architecture side. The direction is right, and it has a precedent worth citing in the description: Roslyn's own cross-language reference cache, SolutionCompilationState.SkeletonReferenceCache, keys the metadata-only skeleton of a project on Project.GetDependentSemanticVersionAsync and, for an unchanged version, returns what it has "regardless of whether it succeeded or not". The editor already treats that version as "the reference did not change" in WorkspaceExtensions.getReferencedProjectVersions (the snapshot-reuse guard), so this PR makes the PE-reference stamp consistent with that guard instead of racing ahead of it.

Findings, most important first. Each has an inline suggestion; the first three go together.

1. A cache hit still awaits GetCompilationAsync. On a hit the compilation only feeds Refresh, and after the first emit result nobody reads it again — but when Roslyn has dropped the final compilation under memory pressure the call rebuilds it and the entry pins it: one full compilation per referenced C# project per options recompute, in exactly the scenario the PR targets. Roslyn's cache checks the version first and touches the compilation only on a miss.
2. Refresh races Emitted() (reactor thread vs. FCS emit thread, no shared lock) and can re-pin a compilation after its emit succeeded; TryGetCompilation also ignores pinned. With (1), Refresh disappears and the entry becomes single-writer.
3. and! would be the first use in the repository (FSharp.instructions.md: no foothold) and buys nothing here. Gone with (1).
4. The new peReferences.TryRemove in ClearOptions is dead code — it sits in the arm that is entered for F# project ids only, so a removed C# project keeps its entry until the next sweep. The ClearSingleFileOptionsCache twin is dead for the same reason (miscellaneous-files project), and so are the pre-existing lastSuccessfulCompilations lines next to them.
5. Nits: three // Stop strongly holding… comments restate Emitted(); the comment above peReferences is now covered by the type summary; the release note carries the war story.

Tests (the checklist asks where this could be covered): the editor test host cannot hold a C# project today — TestHostWorkspaceServices.GetLanguageServices in RoslynHelpers.fs throws for anything but F#, so SupportsCompilation is never true there. Two routes: (a) compose Microsoft.CodeAnalysis.CSharp.Workspaces into the test host and assert that TryGetOptionsByProject hands back the same ReferencedProjects.[0] object across a body-only edit of the C# project and a different one after a declaration edit; (b) cheaper — make the version/pin logic an internal type with the emit function injected and test it directly: same object for the same version, new object on a version change, pin dropped after a result, pin kept and stamp bumped on cancellation. I checked the suggested blocks with Fantomas and with a stand-in script covering the cases in (b); a full FSharp.Editor build was not run.

Not for this PR, but worth knowing: with the transparent compiler off, Stamp = hash(GetDependentVersionAsync) still creates a new IncrementalBuilder — which imports its non-framework references afresh — on every edit in a referenced C# project, body-level included, so the legacy path does not benefit. And FCS keys on a wall-clock DateTime: two version changes inside one clock tick share a stamp; carrying the previous entry's stamp forward (max UtcNow (previous + 1 tick)) would make it strictly increasing per project.

[github-actions]

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: changes PE reference caching in IDE project options reactor

Generated by PR Tooling Safety Check · opus46 5M · ◷

[T-Gro]

🤖🕵️⏱️🔥
Add an exact-head trace or focused test that counts PE emits, imports, and checks across unchanged and changed dependent semantic versions. Until then, limit the claim to the proven avoided PE emit and reader construction.

[xperiandri]

Added a focused test in 3d74593: tests/fsharp/Compiler/Service/MultiProjectTests.fs counts PE emits directly.

• Reusing a CSharp reference's stamp avoids re-emitting a recreated Compilation — checks the same F# file against two PEReferences backed by two independent CSharpCompilation instances sharing one stamp; the second reference's emit counter stays at 0.
• Changing a CSharp reference's stamp does re-emit the new Compilation — same setup with a different stamp; the second reference's emit counter is ≥ 1 (the control, ruling out the first test passing for an unrelated reason).

Both pass locally (2/2), pre-existing MultiProjectTests facts (5 total) unaffected.

This proves the emit/import avoidance directly (no editor Workspace needed — TestHostWorkspaceServices.GetLanguageServices throws for non-F#, which is why this lives at the FCS level rather than exercising FSharpProjectOptionsManager itself). I also softened the PR description to lead with what's proven and call the CPU trace an illustration rather than a measurement, per your note.

[T-Gro]

@xperiandri TODO - I think this hasnt been addressed

[xperiandri]

It is in, in tests/fsharp/Compiler/Service/MultiProjectTests.fs (lines 276 and 291) — two facts that count emits directly rather than timing anything:

• Reusing a CSharp reference's stamp avoids re-emitting a recreated Compilation — one F# file checked against two PEReferences backed by two independent CSharpCompilation instances that share a stamp. The second reference's emit counter stays at 0.
• Changing a CSharp reference's stamp does re-emit the new Compilation — the same setup with a different stamp, asserting the counter is at least 1. It is there as the control: without it the first test would also pass if the reference were never consulted at all.

So of the three things asked for, PE emits are counted and proven, in both directions. Imports and checks are not counted — nothing in the test host exposes those counters, and reaching them would mean driving FSharpProjectOptionsManager through an editor workspace, which TestHostWorkspaceServices.GetLanguageServices refuses for a non-F# project. The PR description was narrowed at the same time to claim only the avoided emit and reader construction, and to call the CPU trace an illustration rather than a measurement.

If you want the import and check counts as well, I would rather do that as a separate change that gives FCS a test hook for them, since the same hook is what any future claim about reference invalidation will need.

[T-Gro]

🤖🕵️ If this fixes an issue or implements an RFC/suggestion, link it (Fixes #... when applicable). Otherwise, give a short management-level summary in simplified technical English: what user scenario improves and what this achieves.

Please apply this PR-description guidance. Remove the implementation inventory already visible in Files, but keep necessary scope, compatibility, and dependency caveats.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved moderate correctness issues remain, and coverage does not exercise the manager’s new cache behavior.

Review effort: Lite
Findings: 2

Open (2)

• Remove duplicate release-note bullets · New
• Add manager coverage for per-project PE reference caching · New

What changed in this PR

Caches in-memory C# PE references by dependent semantic version to reduce redundant metadata emission and F# rechecking.

Changes:

• Adds per-project PE reference caching and cleanup.
• Adds focused reuse and invalidation tests.
• Adds Visual Studio release notes.

File	Review findings
vsintegration/​src/​FSharp.Editor/​LanguageService/​FSharpProjectOptionsManager.fs	Four moderate findings (1 vote each): body-only C# edits still invalidate F# checks; same-version cache hits can retain a failed compilation; DateTime.UtcNow may produce equal stamps for replacements; and output-path changes are not included in the cache identity.
tests/​fsharp/​Compiler/​Service/​MultiProjectTests.fs	Nit (2 votes): tests exercise FCS directly rather than FSharpProjectOptionsManager, so the new per-project cache behavior is not covered.
docs/​release-notes/​.VisualStudio/​18.vNext.md	Nit (4 votes): three duplicate release-note bullets should be reduced to one, preferably the entry with the PR link.