Cache reference timestamps behind an `IVsAsyncFileChangeEx2` watcher for snapshot reuse by xperiandri · Pull Request #20457 · dotnet/fsharp

xperiandri · 2026-09-05T19:26:29Z

The transparent compiler's snapshot-reuse check stats every -r: reference on disk to decide whether a cached snapshot is still valid, on every Roslyn project fork — for a project with a few hundred references, that is hundreds of file-system calls per keystroke-driven typecheck. This replaces that per-check disk read with a stamp cache kept fresh by IVsAsyncFileChangeEx2 notifications, so references are re-checked only when one actually changes on disk.

FSharpProjectSnapshot.FromOptions still stats every reference when a snapshot is built from scratch, and the incremental builder's legacy path stats on every request; both need the same notification wired into FCS and are follow-ups (#20459 covers the first). Script #load sources are invisible to the workspace and are not watched.

Checklist

Test cases added
Performance benchmarks added in case of performance changes
Release notes entry updated

github-actions · 2026-09-05T19:26:41Z

✅ Release notes checked

✅ Found changes and release notes in following paths:

Change path Release notes path Description

`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.200.md

github-actions · 2026-09-05T20:31:41Z

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: adds FileChangeWatcher + FSharpReferenceChangeTracker to VS integration that execute at design time

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

xperiandri

Fresh-eyes pass over the whole PR against the current Roslyn FileChangeWatcher / ReferenceFileChangeTracker and against what FCS actually does with references. The shape is right: batched advise/unadvise with the 500 ms window, directory watches for the reference roots, ref-counted per-file watches, free-threaded sink. Three things need fixing, and one framing point matters for the description.

Must fix (inline, with suggestions):

WatchFiles runs are coalesced across sinks and advised with the first one. Latent today, breaks as soon as a second context exists.
FSharpReferenceChangeTracker races Timer.Change against Timer.Dispose and can throw inside the VS file change callback; it also allocates a timer per unwatched path before checking whether anyone watches it.
cache.TryRemove in onWatchedReferenceChanged never reaches consumers (they go through ProjectCache.Projects) and causes a second InvalidateConfiguration at the next recompute, discarding a builder that may already be rebuilt. InvalidateConfiguration alone is the right call.

Framing. As it stands the PR does not change observable latency. On the legacy path getOrCreateBuilder (BackgroundCompiler.fs L492) evaluates IsReferencesInvalidated on every request, and that stats every reference with a fresh TimeStampCache (IncrementalBuild.fs L1245). On the transparent-compiler path the snapshot reuse compares ReferencesOnDisk by stat (WorkspaceExtensions.fs L250) and FromOptions stats on creation. Both notice a rebuilt dll at the next request anyway, and InvalidateConfiguration only swaps in a lazy builder node without computing anything. So "keeps serving options and snapshots computed against the old assembly" is not what happens today. The value of the watcher arrives when it replaces those stat loops: a reference-change notification for the incremental builder on the FCS side (the analogue of useChangeNotifications for sources) and a watcher-invalidated stamp cache for snapshots. Worth presenting this PR as that infrastructure and listing both follow-ups explicitly.

Also inline: diff-based watch updates instead of stop-all/start-all on every options recompute, NUGET_PACKAGES and whether the NuGet cache should be a directory watch at all (Roslyn does not do that), a shared no-op token, the drain loop, logging of swallowed exceptions, the design-review doc, and applyBatch test coverage. The Fixes # (issue, if applicable) placeholder is still in the description.

All suggestions were applied together on the branch and pass dotnet fantomas --check; the changed regions were type-checked and smoke-tested with stand-ins for the VS interop types (sink grouping, drain, debounce and the timer race under 8 concurrent producers, watch diffing).

xperiandri · 2026-09-05T23:05:19Z

Addressed the fresh-eyes review in d8dce7a / 704a0ad; each inline thread has a reply. The description is reframed as you suggested: the watcher is infrastructure, the current stat loops (IsReferencesInvalidated, ReferencesOnDisk) still notice a rebuilt dll on the next request, and the latency win arrives with the two follow-ups now listed explicitly (an FCS-side reference-change notification for the incremental builder, and a watcher-invalidated stamp cache for snapshots), plus script #r/#load watching. The Fixes # placeholder is gone.

xperiandri · 2026-09-06T01:04:01Z

Reworked in c15f47b after checking the premise in VS instead of assuming it.

Breakpoints on both invalidation paths — onWatchedReferenceChanged in the reactor and the isProjectInvalidated branch of tryComputeOptions — with a referenced project rebuilt from the command line: Roslyn's path fires first. ProjectSystemProjectFactory already advises every MetadataReference path, swaps the reference when the file changes and bumps Project.Version, and the reactor recomputes and calls InvalidateConfiguration off the back of that. The tracker gets there later by construction: Roslyn batches over 500 ms, this adds a 2 s debounce on top of the same window.

So the reference subscription is gone and FSharpProjectOptionsReactor is untouched by this PR. What remains is the transport plus its tests, and the description now says plainly that it has no consumer yet and lists the three the workspace genuinely does not cover: script #load sources, the ReferencesOnDisk stamp cache, and an FCS-side notification replacing IsReferencesInvalidated.

Happy to fold the first of those into this PR instead, if you would rather not take a transport on its own.

xperiandri · 2026-09-06T02:05:49Z

The transport now has its consumer, and it is the one that actually removes a stat loop: the snapshot-reuse guard in createProjectSnapshot no longer stats every -r: per new Project instance; it reads stamps that the watcher keeps inside its watch entries (dadf230f5a, 744355228a, 7153224f14, 15bd8ce03e).

The reactor subscription is back, but not for the reason it was removed in c15f47bd52: it registers the -r: set so the stamps exist, and passes no change handler. Invalidating the FCS build stays Roslyn's job, as measured earlier.

Soundness rule, since it is the one thing worth reading closely: a cached stamp lives in the watch entry and is dropped on the raw notification, so it can only be served while a notification can still reach it; anything unwatched is stat'd directly; a mismatch against the snapshot's own stamps drops the project's stamps, so a missed notification costs one re-stat pass, not a rebuild per Project instance.

Description and docs/ide/file-watching.md are updated accordingly; the three remaining stat sites (FromOptions, IsReferencesInvalidated, script #load) are listed as follow-ups.

xperiandri · 2026-09-06T04:06:29Z

CI: every Windows leg failed with FS1116/FS1118 at FileChangeWatcher.fs(31,12) (EndsWithOrdinal), and CheckCodeFormatting flagged WorkspaceExtensions.fs.

Cause: inline members of the internal module Internal.Utilities.Library cannot be inlined into another assembly, InternalsVisibleTo or not — the optimizer drops their optimization data at the assembly boundary, so a consumer compiled with --optimize+ (Release) fails with FS1118, while --optimize- (Debug) compiles and calls the compiled method instead. Reproduced with a two-file library: module internal Lib + [<InternalsVisibleTo>] + let inline f … gives FS1116/FS1118 in the consumer under --optimize+ with both the SDK and the bootstrap compiler; a public module works; --realsig and a signature file make no difference. FSharp.Editor on main uses none of those helpers, which is why it only surfaced here.

Fix: call String.StartsWith/EndsWith with an explicit StringComparison directly in FSharp.Editor (plus the fantomas fix); the illib StartsWithOrdinalIgnoreCase helper stays. Verified locally: FSharp.Editor builds in Release with that change.

A queued advise is not a subscription: `EnqueueWatchingFile` returns before the batched `AdviseDirChangeAsync`/`AdviseFileChangesAsync` call even runs, and `GetLastWriteTimeUtc` was caching the first stat it saw regardless of whether a notification could ever reach that path yet. A change landing in that window - or a permanently failed advise, e.g. a Reference Assemblies directory that does not exist on this machine - left a stale stamp with no way to invalidate it. `IFSharpWatchedFile` now reports `IsActive`, true only once its advise has actually succeeded: a per-file token via its `Cookie`, a directory via a `bool ref` the corresponding `WatchDir` op flips on success. The stamp cache in `FSharpReferenceChangeTracker.GetLastWriteTimeUtc` gates on it, so a pending or failed watch always stats directly, and the first cached read is guaranteed to happen no earlier than the moment a change could have been observed. This also fixes a real bug the review comment's wording pointed at: `applyBatch`'s single try/with wrapped the whole `while` loop, so one failing operation (that nonexistent Reference Assemblies directory, on most dev machines without the .NET Framework SDK) silently dropped every other operation queued in the same batch. Each case now catches its own failure and lets the rest of the batch proceed. Addresses dotnet#20457 (comment) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

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.

The FileChangeWatcher worktree is pull-model I/O (OpenFileForReadShimAsync plus last-write timestamps on FSharpFileSnapshot), not a push watcher. Roslyn's FileChangeWatcher is the reference: IVsAsyncFileChangeEx2, directory subscriptions, 500 ms AsyncBatchingWorkQueue, free-threaded sinks, coalesced metadata-reference invalidation. This repo already has two IVsFileChangeEx clients (legacy FileChangeManager and deprecated FSharpSource.SetDependencyFiles). The intended FSharp.Editor replacement lives only in stash@{7} (54465595717b8bb746cb2633d5a4aa834888a481): FileChangeWatcher.fs plus FileChangeWatcherHub, wired to FSharpProjectOptionsReactor for -r: assemblies. It is IVsFileChangeEx + JTF.Run, not IVsAsyncFileChangeEx2. No commit, branch, or GitHub hit implements IVsAsyncFileChangeEx2. Recommended split: ship the async read shim on its own; restore the stash watcher or jump straight to IVsAsyncFileChangeEx2 with directory batching; invalidate FCS via NotifyFileChanged instead of O(N) timestamp polling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Roslyn-shaped push file watching for FSharp.Editor: - FSharpFileChangeWatcher: batched advise/unadvise (500ms window, coalesced same-kind ops), service obtained via Task without blocking on UI thread - FileChangeContext: free-threaded sink (IVsFreeThreadedFileChangeEvents2), directory subscriptions with extension filters, per-file watches covered by watched directories become no-op tokens - FSharpReferenceChangeTracker: ref-counted reference watching with 2s debounce; default directory watches for DOTNET_ROOT\packs, dotnet\packs, Reference Assemblies, NuGet cache (.dll filter) Modeled on Roslyn FileChangeWatcher/ReferenceFileChangeTracker (all internal there, not reusable from F#). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Subscribe each project's on-disk '-r:' reference assemblies via FSharpReferenceChangeTracker when options are computed; on a watched dll change, drop that project's cached options and invalidate the checker configuration. Subscriptions are ref-counted, cleared on project removal and reactor disposal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Cover WatchedDirectory path matching, tracker ref-counting, debounce of burst notifications, and dispose. Tests use an in-memory IFSharpFileChangeWatcher mock so they do not need a live IVsAsyncFileChangeEx2 service. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* FSharpProjectOptionsReactor/Manager take the IFSharpFileChangeWatcher outright; the only caller always has one, so the option wrappers and the Option.iter/map plumbing around the tracker go away. * Reference watches per project are an OrdinalIgnoreCase HashSet, so a change notification is a Contains instead of an Array.exists with an explicit comparison. * FSharpWatchedFileToken.Cookie and the test mock's context are voption. * applyBatch indexes the drained ResizeArray directly instead of converting it to a list and re-slicing it with takeWhile/skip/collect. * StartsWithOrdinal / EndsWithOrdinal / EndsWithOrdinalIgnoreCase from Internal.Utilities.Library at the ordinal call sites, interpolation for the trailing separator, and the default watched directories go through one Seq chain materialized once.

The list-typed applyBatch reads better than the index walk; only the voption use sites differ from the original body.

* IFSharpFileChangeWatcher.CreateContext and WatchedDirectory take ImmutableArray, the same contract as Roslyn's FileChangeWatcher; the set is built once and scanned on every EnqueueWatchingFile. * applyBatch is a cancellableTask and passes its token to every IVsAsyncFileChangeEx2 call. The agent runs under a token owned by the watcher, which is now IDisposable; cancellation is no longer swallowed by the loop's catch-all. * Batches are sliced and collected as arrays, so the cookie and path arrays go to the service without a List.toArray copy.

The ignore-case StartsWith was the one ordinal comparison in FileChangeWatcher.fs without an illib helper; the sibling of the existing EndsWithOrdinalIgnoreCase closes that gap and the watched- directory check uses it.

* WatchFiles runs are coalesced only while the sink is the same one, so a second context's files are never advised with the first context's sink. * FSharpReferenceChangeTracker keeps its timers in a Dictionary under the gate, checks that a path is watched before allocating a timer, and no longer races Timer.Change against Timer.Dispose inside the VS callback. * onWatchedReferenceChanged only invalidates the FCS configuration; the cached options are still correct and dropping them forced a second InvalidateConfiguration on the next recompute. * watchReferenceFiles diffs the new '-r:' set against the previous one, so an unchanged reference list touches no watches. * Unwatching clears the token's cookie, so a token without a cookie is a no-op instead of a second unadvise. * NUGET_PACKAGES is honoured for the NuGet cache directory watch, the drain loop uses CurrentQueueLength, swallowed batch failures go to the F# output pane, covered paths share one no-op token, and the batching window is a constructor parameter so tests can shorten it. * The design-review working note is replaced by a short design note. * Tests cover applyBatch against a recording IVsAsyncFileChangeEx2.

An F# optional parameter is an option cell per call; the production callers never pass the delay, so give them a constructor without it and keep the explicit-delay one for tests.

Checked in VS with breakpoints on both paths: for a `-r:` the workspace holds as a MetadataReference, Roslyn advises the file itself, swaps the reference when it changes and bumps Project.Version, so the reactor recomputes and calls InvalidateConfiguration on its own. That path hits first — Roslyn batches over 500 ms where the tracker adds a 2 s debounce on top — and a second subscription only invalidates the same configuration again, later. So FSharpProjectOptionsReactor goes back to what it was, and this PR ships the transport alone. The consumers that the workspace does not already cover — script `#load` sources, the snapshot stamp cache, an FCS-side reference notification — follow separately.

FSharpReferenceChangeTracker now records each watched path's last-write stamp in the same entry as its ref-count and token, and drops it on the raw change notification. IReferenceStamps serves a cached stamp only while the path is watched — a notification can still reach it — and stats unwatched paths directly.

The reactor registers the '-r:' paths of every project it computes options for, diffing against the previous set so an unchanged list touches no watches, and exposes the tracker's stamps. It passes no change handler: invalidating the FCS build is Roslyn's job, which swaps the MetadataReference and bumps Project.Version.

The snapshot-reuse guard compared ReferencesOnDisk by stat'ing every '-r:' on each new Project instance, before the same-version fast path. It now reads the tracker's stamps, and on a mismatch drops the project's stamps so a missed notification costs one re-stat rather than a rebuild per Project instance.

FSharpReferenceChangeTracker gets a public Dispose with the interface forwarding to it, the MailboxProcessor pattern, so the reactor disposes it and the agent without casts. The reactor's opens follow the System / FSharp.Compiler / Microsoft / Internal.Utilities grouping.

Inline members of the internal Internal.Utilities.Library module cannot be inlined into another assembly, InternalsVisibleTo or not: the optimizer drops their optimization data at the assembly boundary, so FSharp.Editor fails with FS1116/FS1118 under --optimize+ (every Windows Release leg of the CI). Debug compiled only because --optimize- never tries to inline them. Also formats WorkspaceExtensions.fs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

A queued advise is not a subscription: `EnqueueWatchingFile` returns before the batched `AdviseDirChangeAsync`/`AdviseFileChangesAsync` call even runs, and `GetLastWriteTimeUtc` was caching the first stat it saw regardless of whether a notification could ever reach that path yet. A change landing in that window - or a permanently failed advise, e.g. a Reference Assemblies directory that does not exist on this machine - left a stale stamp with no way to invalidate it. `IFSharpWatchedFile` now reports `IsActive`, true only once its advise has actually succeeded: a per-file token via its `Cookie`, a directory via a `bool ref` the corresponding `WatchDir` op flips on success. The stamp cache in `FSharpReferenceChangeTracker.GetLastWriteTimeUtc` gates on it, so a pending or failed watch always stats directly, and the first cached read is guaranteed to happen no earlier than the moment a change could have been observed. This also fixes a real bug the review comment's wording pointed at: `applyBatch`'s single try/with wrapped the whole `while` loop, so one failing operation (that nonexistent Reference Assemblies directory, on most dev machines without the .NET Framework SDK) silently dropped every other operation queued in the same batch. Each case now catches its own failure and lets the rest of the batch proceed. Addresses dotnet#20457 (comment) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

FSharp.Editor's own voption-returning extensions (Common/Extensions.fs) cover this; no reason to allocate an option here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…tive tests MockWatchedFile implements IDisposable (through IFSharpWatchedFile), so constructing it without `new` is FS0760, an error under the repo's warnings-as-errors. That one line failed FSharp.Editor.Tests on all Windows jobs of the last CI run; FSharp.Editor itself compiled fine in Release. The two watcher tests asserted IsActive against wall-clock timing: right after enqueueing (relying on the 100 ms batching window not having elapsed) and right after the second recorded call (the flag is set a few instructions after that call returns). Both could flip on a loaded agent. The first now holds the service back with a TaskCompletionSource, so the advise cannot run until the test releases it; both wait for activation with SpinWait.SpinUntil instead of reading the flag once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Address the watcher notification, synchronization, and disposal issues before approval.

Review effort: Lite
Findings: 1 · 1

Open (2)

Subscribe to file creation and deletion notifications · New
Remove duplicate release note entry · New

What changed in this PR

Adds asynchronous file watching and cached reference timestamps to reduce filesystem stats during snapshot reuse.

Changes:

Adds batched IVsAsyncFileChangeEx2 watching and stamp caching.
Integrates cached stamps into project snapshot reuse.
Adds tests, documentation, and release notes.

File	Summary
`vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj`	Registers watcher tests.
`vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs`	Adds watcher and cache tests.
`vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs`	Uses cached reference stamps.
`vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs`	Initializes the watcher.
`vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs`	Tracks reference watches. Moderate: cleanup is not reactor-serialized. Moderate: watcher resources are not disposed on solution close.
`vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs`	Implements file watching and stamp caching. Moderate: add/delete notifications are not subscribed to.
`vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj`	Includes watcher sources.
`src/Compiler/Utilities/illib.fsi`	Declares a string comparison helper.
`src/Compiler/Utilities/illib.fs`	Adds an unused helper. Nit: remove it or use it.
`docs/release-notes/.VisualStudio/18.vNext.md`	Adds Visual Studio release notes. Nit: remove the duplicate entry.
`docs/release-notes/.FSharp.Compiler.Service/11.0.100.md`	Adds FCS release notes.
`docs/ide/file-watching.md`	Documents watcher design.

xperiandri · 2026-09-26T12:47:21Z

+    let watchFlags =
+        _VSFILECHANGEFLAGS.VSFILECHG_Size ||| _VSFILECHANGEFLAGS.VSFILECHG_Time


Fixed in c1a2d725d6: a per-file watch now asks for VSFILECHG_Add and VSFILECHG_Del beside size and time.

The inconsistency you point at was real, and the comment above the flags was wrong about it — Roslyn uses one set for subscribing a file (Size | Time, FileChangeWatcher.cs:406) and interprets Add and Del in the callback, which reach it from directory watches, since AdviseDirChange takes no flags and reports every kind. Ours copied both halves and so could never see the two kinds it filtered for on a file outside the watched roots.

What that cost is the case this PR exists for: a reference watched before the project producing it has been built is created, not written, and a rebuild that replaces it by renaming a temporary file over it is a delete plus an add. Neither told anyone, and the options kept the stamp they had. The comment now says which set is which and why.

Seventeen *FileChangeWatcher* tests pass.

xperiandri · 2026-09-26T12:47:28Z

+* Watch on-disk `-r:` reference assemblies via `IVsAsyncFileChangeEx2`, so F# project options are invalidated when a referenced assembly is rebuilt instead of waiting for a timestamp poll.
+* Watch on-disk `-r:` reference assemblies via `IVsAsyncFileChangeEx2`, so F# project options are invalidated when a referenced assembly is rebuilt instead of waiting for a timestamp poll. ([PR #20457](https://github.com/dotnet/fsharp/pull/20457))


Fixed in a3ec7f5e6c: the copy without the link is gone. The FCS entry this PR adds moved with it out of 11.0.100, which shipped while the PR was in review, into 11.0.200.

Linking the note added a copy of it instead of editing the first, and `main` has since opened 11.0.200 for SDK 11.0.200, leaving 11.0.100 shipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A watch on one file asked only for size and time, so a reference that did not exist when the watch started - the output of a project not built yet - or one replaced by a rename rather than a write told no one, and the options kept the stamp they had. The callback filter already accepted both kinds, because a directory watch reports them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

xperiandri requested a review from a team as a code owner September 5, 2026 19:26

github-project-automation Bot added this to F# Compiler and Tooling Sep 5, 2026

github-project-automation Bot moved this to New in F# Compiler and Tooling Sep 5, 2026