From 7f287a9b61b0a625d58384c791c8dec2b16ff9ef Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 17 Aug 2026 22:35:05 +0200 Subject: [PATCH 01/25] Record file-watching design review vs Roslyn 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> --- docs/ide/file-watching-design-review.md | 71 +++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 docs/ide/file-watching-design-review.md diff --git a/docs/ide/file-watching-design-review.md b/docs/ide/file-watching-design-review.md new file mode 100644 index 00000000000..9d5e52a32f7 --- /dev/null +++ b/docs/ide/file-watching-design-review.md @@ -0,0 +1,71 @@ +# F# file watching: design review vs Roslyn + +## Verdict + +The `FileChangeWatcher` working tree is **not** a file-change watcher. It is a pull-model I/O change: + +- `IFileSystem.OpenFileForReadShimAsync` + `Stream.ReadAllTextAsync` +- `FSharpFileSnapshot.CreateFromFileSystem` still versions the file as `GetLastWriteTimeShim(fileName).Ticks` at construction time + +That is useful (it stops blocking the ThreadPool on disk reads) and orthogonal to watching. It is not comparable to Roslyn's `FileChangeWatcher`. + +Roslyn's implementation is a **push** service over `IVsAsyncFileChangeEx2`: + +- directory subscriptions (`WatchedDirectory`) instead of one cookie per file +- `AsyncBatchingWorkQueue` (500 ms) so advise/unadvise is batched and never blocks the UI / thread pool +- free-threaded sinks (`IVsFreeThreadedFileChangeEvents2`) +- coalesced invalidation of metadata references (`FileWatchedReferenceFactory`) + +## What already exists in this repo + +Three layers, none of them `IVsAsyncFileChangeEx2`: + +| Layer | API | Role | +|---|---|---| +| `vsintegration/src/FSharp.ProjectSystem.Base/FileChangeManager.cs` | `IVsFileChangeEx` | Legacy project-system reload of nested items | +| `vsintegration/src/FSharp.LanguageService/FSharpSource.fs` (`SetDependencyFiles`) | `IVsFileChangeEx` | Deprecated unroslynized LS: watch `#r` / dependency files | +| **uncommitted** `stash@{7}` → `vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs` | `IVsFileChangeEx` | Intended modern replacement in `FSharp.Editor` | + +There is **no commit** (on any branch, stash patch, or GitHub search) that implements `IVsAsyncFileChangeEx2`. The work that was remembered as "the async watcher" is the stash below; it uses the older sync advise API, marshalled onto the UI thread. + +## Recovered implementation (`stash@{7}`) + +Stash: `WIP on revert-20080-t-gro-net11-upgrade: 8bf5dca37` + +Index commit that added the file: + +`54465595717b8bb746cb2633d5a4aa834888a481` + +Shape: + +- `IFileChangeWatcher.WatchFile` → `IVsFileChangeEx.AdviseFileChange` / `UnadviseFileChange` +- `FileChangeWatcherHub`: one cookie per path, ref-counted, 500 ms debounce +- Wired into `FSharpProjectOptionsReactor` for `-r:` reference assemblies +- Exposed as `FSharpProjectOptionsManager.WatchFile` so `WorkspaceExtensions` snapshot cache can share the same subscriptions + +This is the right *place* (FSharp.Editor, reference assemblies, debounce, share across projects). It is the wrong *shell API*: + +- `JoinableTaskFactory.Run` + `SwitchToMainThreadAsync` on every advise/unadvise +- no directory watches → N cookies for a NuGet cache +- no batching of subscribe/unsubscribe +- not free-threaded + +## Target design (Roslyn-shaped) + +1. Keep the async read shim. It is independent and should ship on its own. +2. Restore `FileChangeWatcher.fs` from `stash@{7}`, then replace `IVsFileChangeEx` with `IVsAsyncFileChangeEx2`: + - obtain the service asynchronously (same as Roslyn's `Task`) + - queue advise/unadvise on a 500 ms batching work queue; never `JTF.Run` + - subscribe to directories (NuGet cache, output folders) with extension filters; fall back to per-file only for stray paths + - implement `IVsFreeThreadedFileChangeEvents2` so callbacks do not hop to the UI thread +3. On a coalesced change: `checker.NotifyFileChanged` / `InvalidateConfiguration` for the owning project only. Stop O(N) `stat` of reference timestamps on every incremental check. +4. Scripts: watch `#r` / `#load` paths the same way; drop caret-move `NotifyFileChanged`. +5. Do not invent a second watcher. Project system (`FileChangeManager`) and FCS (`TimeStampCache`) should consume this service or stay on their existing contracts. + +## Suggested split + +- PR 1: async `OpenFileForReadShim` (already in the `FileChangeWatcher` worktree). +- PR 2: restore stash watcher as-is (`IVsFileChangeEx`) behind the existing reactor hook — functional, limited. +- PR 3: swap the shell API to `IVsAsyncFileChangeEx2` + directory batching. + +PR 2 is optional if PR 3 is done immediately. From 6246526e5d9cb8fd4264547225a0a31e12c3b168 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 17 Aug 2026 23:31:12 +0200 Subject: [PATCH 02/25] Add IVsAsyncFileChangeEx2 file change watcher adapter 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> --- .../src/FSharp.Editor/FSharp.Editor.fsproj | 1 + .../LanguageService/FileChangeWatcher.fs | 353 ++++++++++++++++++ 2 files changed, 354 insertions(+) create mode 100644 vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index 319bdd5a264..6c510a77c14 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -55,6 +55,7 @@ + diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs new file mode 100644 index 00000000000..605864f82c7 --- /dev/null +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -0,0 +1,353 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace Microsoft.VisualStudio.FSharp.Editor + +open System +open System.Collections.Concurrent +open System.Collections.Generic +open System.Threading +open System.Threading.Tasks +open Microsoft.VisualStudio +open Microsoft.VisualStudio.Shell +open Microsoft.VisualStudio.Shell.Interop + +// Push-based file watching for FSharp.Editor, modelled on Roslyn's +// Microsoft.VisualStudio.LanguageServices FileChangeWatcher (which is internal and not +// exposed through ExternalAccess.FSharp). Uses the free-threaded IVsAsyncFileChangeEx2 +// service: subscriptions are batched off the UI thread and callbacks never marshal to it. + +/// A directory to watch recursively (with optional extension filters) so that individual +/// files under it don't each need their own advise cookie. +[] +type internal WatchedDirectory(path: string, extensionFilters: string list) = + let path = + if path.EndsWith(string IO.Path.DirectorySeparatorChar) then + path + else + path + string IO.Path.DirectorySeparatorChar + + do + for filter in extensionFilters do + if not (filter.StartsWith ".") then + invalidArg (nameof extensionFilters) $"Filter '{filter}' must start with a period." + + member _.Path = path + member _.ExtensionFilters = extensionFilters + + static member FilePathCoveredByWatchedDirectories(watchedDirectories: WatchedDirectory list, filePath: string) = + watchedDirectories + |> List.exists (fun w -> + filePath.StartsWith(w.Path, StringComparison.OrdinalIgnoreCase) + && (w.ExtensionFilters.IsEmpty + || w.ExtensionFilters + |> List.exists (fun f -> filePath.EndsWith(f, StringComparison.OrdinalIgnoreCase)))) + +/// A single watched file; disposing stops watching. +type internal IFSharpWatchedFile = + inherit IDisposable + +/// A group of file/directory watches sharing one event sink. Disposing unsubscribes everything. +type internal IFSharpFileChangeContext = + inherit IDisposable + + [] + abstract FileChanged: IEvent + + /// Starts watching a file without waiting for the OS registration. No-op (but still valid + /// to dispose) when the path is already covered by one of the context's watched directories. + abstract EnqueueWatchingFile: filePath: string -> IFSharpWatchedFile + +type internal IFSharpFileChangeWatcher = + abstract CreateContext: watchedDirectories: WatchedDirectory list -> IFSharpFileChangeContext + +[] +module private FileChangeWatcherImpl = + + // Same flags Roslyn uses for both subscribing and filtering callbacks. + let watchFlags = _VSFILECHANGEFLAGS.VSFILECHG_Size ||| _VSFILECHANGEFLAGS.VSFILECHG_Time + + let relevantFlags = + _VSFILECHANGEFLAGS.VSFILECHG_Time + ||| _VSFILECHANGEFLAGS.VSFILECHG_Add + ||| _VSFILECHANGEFLAGS.VSFILECHG_Del + ||| _VSFILECHANGEFLAGS.VSFILECHG_Size + + /// Empirically strong batching window during high activity (solution open/close); see + /// Roslyn's FileChangeWatcher. + let batchingDelay = TimeSpan.FromMilliseconds 500. + +[] +type internal FSharpWatchedFileToken() = + member val Cookie: uint32 option = None with get, set + +/// Subscription operations queued for batched application against the file change service. +type private WatcherOperation = + | WatchDir of path: string * filters: string list * sink: IVsFreeThreadedFileChangeEvents2 * cookies: List + | WatchFiles of paths: string list * tokens: FSharpWatchedFileToken list * sink: IVsFreeThreadedFileChangeEvents2 + | UnwatchFiles of tokens: FSharpWatchedFileToken list + | UnwatchDirs of cookies: List + +[] +type internal FSharpFileChangeWatcher(fileChangeService: Task) = + + let applyBatch (service: IVsAsyncFileChangeEx2) (ops: WatcherOperation list) = + task { + // Coalesce adjacent same-kind operations into single service calls, preserving order + // between kinds (a watch enqueued before an unwatch must be applied first). + let mutable pending = ops + + while not pending.IsEmpty do + match pending with + | [] -> () + | WatchDir(path, filters, sink, cookies) :: rest -> + pending <- rest + let! cookie = service.AdviseDirChangeAsync(path, true, sink, CancellationToken.None) + cookies.Add cookie + + if not filters.IsEmpty then + do! service.FilterDirectoryChangesAsync(cookie, List.toArray filters, CancellationToken.None) + + | WatchFiles _ :: _ -> + let batch = pending |> List.takeWhile (function WatchFiles _ -> true | _ -> false) + pending <- pending |> List.skip batch.Length + + let paths = batch |> List.collect (function WatchFiles(p, _, _) -> p | _ -> []) + let tokens = batch |> List.collect (function WatchFiles(_, t, _) -> t | _ -> []) + let sink = batch |> List.pick (function WatchFiles(_, _, s) -> Some s | _ -> None) + + let! cookies = service.AdviseFileChangesAsync(List.toArray paths, watchFlags, sink, CancellationToken.None) + + (tokens, List.ofArray cookies) + ||> List.iter2 (fun token cookie -> token.Cookie <- Some cookie) + + | UnwatchFiles _ :: _ -> + let batch = pending |> List.takeWhile (function UnwatchFiles _ -> true | _ -> false) + pending <- pending |> List.skip batch.Length + + let cookies = + batch + |> List.collect (function UnwatchFiles t -> t | _ -> []) + |> List.choose (fun token -> token.Cookie) + + if not cookies.IsEmpty then + let! _ = service.UnadviseFileChangesAsync(List.toArray cookies, CancellationToken.None) + () + + | UnwatchDirs cookies :: rest -> + pending <- rest + + if cookies.Count > 0 then + let! _ = service.UnadviseDirChangesAsync(cookies.ToArray(), CancellationToken.None) + () + } + + // Single consumer loop: waits for the first queued operation, sleeps out the batching + // window, drains the queue and applies everything in one pass. Nothing ever blocks on the + // service being available. + let agent = + MailboxProcessor.Start(fun inbox -> + async { + while true do + try + let! first = inbox.Receive() + do! Async.Sleep(int batchingDelay.TotalMilliseconds) + + let ops = ResizeArray [ first ] + let mutable draining = true + + while draining do + match! inbox.TryReceive 0 with + | Some op -> ops.Add op + | None -> draining <- false + + let! service = fileChangeService |> Async.AwaitTask + do! applyBatch service (List.ofSeq ops) |> Async.AwaitTask + with _ -> + // Never let a failed advise/unadvise (e.g. non-existent path) kill the + // subscription loop; we simply won't get events for that path. + () + }) + + member private _.Enqueue(op: WatcherOperation) = agent.Post op + + /// Production factory: obtains SVsFileChangeEx asynchronously without blocking any + /// background thread on UI-thread availability. + static member CreateDefaultServiceTask() = + task { + let! service = AsyncServiceProvider.GlobalProvider.GetServiceAsync(typeof) + return service :?> IVsAsyncFileChangeEx2 + } + + interface IFSharpFileChangeWatcher with + member _.CreateContext(watchedDirectories) = + new FileChangeContext(agent.Post, watchedDirectories) :> IFSharpFileChangeContext + +and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watchedDirectories: WatchedDirectory list) as this = + + let gate = obj () + let mutable disposed = false + let activeFileTokens = HashSet() + let directoryCookies = List() + let fileChanged = Event() + + let raiseChanges (count: uint32) (files: string[]) (changeFlags: uint32[]) = + for i in 0 .. int count - 1 do + if (enum<_VSFILECHANGEFLAGS> (int changeFlags[i]) &&& relevantFlags) <> enum<_VSFILECHANGEFLAGS> 0 then + fileChanged.Trigger files[i] + + VSConstants.S_OK + + do + for watchedDirectory in watchedDirectories do + enqueue ( + WatchDir( + watchedDirectory.Path, + watchedDirectory.ExtensionFilters, + this :> IVsFreeThreadedFileChangeEvents2, + directoryCookies + ) + ) + + member private _.StopWatchingFile(token: FSharpWatchedFileToken) = + lock gate (fun () -> activeFileTokens.Remove token |> ignore) + enqueue (UnwatchFiles [ token ]) + + interface IFSharpFileChangeContext with + [] + member _.FileChanged = fileChanged.Publish + + member _.EnqueueWatchingFile filePath = + if WatchedDirectory.FilePathCoveredByWatchedDirectories(watchedDirectories, filePath) then + // Covered by a directory watch; nothing extra to subscribe. + { new IFSharpWatchedFile with + member _.Dispose() = () + } + else + let token = FSharpWatchedFileToken() + lock gate (fun () -> activeFileTokens.Add token |> ignore) + enqueue (WatchFiles([ filePath ], [ token ], this :> IVsFreeThreadedFileChangeEvents2)) + + { new IFSharpWatchedFile with + member _.Dispose() = this.StopWatchingFile token + } + + interface IDisposable with + member _.Dispose() = + let alreadyDisposed = lock gate (fun () -> + let d = disposed + disposed <- true + d) + + if not alreadyDisposed then + enqueue (UnwatchDirs directoryCookies) + enqueue (UnwatchFiles(lock gate (fun () -> List.ofSeq activeFileTokens))) + + // Free-threaded sink: callbacks arrive on background threads and stay there. + interface IVsFreeThreadedFileChangeEvents2 with + member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + member _.DirectoryChanged _ = VSConstants.E_NOTIMPL + member _.DirectoryChangedEx(_, _) = VSConstants.E_NOTIMPL + member _.DirectoryChangedEx2(_, cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + + interface IVsFreeThreadedFileChangeEvents with + member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + member _.DirectoryChanged _ = VSConstants.E_NOTIMPL + member _.DirectoryChangedEx(_, _) = VSConstants.E_NOTIMPL + + interface IVsFileChangeEvents with + member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + member _.DirectoryChanged _ = VSConstants.E_NOTIMPL + +/// Ref-counted, debounced watching of reference assemblies (or any other off-workspace files), +/// modelled on Roslyn's ReferenceFileChangeTracker. Multiple projects watching the same dll +/// share one subscription; bursts of writes produce a single callback per path. +[] +type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, onChanged: string -> unit) = + + /// Delay between the last observed change to a path and the callback: a rebuild typically + /// writes a temp file then renames, producing several rapid notifications. + static let notificationDelay = TimeSpan.FromSeconds 2. + + let gate = obj () + let mutable disposed = false + let watchedFiles = Dictionary(StringComparer.OrdinalIgnoreCase) + let pendingTimers = ConcurrentDictionary(StringComparer.OrdinalIgnoreCase) + + // On each platform there is a place framework reference assemblies live; these rarely change + // but account for most watched paths, so cover them with directory watches up front. + static let defaultWatchedDirectories () = + let dotnetRoot = Environment.GetEnvironmentVariable "DOTNET_ROOT" + + [ + if not (String.IsNullOrEmpty dotnetRoot) then + IO.Path.Combine(dotnetRoot, "packs") + + IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.ProgramFiles, "dotnet", "packs") + + IO.Path.Combine( + Environment.GetFolderPath Environment.SpecialFolder.ProgramFilesX86, + "Reference Assemblies", + "Microsoft", + "Framework" + ) + + IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.UserProfile, ".nuget", "packages") + ] + |> List.distinct + |> List.map (fun d -> WatchedDirectory(d, [ ".dll" ])) + + let context = + lazy + (let ctx = watcher.CreateContext(defaultWatchedDirectories ()) + + ctx.FileChanged.Add(fun path -> + let fire (_: obj) = + pendingTimers.TryRemove path + |> function + | true, timer -> timer.Dispose() + | _ -> () + + // Only notify for paths someone is actually watching; directory watches + // cover whole trees. + let isWatched = lock gate (fun () -> watchedFiles.ContainsKey path) + + if isWatched then + onChanged path + + let timer = pendingTimers.GetOrAdd(path, fun _ -> new Timer(fire, null, Timeout.Infinite, Timeout.Infinite)) + timer.Change(notificationDelay, Timeout.InfiniteTimeSpan) |> ignore) + + ctx) + + /// Starts watching a path, ref-counted. Call StopWatchingReference exactly once per start. + member _.StartWatchingReference(fullFilePath: string) = + lock gate (fun () -> + if not disposed then + match watchedFiles.TryGetValue fullFilePath with + | true, (token, count) -> watchedFiles[fullFilePath] <- (token, count + 1) + | _ -> watchedFiles[fullFilePath] <- (context.Value.EnqueueWatchingFile fullFilePath, 1)) + + member _.StopWatchingReference(fullFilePath: string) = + lock gate (fun () -> + if not disposed then + match watchedFiles.TryGetValue fullFilePath with + | true, (token, 1) -> + watchedFiles.Remove fullFilePath |> ignore + token.Dispose() + | true, (token, count) -> watchedFiles[fullFilePath] <- (token, count - 1) + | _ -> ()) + + interface IDisposable with + member _.Dispose() = + lock gate (fun () -> + if not disposed then + disposed <- true + watchedFiles.Clear() + + for KeyValue(_, timer) in pendingTimers do + timer.Dispose() + + pendingTimers.Clear() + + if context.IsValueCreated then + context.Value.Dispose()) From dc012040eae79e6e0167c50084b91173c2f14c83 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 17 Aug 2026 23:36:03 +0200 Subject: [PATCH 03/25] Wire reference file watching into FSharpProjectOptionsReactor 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> --- .../FSharpProjectOptionsManager.fs | 53 +++++++++++++++++-- .../LanguageService/LanguageService.fs | 6 ++- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index db73206996b..c1dda3ba579 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -116,7 +116,7 @@ type private FSharpProjectOptionsMessage = | ClearSingleFileOptionsCache of DocumentId [] -type private FSharpProjectOptionsReactor(checker: FSharpChecker) = +type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatcher: IFSharpFileChangeWatcher option) = let cancellationTokenSource = new CancellationTokenSource() // Store command line options @@ -128,6 +128,44 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let cache = ConcurrentDictionary() + // Push invalidation for on-disk '-r:' reference assemblies (not tracked by the Roslyn + // workspace): when one changes after an external rebuild, drop the cached options of every + // project referencing it instead of waiting for a timestamp poll to notice. + let referenceWatches = ConcurrentDictionary() + + let onWatchedReferenceChanged (path: string) = + for KeyValue(projectId, paths) in referenceWatches do + if + paths + |> Array.exists (fun p -> String.Equals(p, path, StringComparison.OrdinalIgnoreCase)) + then + match cache.TryRemove projectId with + | true, (_, _, projectOptions) -> checker.InvalidateConfiguration(projectOptions, userOpName = "onWatchedReferenceChanged") + | _ -> () + + let referenceChangeTracker = + fileChangeWatcher + |> Option.map (fun watcher -> new FSharpReferenceChangeTracker(watcher, onWatchedReferenceChanged)) + + let clearReferenceWatches (projectId: ProjectId) = + match referenceWatches.TryRemove projectId, referenceChangeTracker with + | (true, paths), Some tracker -> paths |> Array.iter (fun p -> tracker.StopWatchingReference p) + | _ -> () + + let watchReferenceFiles (projectId: ProjectId) (projectOptions: FSharpProjectOptions) = + referenceChangeTracker + |> Option.iter (fun tracker -> + clearReferenceWatches projectId + + let paths = + projectOptions.OtherOptions + |> Array.filter (fun x -> x.StartsWith("-r:", StringComparison.Ordinal)) + |> Array.map (fun x -> x.Substring "-r:".Length) + + if paths.Length > 0 then + paths |> Array.iter (fun p -> tracker.StartWatchingReference p) + referenceWatches[projectId] <- paths) + let singleFileCache = ConcurrentDictionary() @@ -431,6 +469,8 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = cache.[projectId] <- struct (project, parsingOptions, projectOptions) + watchReferenceFiles projectId projectOptions + return ValueSome struct (parsingOptions, projectOptions) | true, struct (oldProject, parsingOptions, projectOptions) -> @@ -516,6 +556,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = | _ -> () legacyProjectSites.TryRemove(projectId) |> ignore + clearReferenceWatches projectId | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> @@ -558,18 +599,24 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = singleFileCache.Clear() lastSuccessfulCompilations.Clear() + for projectId in referenceWatches.Keys |> Array.ofSeq do + clearReferenceWatches projectId + member _.ScriptUpdated = scriptUpdatedEvent.Publish interface IDisposable with member _.Dispose() = + referenceChangeTracker + |> Option.iter (fun tracker -> (tracker :> IDisposable).Dispose()) + cancellationTokenSource.Cancel() cancellationTokenSource.Dispose() (agent :> IDisposable).Dispose() /// Manages mappings of Roslyn workspace Projects/Documents to FCS. -type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace) = +type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace, ?fileChangeWatcher: IFSharpFileChangeWatcher) = - let reactor = new FSharpProjectOptionsReactor(checker) + let reactor = new FSharpProjectOptionsReactor(checker, fileChangeWatcher) do // We need to listen to this event for lifecycle purposes. diff --git a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs index 427baf0c6ab..3946e523630 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs @@ -205,7 +205,11 @@ type internal FSharpWorkspaceServiceFactory |> CancellableTask.startAsTask CancellationToken.None |> ignore) - let optionsManager = FSharpProjectOptionsManager(checker, workspace) + let fileChangeWatcher = + FSharpFileChangeWatcher(FSharpFileChangeWatcher.CreateDefaultServiceTask()) + + let optionsManager = + FSharpProjectOptionsManager(checker, workspace, fileChangeWatcher) { new IFSharpWorkspaceService with member _.Checker = checker From b762cc46dc66a6787cf9ae8fc9315396111290c3 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Tue, 18 Aug 2026 00:01:53 +0200 Subject: [PATCH 04/25] Add FileChangeWatcher unit tests and VS release notes 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> --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../LanguageService/FileChangeWatcher.fs | 87 ++++++++++--- .../FSharp.Editor.Tests.fsproj | 1 + .../FileChangeWatcherTests.fs | 121 ++++++++++++++++++ 4 files changed, 189 insertions(+), 21 deletions(-) create mode 100644 vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index e6034dca8df..a1049888fff 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -12,6 +12,7 @@ * 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)) * Avoid using `cancellableTask` in `DocumentCache`; the editor cache now uses direct `CancellationToken`-aware `task` wrappers, avoiding the background `Task.Run` offload and a larger wrapper closure from the `cancellableTask` builder. ([Issue #20268](https://github.com/dotnet/fsharp/issues/20268)) * Cache document diagnostics by version stamp, so an unchanged document is not reanalyzed on every crawler pass. ([Issue #20120](https://github.com/dotnet/fsharp/issues/20120), [PR #20121](https://github.com/dotnet/fsharp/pull/20121)) +* 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. * 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/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 605864f82c7..dabc278ae2a 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -64,7 +64,8 @@ type internal IFSharpFileChangeWatcher = module private FileChangeWatcherImpl = // Same flags Roslyn uses for both subscribing and filtering callbacks. - let watchFlags = _VSFILECHANGEFLAGS.VSFILECHG_Size ||| _VSFILECHANGEFLAGS.VSFILECHG_Time + let watchFlags = + _VSFILECHANGEFLAGS.VSFILECHG_Size ||| _VSFILECHANGEFLAGS.VSFILECHG_Time let relevantFlags = _VSFILECHANGEFLAGS.VSFILECHG_Time @@ -108,12 +109,31 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task - let batch = pending |> List.takeWhile (function WatchFiles _ -> true | _ -> false) + let batch = + pending + |> List.takeWhile (function + | WatchFiles _ -> true + | _ -> false) + pending <- pending |> List.skip batch.Length - let paths = batch |> List.collect (function WatchFiles(p, _, _) -> p | _ -> []) - let tokens = batch |> List.collect (function WatchFiles(_, t, _) -> t | _ -> []) - let sink = batch |> List.pick (function WatchFiles(_, _, s) -> Some s | _ -> None) + let paths = + batch + |> List.collect (function + | WatchFiles(p, _, _) -> p + | _ -> []) + + let tokens = + batch + |> List.collect (function + | WatchFiles(_, t, _) -> t + | _ -> []) + + let sink = + batch + |> List.pick (function + | WatchFiles(_, _, s) -> Some s + | _ -> None) let! cookies = service.AdviseFileChangesAsync(List.toArray paths, watchFlags, sink, CancellationToken.None) @@ -121,12 +141,19 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task List.iter2 (fun token cookie -> token.Cookie <- Some cookie) | UnwatchFiles _ :: _ -> - let batch = pending |> List.takeWhile (function UnwatchFiles _ -> true | _ -> false) + let batch = + pending + |> List.takeWhile (function + | UnwatchFiles _ -> true + | _ -> false) + pending <- pending |> List.skip batch.Length let cookies = batch - |> List.collect (function UnwatchFiles t -> t | _ -> []) + |> List.collect (function + | UnwatchFiles t -> t + | _ -> []) |> List.choose (fun token -> token.Cookie) if not cookies.IsEmpty then @@ -192,7 +219,10 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc let raiseChanges (count: uint32) (files: string[]) (changeFlags: uint32[]) = for i in 0 .. int count - 1 do - if (enum<_VSFILECHANGEFLAGS> (int changeFlags[i]) &&& relevantFlags) <> enum<_VSFILECHANGEFLAGS> 0 then + if + (enum<_VSFILECHANGEFLAGS> (int changeFlags[i]) &&& relevantFlags) + <> enum<_VSFILECHANGEFLAGS> 0 + then fileChanged.Trigger files[i] VSConstants.S_OK @@ -233,10 +263,11 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc interface IDisposable with member _.Dispose() = - let alreadyDisposed = lock gate (fun () -> - let d = disposed - disposed <- true - d) + let alreadyDisposed = + lock gate (fun () -> + let d = disposed + disposed <- true + d) if not alreadyDisposed then enqueue (UnwatchDirs directoryCookies) @@ -244,34 +275,46 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc // Free-threaded sink: callbacks arrive on background threads and stay there. interface IVsFreeThreadedFileChangeEvents2 with - member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = + raiseChanges cChanges rgpszFile rggrfChange + member _.DirectoryChanged _ = VSConstants.E_NOTIMPL member _.DirectoryChangedEx(_, _) = VSConstants.E_NOTIMPL - member _.DirectoryChangedEx2(_, cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + + member _.DirectoryChangedEx2(_, cChanges, rgpszFile, rggrfChange) = + raiseChanges cChanges rgpszFile rggrfChange interface IVsFreeThreadedFileChangeEvents with - member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = + raiseChanges cChanges rgpszFile rggrfChange + member _.DirectoryChanged _ = VSConstants.E_NOTIMPL member _.DirectoryChangedEx(_, _) = VSConstants.E_NOTIMPL interface IVsFileChangeEvents with - member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = raiseChanges cChanges rgpszFile rggrfChange + member _.FilesChanged(cChanges, rgpszFile, rggrfChange) = + raiseChanges cChanges rgpszFile rggrfChange + member _.DirectoryChanged _ = VSConstants.E_NOTIMPL /// Ref-counted, debounced watching of reference assemblies (or any other off-workspace files), /// modelled on Roslyn's ReferenceFileChangeTracker. Multiple projects watching the same dll /// share one subscription; bursts of writes produce a single callback per path. [] -type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, onChanged: string -> unit) = +type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, onChanged: string -> unit, ?notificationDelay: TimeSpan) = /// Delay between the last observed change to a path and the callback: a rebuild typically /// writes a temp file then renames, producing several rapid notifications. - static let notificationDelay = TimeSpan.FromSeconds 2. + let notificationDelay = defaultArg notificationDelay (TimeSpan.FromSeconds 2.) let gate = obj () let mutable disposed = false - let watchedFiles = Dictionary(StringComparer.OrdinalIgnoreCase) - let pendingTimers = ConcurrentDictionary(StringComparer.OrdinalIgnoreCase) + + let watchedFiles = + Dictionary(StringComparer.OrdinalIgnoreCase) + + let pendingTimers = + ConcurrentDictionary(StringComparer.OrdinalIgnoreCase) // On each platform there is a place framework reference assemblies live; these rarely change // but account for most watched paths, so cover them with directory watches up front. @@ -314,7 +357,9 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on if isWatched then onChanged path - let timer = pendingTimers.GetOrAdd(path, fun _ -> new Timer(fire, null, Timeout.Infinite, Timeout.Infinite)) + let timer = + pendingTimers.GetOrAdd(path, fun _ -> new Timer(fire, null, Timeout.Infinite, Timeout.Infinite)) + timer.Change(notificationDelay, Timeout.InfiniteTimeSpan) |> ignore) ctx) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index ecce1205b8c..01b6be54542 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -82,6 +82,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs new file mode 100644 index 00000000000..829cfd68226 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Editor.Tests + +open System +open System.Threading +open Xunit +open Microsoft.VisualStudio.FSharp.Editor + +type private MockFileChangeContext() = + let fileChanged = Event() + let watched = ResizeArray() + + member _.WatchedFiles = List.ofSeq watched + member _.Fire path = fileChanged.Trigger path + + interface IFSharpFileChangeContext with + [] + member _.FileChanged = fileChanged.Publish + + member _.EnqueueWatchingFile path = + watched.Add path + + { new IFSharpWatchedFile with + member _.Dispose() = watched.Remove path |> ignore + } + + member _.Dispose() = watched.Clear() + +type private MockFileChangeWatcher() = + let mutable context: MockFileChangeContext option = None + + member _.Context = context + + interface IFSharpFileChangeWatcher with + member _.CreateContext _ = + let ctx = new MockFileChangeContext() + context <- Some ctx + ctx :> IFSharpFileChangeContext + +module FileChangeWatcherTests = + + let private testDelay = TimeSpan.FromMilliseconds 50. + + [] + let ``WatchedDirectory covers files under it matching the extension filter`` () = + let dirs = [ WatchedDirectory(@"C:\refs", [ ".dll" ]) ] + + Assert.True(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\refs\sub\a.dll")) + Assert.True(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\REFS\A.DLL")) + Assert.False(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\refs\a.xml")) + Assert.False(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\other\a.dll")) + + [] + let ``WatchedDirectory without filters covers any file under it`` () = + let dirs = [ WatchedDirectory(@"C:\refs", []) ] + + Assert.True(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\refs\a.xml")) + Assert.False(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\refsx\a.xml")) + + [] + let ``Tracker ref-counts subscriptions per path`` () = + let watcher = MockFileChangeWatcher() + use tracker = new FSharpReferenceChangeTracker(watcher, ignore, testDelay) + + tracker.StartWatchingReference @"C:\x\a.dll" + tracker.StartWatchingReference @"C:\x\a.dll" + tracker.StartWatchingReference @"C:\x\b.dll" + + Assert.Equal([ @"C:\x\a.dll"; @"C:\x\b.dll" ], watcher.Context.Value.WatchedFiles) + + tracker.StopWatchingReference @"C:\x\a.dll" + Assert.Contains(@"C:\x\a.dll", watcher.Context.Value.WatchedFiles) + + tracker.StopWatchingReference @"C:\x\a.dll" + Assert.Equal([ @"C:\x\b.dll" ], watcher.Context.Value.WatchedFiles) + + [] + let ``Tracker debounces bursts into a single callback for watched paths only`` () = + let watcher = MockFileChangeWatcher() + let calls = ResizeArray() + use signal = new ManualResetEventSlim(false) + + use tracker = + new FSharpReferenceChangeTracker( + watcher, + (fun path -> + lock calls (fun () -> calls.Add path) + signal.Set()), + testDelay + ) + + tracker.StartWatchingReference @"C:\x\a.dll" + let context = watcher.Context.Value + + context.Fire @"C:\x\a.dll" + context.Fire @"C:\x\a.dll" + context.Fire @"C:\x\unwatched.dll" + + Assert.True(signal.Wait(TimeSpan.FromSeconds 10.)) + // Allow a trailing duplicate timer to fire if one was pending. + Thread.Sleep(testDelay + testDelay) + + Assert.Equal([ @"C:\x\a.dll" ], lock calls (fun () -> List.ofSeq calls)) + + [] + let ``Disposed tracker ignores further changes`` () = + let watcher = MockFileChangeWatcher() + let mutable called = false + + let tracker = + new FSharpReferenceChangeTracker(watcher, (fun _ -> called <- true), testDelay) + + tracker.StartWatchingReference @"C:\x\a.dll" + let context = watcher.Context.Value + (tracker :> IDisposable).Dispose() + + context.Fire @"C:\x\a.dll" + Thread.Sleep(testDelay + testDelay) + + Assert.False called From 934979f25444db6b0827c57ab3d468f440aea778 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 21:26:38 +0200 Subject: [PATCH 05/25] Link the file-watching release note to its PR --- 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 a1049888fff..dbb51e259f2 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -13,6 +13,7 @@ * Avoid using `cancellableTask` in `DocumentCache`; the editor cache now uses direct `CancellationToken`-aware `task` wrappers, avoiding the background `Task.Run` offload and a larger wrapper closure from the `cancellableTask` builder. ([Issue #20268](https://github.com/dotnet/fsharp/issues/20268)) * Cache document diagnostics by version stamp, so an unchanged document is not reanalyzed on every crawler pass. ([Issue #20120](https://github.com/dotnet/fsharp/issues/20120), [PR #20121](https://github.com/dotnet/fsharp/pull/20121)) * 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)) * 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 a75cac6c988e18f5355b29e331b11ba1c7c82779 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 22:34:55 +0200 Subject: [PATCH 06/25] Address review: mandatory watcher, voption, illib string helpers * 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. --- .../FSharpProjectOptionsManager.fs | 46 +++---- .../LanguageService/FileChangeWatcher.fs | 126 ++++++++---------- .../FileChangeWatcherTests.fs | 4 +- 3 files changed, 80 insertions(+), 96 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index c1dda3ba579..2af5298b7c7 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -21,6 +21,7 @@ open System.Windows open Microsoft.VisualStudio open FSharp.Compiler.Text open Microsoft.VisualStudio.TextManager.Interop +open Internal.Utilities.Library #nowarn "57" @@ -116,7 +117,7 @@ type private FSharpProjectOptionsMessage = | ClearSingleFileOptionsCache of DocumentId [] -type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatcher: IFSharpFileChangeWatcher option) = +type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatcher: IFSharpFileChangeWatcher) = let cancellationTokenSource = new CancellationTokenSource() // Store command line options @@ -131,40 +132,39 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch // Push invalidation for on-disk '-r:' reference assemblies (not tracked by the Roslyn // workspace): when one changes after an external rebuild, drop the cached options of every // project referencing it instead of waiting for a timestamp poll to notice. - let referenceWatches = ConcurrentDictionary() + let referenceWatches = ConcurrentDictionary>() let onWatchedReferenceChanged (path: string) = for KeyValue(projectId, paths) in referenceWatches do - if - paths - |> Array.exists (fun p -> String.Equals(p, path, StringComparison.OrdinalIgnoreCase)) - then + if paths.Contains path then match cache.TryRemove projectId with | true, (_, _, projectOptions) -> checker.InvalidateConfiguration(projectOptions, userOpName = "onWatchedReferenceChanged") | _ -> () let referenceChangeTracker = - fileChangeWatcher - |> Option.map (fun watcher -> new FSharpReferenceChangeTracker(watcher, onWatchedReferenceChanged)) + new FSharpReferenceChangeTracker(fileChangeWatcher, onWatchedReferenceChanged) let clearReferenceWatches (projectId: ProjectId) = - match referenceWatches.TryRemove projectId, referenceChangeTracker with - | (true, paths), Some tracker -> paths |> Array.iter (fun p -> tracker.StopWatchingReference p) + match referenceWatches.TryRemove projectId with + | true, paths -> + for path in paths do + referenceChangeTracker.StopWatchingReference path | _ -> () let watchReferenceFiles (projectId: ProjectId) (projectOptions: FSharpProjectOptions) = - referenceChangeTracker - |> Option.iter (fun tracker -> - clearReferenceWatches projectId + clearReferenceWatches projectId + + let paths = HashSet(StringComparer.OrdinalIgnoreCase) - let paths = - projectOptions.OtherOptions - |> Array.filter (fun x -> x.StartsWith("-r:", StringComparison.Ordinal)) - |> Array.map (fun x -> x.Substring "-r:".Length) + for option in projectOptions.OtherOptions do + if option.StartsWithOrdinal "-r:" then + paths.Add(option.Substring "-r:".Length) |> ignore - if paths.Length > 0 then - paths |> Array.iter (fun p -> tracker.StartWatchingReference p) - referenceWatches[projectId] <- paths) + if paths.Count > 0 then + for path in paths do + referenceChangeTracker.StartWatchingReference path + + referenceWatches[projectId] <- paths let singleFileCache = ConcurrentDictionary() @@ -606,15 +606,13 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch interface IDisposable with member _.Dispose() = - referenceChangeTracker - |> Option.iter (fun tracker -> (tracker :> IDisposable).Dispose()) - + (referenceChangeTracker :> IDisposable).Dispose() cancellationTokenSource.Cancel() cancellationTokenSource.Dispose() (agent :> IDisposable).Dispose() /// Manages mappings of Roslyn workspace Projects/Documents to FCS. -type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace, ?fileChangeWatcher: IFSharpFileChangeWatcher) = +type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace, fileChangeWatcher: IFSharpFileChangeWatcher) = let reactor = new FSharpProjectOptionsReactor(checker, fileChangeWatcher) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index dabc278ae2a..ede8eddb4d1 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -11,6 +11,8 @@ open Microsoft.VisualStudio open Microsoft.VisualStudio.Shell open Microsoft.VisualStudio.Shell.Interop +open Internal.Utilities.Library + // Push-based file watching for FSharp.Editor, modelled on Roslyn's // Microsoft.VisualStudio.LanguageServices FileChangeWatcher (which is internal and not // exposed through ExternalAccess.FSharp). Uses the free-threaded IVsAsyncFileChangeEx2 @@ -21,14 +23,14 @@ open Microsoft.VisualStudio.Shell.Interop [] type internal WatchedDirectory(path: string, extensionFilters: string list) = let path = - if path.EndsWith(string IO.Path.DirectorySeparatorChar) then + if path.EndsWithOrdinal(string IO.Path.DirectorySeparatorChar) then path else - path + string IO.Path.DirectorySeparatorChar + $"{path}{IO.Path.DirectorySeparatorChar}" do for filter in extensionFilters do - if not (filter.StartsWith ".") then + if not (filter.StartsWithOrdinal ".") then invalidArg (nameof extensionFilters) $"Filter '{filter}' must start with a period." member _.Path = path @@ -39,8 +41,7 @@ type internal WatchedDirectory(path: string, extensionFilters: string list) = |> List.exists (fun w -> filePath.StartsWith(w.Path, StringComparison.OrdinalIgnoreCase) && (w.ExtensionFilters.IsEmpty - || w.ExtensionFilters - |> List.exists (fun f -> filePath.EndsWith(f, StringComparison.OrdinalIgnoreCase)))) + || w.ExtensionFilters |> List.exists filePath.EndsWithOrdinalIgnoreCase)) /// A single watched file; disposing stops watching. type internal IFSharpWatchedFile = @@ -79,7 +80,7 @@ module private FileChangeWatcherImpl = [] type internal FSharpWatchedFileToken() = - member val Cookie: uint32 option = None with get, set + member val Cookie: uint32 voption = ValueNone with get, set /// Subscription operations queued for batched application against the file change service. type private WatcherOperation = @@ -91,77 +92,61 @@ type private WatcherOperation = [] type internal FSharpFileChangeWatcher(fileChangeService: Task) = - let applyBatch (service: IVsAsyncFileChangeEx2) (ops: WatcherOperation list) = + let applyBatch (service: IVsAsyncFileChangeEx2) (ops: ResizeArray) = task { // Coalesce adjacent same-kind operations into single service calls, preserving order // between kinds (a watch enqueued before an unwatch must be applied first). - let mutable pending = ops + let mutable i = 0 - while not pending.IsEmpty do - match pending with - | [] -> () - | WatchDir(path, filters, sink, cookies) :: rest -> - pending <- rest + while i < ops.Count do + match ops[i] with + | WatchDir(path, filters, sink, cookies) -> + i <- i + 1 let! cookie = service.AdviseDirChangeAsync(path, true, sink, CancellationToken.None) cookies.Add cookie if not filters.IsEmpty then do! service.FilterDirectoryChangesAsync(cookie, List.toArray filters, CancellationToken.None) - | WatchFiles _ :: _ -> - let batch = - pending - |> List.takeWhile (function - | WatchFiles _ -> true - | _ -> false) - - pending <- pending |> List.skip batch.Length - - let paths = - batch - |> List.collect (function - | WatchFiles(p, _, _) -> p - | _ -> []) - - let tokens = - batch - |> List.collect (function - | WatchFiles(_, t, _) -> t - | _ -> []) - - let sink = - batch - |> List.pick (function - | WatchFiles(_, _, s) -> Some s - | _ -> None) - - let! cookies = service.AdviseFileChangesAsync(List.toArray paths, watchFlags, sink, CancellationToken.None) - - (tokens, List.ofArray cookies) - ||> List.iter2 (fun token cookie -> token.Cookie <- Some cookie) - - | UnwatchFiles _ :: _ -> - let batch = - pending - |> List.takeWhile (function - | UnwatchFiles _ -> true - | _ -> false) - - pending <- pending |> List.skip batch.Length - - let cookies = - batch - |> List.collect (function - | UnwatchFiles t -> t - | _ -> []) - |> List.choose (fun token -> token.Cookie) - - if not cookies.IsEmpty then - let! _ = service.UnadviseFileChangesAsync(List.toArray cookies, CancellationToken.None) + | WatchFiles(_, _, sink) -> + let paths = ResizeArray() + let tokens = ResizeArray() + let mutable sameKind = true + + while sameKind && i < ops.Count do + match ops[i] with + | WatchFiles(p, t, _) -> + paths.AddRange p + tokens.AddRange t + i <- i + 1 + | _ -> sameKind <- false + + let! cookies = service.AdviseFileChangesAsync(paths.ToArray(), watchFlags, sink, CancellationToken.None) + + for j in 0 .. tokens.Count - 1 do + tokens[j].Cookie <- ValueSome cookies[j] + + | UnwatchFiles _ -> + let cookies = ResizeArray() + let mutable sameKind = true + + while sameKind && i < ops.Count do + match ops[i] with + | UnwatchFiles tokens -> + for token in tokens do + match token.Cookie with + | ValueSome cookie -> cookies.Add cookie + | ValueNone -> () + + i <- i + 1 + | _ -> sameKind <- false + + if cookies.Count > 0 then + let! _ = service.UnadviseFileChangesAsync(cookies.ToArray(), CancellationToken.None) () - | UnwatchDirs cookies :: rest -> - pending <- rest + | UnwatchDirs cookies -> + i <- i + 1 if cookies.Count > 0 then let! _ = service.UnadviseDirChangesAsync(cookies.ToArray(), CancellationToken.None) @@ -188,7 +173,7 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task draining <- false let! service = fileChangeService |> Async.AwaitTask - do! applyBatch service (List.ofSeq ops) |> Async.AwaitTask + do! applyBatch service ops |> Async.AwaitTask with _ -> // Never let a failed advise/unadvise (e.g. non-existent path) kill the // subscription loop; we simply won't get events for that path. @@ -321,7 +306,7 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on static let defaultWatchedDirectories () = let dotnetRoot = Environment.GetEnvironmentVariable "DOTNET_ROOT" - [ + seq { if not (String.IsNullOrEmpty dotnetRoot) then IO.Path.Combine(dotnetRoot, "packs") @@ -335,9 +320,10 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on ) IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.UserProfile, ".nuget", "packages") - ] - |> List.distinct - |> List.map (fun d -> WatchedDirectory(d, [ ".dll" ])) + } + |> Seq.distinct + |> Seq.map (fun d -> WatchedDirectory(d, [ ".dll" ])) + |> List.ofSeq let context = lazy diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs index 829cfd68226..2baaea32d0c 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -28,14 +28,14 @@ type private MockFileChangeContext() = member _.Dispose() = watched.Clear() type private MockFileChangeWatcher() = - let mutable context: MockFileChangeContext option = None + let mutable context: MockFileChangeContext voption = ValueNone member _.Context = context interface IFSharpFileChangeWatcher with member _.CreateContext _ = let ctx = new MockFileChangeContext() - context <- Some ctx + context <- ValueSome ctx ctx :> IFSharpFileChangeContext module FileChangeWatcherTests = From ed72a8b026b0f39552dfdf867f1bbfa5f80d7a97 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 22:56:13 +0200 Subject: [PATCH 07/25] Address review: keep applyBatch on a list, Seq.toList The list-typed applyBatch reads better than the index walk; only the voption use sites differ from the original body. --- .../LanguageService/FileChangeWatcher.fs | 111 +++++++++++------- 1 file changed, 66 insertions(+), 45 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index ede8eddb4d1..1823815cda8 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -92,61 +92,82 @@ type private WatcherOperation = [] type internal FSharpFileChangeWatcher(fileChangeService: Task) = - let applyBatch (service: IVsAsyncFileChangeEx2) (ops: ResizeArray) = + let applyBatch (service: IVsAsyncFileChangeEx2) (ops: WatcherOperation list) = task { // Coalesce adjacent same-kind operations into single service calls, preserving order // between kinds (a watch enqueued before an unwatch must be applied first). - let mutable i = 0 + let mutable pending = ops - while i < ops.Count do - match ops[i] with - | WatchDir(path, filters, sink, cookies) -> - i <- i + 1 + while not pending.IsEmpty do + match pending with + | [] -> () + | WatchDir(path, filters, sink, cookies) :: rest -> + pending <- rest let! cookie = service.AdviseDirChangeAsync(path, true, sink, CancellationToken.None) cookies.Add cookie if not filters.IsEmpty then do! service.FilterDirectoryChangesAsync(cookie, List.toArray filters, CancellationToken.None) - | WatchFiles(_, _, sink) -> - let paths = ResizeArray() - let tokens = ResizeArray() - let mutable sameKind = true - - while sameKind && i < ops.Count do - match ops[i] with - | WatchFiles(p, t, _) -> - paths.AddRange p - tokens.AddRange t - i <- i + 1 - | _ -> sameKind <- false - - let! cookies = service.AdviseFileChangesAsync(paths.ToArray(), watchFlags, sink, CancellationToken.None) - - for j in 0 .. tokens.Count - 1 do - tokens[j].Cookie <- ValueSome cookies[j] - - | UnwatchFiles _ -> - let cookies = ResizeArray() - let mutable sameKind = true - - while sameKind && i < ops.Count do - match ops[i] with - | UnwatchFiles tokens -> - for token in tokens do - match token.Cookie with - | ValueSome cookie -> cookies.Add cookie - | ValueNone -> () - - i <- i + 1 - | _ -> sameKind <- false - - if cookies.Count > 0 then - let! _ = service.UnadviseFileChangesAsync(cookies.ToArray(), CancellationToken.None) + | WatchFiles _ :: _ -> + let batch = + pending + |> List.takeWhile (function + | WatchFiles _ -> true + | _ -> false) + + pending <- pending |> List.skip batch.Length + + let paths = + batch + |> List.collect (function + | WatchFiles(p, _, _) -> p + | _ -> []) + + let tokens = + batch + |> List.collect (function + | WatchFiles(_, t, _) -> t + | _ -> []) + + let sink = + batch + |> List.pick (function + | WatchFiles(_, _, s) -> Some s + | _ -> None) + + let! cookies = service.AdviseFileChangesAsync(List.toArray paths, watchFlags, sink, CancellationToken.None) + + (tokens, List.ofArray cookies) + ||> List.iter2 (fun token cookie -> token.Cookie <- ValueSome cookie) + + | UnwatchFiles _ :: _ -> + let batch = + pending + |> List.takeWhile (function + | UnwatchFiles _ -> true + | _ -> false) + + pending <- pending |> List.skip batch.Length + + let cookies = + [ + for op in batch do + match op with + | UnwatchFiles tokens -> + for token in tokens do + match token.Cookie with + | ValueSome cookie -> cookie + | ValueNone -> () + | _ -> () + ] + + if not cookies.IsEmpty then + let! _ = service.UnadviseFileChangesAsync(List.toArray cookies, CancellationToken.None) () - | UnwatchDirs cookies -> - i <- i + 1 + | UnwatchDirs cookies :: rest -> + pending <- rest if cookies.Count > 0 then let! _ = service.UnadviseDirChangesAsync(cookies.ToArray(), CancellationToken.None) @@ -173,7 +194,7 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task draining <- false let! service = fileChangeService |> Async.AwaitTask - do! applyBatch service ops |> Async.AwaitTask + do! applyBatch service (List.ofSeq ops) |> Async.AwaitTask with _ -> // Never let a failed advise/unadvise (e.g. non-existent path) kill the // subscription loop; we simply won't get events for that path. @@ -323,7 +344,7 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on } |> Seq.distinct |> Seq.map (fun d -> WatchedDirectory(d, [ ".dll" ])) - |> List.ofSeq + |> Seq.toList let context = lazy From 38d0eb584a1d6a980985a8df2aba79d947443b1e Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 23:25:30 +0200 Subject: [PATCH 08/25] Address review: ImmutableArray contract, cancellable batch application * 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. --- .../LanguageService/FileChangeWatcher.fs | 169 ++++++++++-------- .../LanguageService/LanguageService.fs | 2 +- .../FileChangeWatcherTests.fs | 7 +- 3 files changed, 103 insertions(+), 75 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 1823815cda8..2ef805b896a 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -5,6 +5,7 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System open System.Collections.Concurrent open System.Collections.Generic +open System.Collections.Immutable open System.Threading open System.Threading.Tasks open Microsoft.VisualStudio @@ -13,6 +14,8 @@ open Microsoft.VisualStudio.Shell.Interop open Internal.Utilities.Library +open CancellableTasks + // Push-based file watching for FSharp.Editor, modelled on Roslyn's // Microsoft.VisualStudio.LanguageServices FileChangeWatcher (which is internal and not // exposed through ExternalAccess.FSharp). Uses the free-threaded IVsAsyncFileChangeEx2 @@ -21,7 +24,7 @@ open Internal.Utilities.Library /// A directory to watch recursively (with optional extension filters) so that individual /// files under it don't each need their own advise cookie. [] -type internal WatchedDirectory(path: string, extensionFilters: string list) = +type internal WatchedDirectory(path: string, extensionFilters: ImmutableArray) = let path = if path.EndsWithOrdinal(string IO.Path.DirectorySeparatorChar) then path @@ -36,12 +39,12 @@ type internal WatchedDirectory(path: string, extensionFilters: string list) = member _.Path = path member _.ExtensionFilters = extensionFilters - static member FilePathCoveredByWatchedDirectories(watchedDirectories: WatchedDirectory list, filePath: string) = + static member FilePathCoveredByWatchedDirectories(watchedDirectories: ImmutableArray, filePath: string) = watchedDirectories - |> List.exists (fun w -> + |> Seq.exists (fun w -> filePath.StartsWith(w.Path, StringComparison.OrdinalIgnoreCase) && (w.ExtensionFilters.IsEmpty - || w.ExtensionFilters |> List.exists filePath.EndsWithOrdinalIgnoreCase)) + || w.ExtensionFilters |> Seq.exists filePath.EndsWithOrdinalIgnoreCase)) /// A single watched file; disposing stops watching. type internal IFSharpWatchedFile = @@ -59,7 +62,7 @@ type internal IFSharpFileChangeContext = abstract EnqueueWatchingFile: filePath: string -> IFSharpWatchedFile type internal IFSharpFileChangeWatcher = - abstract CreateContext: watchedDirectories: WatchedDirectory list -> IFSharpFileChangeContext + abstract CreateContext: watchedDirectories: ImmutableArray -> IFSharpFileChangeContext [] module private FileChangeWatcherImpl = @@ -84,7 +87,7 @@ type internal FSharpWatchedFileToken() = /// Subscription operations queued for batched application against the file change service. type private WatcherOperation = - | WatchDir of path: string * filters: string list * sink: IVsFreeThreadedFileChangeEvents2 * cookies: List + | WatchDir of path: string * filters: ImmutableArray * sink: IVsFreeThreadedFileChangeEvents2 * cookies: List | WatchFiles of paths: string list * tokens: FSharpWatchedFileToken list * sink: IVsFreeThreadedFileChangeEvents2 | UnwatchFiles of tokens: FSharpWatchedFileToken list | UnwatchDirs of cookies: List @@ -93,7 +96,9 @@ type private WatcherOperation = type internal FSharpFileChangeWatcher(fileChangeService: Task) = let applyBatch (service: IVsAsyncFileChangeEx2) (ops: WatcherOperation list) = - task { + cancellableTask { + let! ct = CancellableTask.getCancellationToken () + // Coalesce adjacent same-kind operations into single service calls, preserving order // between kinds (a watch enqueued before an unwatch must be applied first). let mutable pending = ops @@ -103,55 +108,55 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task () | WatchDir(path, filters, sink, cookies) :: rest -> pending <- rest - let! cookie = service.AdviseDirChangeAsync(path, true, sink, CancellationToken.None) + let! cookie = service.AdviseDirChangeAsync(path, true, sink, ct) cookies.Add cookie if not filters.IsEmpty then - do! service.FilterDirectoryChangesAsync(cookie, List.toArray filters, CancellationToken.None) + do! service.FilterDirectoryChangesAsync(cookie, Seq.toArray filters, ct) - | WatchFiles _ :: _ -> + | WatchFiles(_, _, sink) :: _ -> let batch = pending - |> List.takeWhile (function + |> Seq.takeWhile (function | WatchFiles _ -> true | _ -> false) + |> Seq.toArray pending <- pending |> List.skip batch.Length let paths = - batch - |> List.collect (function - | WatchFiles(p, _, _) -> p - | _ -> []) + [| + for op in batch do + match op with + | WatchFiles(p, _, _) -> yield! p + | _ -> () + |] let tokens = - batch - |> List.collect (function - | WatchFiles(_, t, _) -> t - | _ -> []) - - let sink = - batch - |> List.pick (function - | WatchFiles(_, _, s) -> Some s - | _ -> None) + [| + for op in batch do + match op with + | WatchFiles(_, t, _) -> yield! t + | _ -> () + |] - let! cookies = service.AdviseFileChangesAsync(List.toArray paths, watchFlags, sink, CancellationToken.None) + let! cookies = service.AdviseFileChangesAsync(paths, watchFlags, sink, ct) - (tokens, List.ofArray cookies) - ||> List.iter2 (fun token cookie -> token.Cookie <- ValueSome cookie) + (tokens, cookies) + ||> Array.iter2 (fun token cookie -> token.Cookie <- ValueSome cookie) | UnwatchFiles _ :: _ -> let batch = pending - |> List.takeWhile (function + |> Seq.takeWhile (function | UnwatchFiles _ -> true | _ -> false) + |> Seq.toArray pending <- pending |> List.skip batch.Length let cookies = - [ + [| for op in batch do match op with | UnwatchFiles tokens -> @@ -160,46 +165,58 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task cookie | ValueNone -> () | _ -> () - ] + |] - if not cookies.IsEmpty then - let! _ = service.UnadviseFileChangesAsync(List.toArray cookies, CancellationToken.None) + if cookies.Length > 0 then + let! _ = service.UnadviseFileChangesAsync(cookies, ct) () | UnwatchDirs cookies :: rest -> pending <- rest if cookies.Count > 0 then - let! _ = service.UnadviseDirChangesAsync(cookies.ToArray(), CancellationToken.None) + let! _ = service.UnadviseDirChangesAsync(cookies.ToArray(), ct) () } + let cancellationTokenSource = new CancellationTokenSource() + // Single consumer loop: waits for the first queued operation, sleeps out the batching // window, drains the queue and applies everything in one pass. Nothing ever blocks on the // service being available. let agent = - MailboxProcessor.Start(fun inbox -> - async { - while true do - try - let! first = inbox.Receive() - do! Async.Sleep(int batchingDelay.TotalMilliseconds) - - let ops = ResizeArray [ first ] - let mutable draining = true - - while draining do - match! inbox.TryReceive 0 with - | Some op -> ops.Add op - | None -> draining <- false - - let! service = fileChangeService |> Async.AwaitTask - do! applyBatch service (List.ofSeq ops) |> Async.AwaitTask - with _ -> - // Never let a failed advise/unadvise (e.g. non-existent path) kill the - // subscription loop; we simply won't get events for that path. - () - }) + MailboxProcessor + .Start( + (fun inbox -> + async { + let! ct = Async.CancellationToken + + while true do + try + let! first = inbox.Receive() + do! Async.Sleep(int batchingDelay.TotalMilliseconds) + + let ops = ResizeArray [ first ] + let mutable draining = true + + while draining do + match! inbox.TryReceive 0 with + | Some op -> ops.Add op + | None -> draining <- false + + let! service = fileChangeService |> Async.AwaitTask + + do! + applyBatch service (List.ofSeq ops) + |> CancellableTask.startAsTask ct + |> Async.AwaitTask + with ex when not (ex :? OperationCanceledException) -> + // Never let a failed advise/unadvise (e.g. non-existent path) kill the + // subscription loop; we simply won't get events for that path. + () + }), + cancellationTokenSource.Token + ) member private _.Enqueue(op: WatcherOperation) = agent.Post op @@ -215,7 +232,13 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task IFSharpFileChangeContext -and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watchedDirectories: WatchedDirectory list) as this = + interface IDisposable with + member _.Dispose() = + cancellationTokenSource.Cancel() + cancellationTokenSource.Dispose() + (agent :> IDisposable).Dispose() + +and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watchedDirectories: ImmutableArray) as this = let gate = obj () let mutable disposed = false @@ -327,24 +350,26 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on static let defaultWatchedDirectories () = let dotnetRoot = Environment.GetEnvironmentVariable "DOTNET_ROOT" - seq { - if not (String.IsNullOrEmpty dotnetRoot) then - IO.Path.Combine(dotnetRoot, "packs") + let directories = + seq { + if not (String.IsNullOrEmpty dotnetRoot) then + IO.Path.Combine(dotnetRoot, "packs") - IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.ProgramFiles, "dotnet", "packs") + IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.ProgramFiles, "dotnet", "packs") - IO.Path.Combine( - Environment.GetFolderPath Environment.SpecialFolder.ProgramFilesX86, - "Reference Assemblies", - "Microsoft", - "Framework" - ) + IO.Path.Combine( + Environment.GetFolderPath Environment.SpecialFolder.ProgramFilesX86, + "Reference Assemblies", + "Microsoft", + "Framework" + ) - IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.UserProfile, ".nuget", "packages") - } - |> Seq.distinct - |> Seq.map (fun d -> WatchedDirectory(d, [ ".dll" ])) - |> Seq.toList + IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.UserProfile, ".nuget", "packages") + } + |> Seq.distinct + |> Seq.map (fun d -> WatchedDirectory(d, ImmutableArray.Create ".dll")) + + directories.ToImmutableArray() let context = lazy diff --git a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs index 3946e523630..810861270d8 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs @@ -206,7 +206,7 @@ type internal FSharpWorkspaceServiceFactory |> ignore) let fileChangeWatcher = - FSharpFileChangeWatcher(FSharpFileChangeWatcher.CreateDefaultServiceTask()) + new FSharpFileChangeWatcher(FSharpFileChangeWatcher.CreateDefaultServiceTask()) let optionsManager = FSharpProjectOptionsManager(checker, workspace, fileChangeWatcher) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs index 2baaea32d0c..579997ddae9 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -3,6 +3,7 @@ namespace FSharp.Editor.Tests open System +open System.Collections.Immutable open System.Threading open Xunit open Microsoft.VisualStudio.FSharp.Editor @@ -44,7 +45,8 @@ module FileChangeWatcherTests = [] let ``WatchedDirectory covers files under it matching the extension filter`` () = - let dirs = [ WatchedDirectory(@"C:\refs", [ ".dll" ]) ] + let dirs = + ImmutableArray.Create(WatchedDirectory(@"C:\refs", ImmutableArray.Create ".dll")) Assert.True(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\refs\sub\a.dll")) Assert.True(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\REFS\A.DLL")) @@ -53,7 +55,8 @@ module FileChangeWatcherTests = [] let ``WatchedDirectory without filters covers any file under it`` () = - let dirs = [ WatchedDirectory(@"C:\refs", []) ] + let dirs = + ImmutableArray.Create(WatchedDirectory(@"C:\refs", ImmutableArray.Empty)) Assert.True(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\refs\a.xml")) Assert.False(WatchedDirectory.FilePathCoveredByWatchedDirectories(dirs, @"C:\refsx\a.xml")) From e90d29b705980021437807e667b4b51ac1391ea0 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 23:42:15 +0200 Subject: [PATCH 09/25] Add String.StartsWithOrdinalIgnoreCase next to EndsWithOrdinalIgnoreCase 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. --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 1 + src/Compiler/Utilities/illib.fs | 3 +++ src/Compiler/Utilities/illib.fsi | 2 ++ .../src/FSharp.Editor/LanguageService/FileChangeWatcher.fs | 2 +- 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index f383908e195..337ee638886 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -169,6 +169,7 @@ * Fix signature generation (`fsc --sig`, `GenerateSignature`, `GetValSignatureText`) dropping the parentheses around a destructured or pattern-annotated tuple parameter when a sibling argument in the same curried group is named, so `(int * float) * z: string` no longer prints as the flat 3-tuple `int * float * z: string`. ([Issue #20397](https://github.com/dotnet/fsharp/issues/20397), [PR #20589](https://github.com/dotnet/fsharp/pull/20589)) ### Added +* `Internal.Utilities.Library`: `String.StartsWithOrdinalIgnoreCase` extension, the `StartsWith` sibling of `EndsWithOrdinalIgnoreCase`. ([PR #20457](https://github.com/dotnet/fsharp/pull/20457)) * FCS: add FSharpCheckFileResults.FileSignature ([PR #20478](https://github.com/dotnet/fsharp/pull/20478)) * Added the `ReraiseInComputationExpressions` language feature (`--langversion:preview`): `reraise ()` in the `with` handler of a computation expression is compiled to a rethrow through `ExceptionDispatchInfo` instead of being rejected with FS0413. ([Suggestion #660](https://github.com/fsharp/fslang-suggestions/issues/660), [RFC FS-1347](https://github.com/fsharp/fslang-design/pull/843), [PR #20405](https://github.com/dotnet/fsharp/pull/20405)) diff --git a/src/Compiler/Utilities/illib.fs b/src/Compiler/Utilities/illib.fs index 7a4f8a76927..a0174da1ab3 100644 --- a/src/Compiler/Utilities/illib.fs +++ b/src/Compiler/Utilities/illib.fs @@ -97,6 +97,9 @@ module internal PervasiveAutoOpens = member inline x.StartsWithOrdinal(value: string) = x.StartsWith(value, StringComparison.Ordinal) + member inline x.StartsWithOrdinalIgnoreCase(value: string) = + x.StartsWith(value, StringComparison.OrdinalIgnoreCase) + member inline x.EndsWithOrdinal(value: string) = x.EndsWith(value, StringComparison.Ordinal) diff --git a/src/Compiler/Utilities/illib.fsi b/src/Compiler/Utilities/illib.fsi index 53dda1ff8a3..450e0968550 100644 --- a/src/Compiler/Utilities/illib.fsi +++ b/src/Compiler/Utilities/illib.fsi @@ -58,6 +58,8 @@ module internal PervasiveAutoOpens = member inline StartsWithOrdinal: value: string -> bool + member inline StartsWithOrdinalIgnoreCase: value: string -> bool + member inline EndsWithOrdinal: value: string -> bool member inline EndsWithOrdinalIgnoreCase: value: string -> bool diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 2ef805b896a..eb1d3e621e1 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -42,7 +42,7 @@ type internal WatchedDirectory(path: string, extensionFilters: ImmutableArray, filePath: string) = watchedDirectories |> Seq.exists (fun w -> - filePath.StartsWith(w.Path, StringComparison.OrdinalIgnoreCase) + filePath.StartsWithOrdinalIgnoreCase w.Path && (w.ExtensionFilters.IsEmpty || w.ExtensionFilters |> Seq.exists filePath.EndsWithOrdinalIgnoreCase)) From 2588bff722d8415b9f0c0c728d09189435ca5d69 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 00:47:47 +0200 Subject: [PATCH 10/25] Address review: per-sink batching, timer race, diffed watches, tests * 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. --- docs/ide/file-watching-design-review.md | 71 -------- docs/ide/file-watching.md | 47 +++++ .../FSharpProjectOptionsManager.fs | 23 ++- .../LanguageService/FileChangeWatcher.fs | 75 +++++--- .../FileChangeWatcherTests.fs | 160 ++++++++++++++++++ 5 files changed, 270 insertions(+), 106 deletions(-) delete mode 100644 docs/ide/file-watching-design-review.md create mode 100644 docs/ide/file-watching.md diff --git a/docs/ide/file-watching-design-review.md b/docs/ide/file-watching-design-review.md deleted file mode 100644 index 9d5e52a32f7..00000000000 --- a/docs/ide/file-watching-design-review.md +++ /dev/null @@ -1,71 +0,0 @@ -# F# file watching: design review vs Roslyn - -## Verdict - -The `FileChangeWatcher` working tree is **not** a file-change watcher. It is a pull-model I/O change: - -- `IFileSystem.OpenFileForReadShimAsync` + `Stream.ReadAllTextAsync` -- `FSharpFileSnapshot.CreateFromFileSystem` still versions the file as `GetLastWriteTimeShim(fileName).Ticks` at construction time - -That is useful (it stops blocking the ThreadPool on disk reads) and orthogonal to watching. It is not comparable to Roslyn's `FileChangeWatcher`. - -Roslyn's implementation is a **push** service over `IVsAsyncFileChangeEx2`: - -- directory subscriptions (`WatchedDirectory`) instead of one cookie per file -- `AsyncBatchingWorkQueue` (500 ms) so advise/unadvise is batched and never blocks the UI / thread pool -- free-threaded sinks (`IVsFreeThreadedFileChangeEvents2`) -- coalesced invalidation of metadata references (`FileWatchedReferenceFactory`) - -## What already exists in this repo - -Three layers, none of them `IVsAsyncFileChangeEx2`: - -| Layer | API | Role | -|---|---|---| -| `vsintegration/src/FSharp.ProjectSystem.Base/FileChangeManager.cs` | `IVsFileChangeEx` | Legacy project-system reload of nested items | -| `vsintegration/src/FSharp.LanguageService/FSharpSource.fs` (`SetDependencyFiles`) | `IVsFileChangeEx` | Deprecated unroslynized LS: watch `#r` / dependency files | -| **uncommitted** `stash@{7}` → `vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs` | `IVsFileChangeEx` | Intended modern replacement in `FSharp.Editor` | - -There is **no commit** (on any branch, stash patch, or GitHub search) that implements `IVsAsyncFileChangeEx2`. The work that was remembered as "the async watcher" is the stash below; it uses the older sync advise API, marshalled onto the UI thread. - -## Recovered implementation (`stash@{7}`) - -Stash: `WIP on revert-20080-t-gro-net11-upgrade: 8bf5dca37` - -Index commit that added the file: - -`54465595717b8bb746cb2633d5a4aa834888a481` - -Shape: - -- `IFileChangeWatcher.WatchFile` → `IVsFileChangeEx.AdviseFileChange` / `UnadviseFileChange` -- `FileChangeWatcherHub`: one cookie per path, ref-counted, 500 ms debounce -- Wired into `FSharpProjectOptionsReactor` for `-r:` reference assemblies -- Exposed as `FSharpProjectOptionsManager.WatchFile` so `WorkspaceExtensions` snapshot cache can share the same subscriptions - -This is the right *place* (FSharp.Editor, reference assemblies, debounce, share across projects). It is the wrong *shell API*: - -- `JoinableTaskFactory.Run` + `SwitchToMainThreadAsync` on every advise/unadvise -- no directory watches → N cookies for a NuGet cache -- no batching of subscribe/unsubscribe -- not free-threaded - -## Target design (Roslyn-shaped) - -1. Keep the async read shim. It is independent and should ship on its own. -2. Restore `FileChangeWatcher.fs` from `stash@{7}`, then replace `IVsFileChangeEx` with `IVsAsyncFileChangeEx2`: - - obtain the service asynchronously (same as Roslyn's `Task`) - - queue advise/unadvise on a 500 ms batching work queue; never `JTF.Run` - - subscribe to directories (NuGet cache, output folders) with extension filters; fall back to per-file only for stray paths - - implement `IVsFreeThreadedFileChangeEvents2` so callbacks do not hop to the UI thread -3. On a coalesced change: `checker.NotifyFileChanged` / `InvalidateConfiguration` for the owning project only. Stop O(N) `stat` of reference timestamps on every incremental check. -4. Scripts: watch `#r` / `#load` paths the same way; drop caret-move `NotifyFileChanged`. -5. Do not invent a second watcher. Project system (`FileChangeManager`) and FCS (`TimeStampCache`) should consume this service or stay on their existing contracts. - -## Suggested split - -- PR 1: async `OpenFileForReadShim` (already in the `FileChangeWatcher` worktree). -- PR 2: restore stash watcher as-is (`IVsFileChangeEx`) behind the existing reactor hook — functional, limited. -- PR 3: swap the shell API to `IVsAsyncFileChangeEx2` + directory batching. - -PR 2 is optional if PR 3 is done immediately. diff --git a/docs/ide/file-watching.md b/docs/ide/file-watching.md new file mode 100644 index 00000000000..2e6073f3113 --- /dev/null +++ b/docs/ide/file-watching.md @@ -0,0 +1,47 @@ +# File watching in FSharp.Editor + +`vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs` is the F# counterpart of +Roslyn's `FileChangeWatcher` and `ReferenceFileChangeTracker` (both internal to +`Microsoft.VisualStudio.LanguageServices` and not exposed through `ExternalAccess.FSharp`). + +## Why a watcher + +The Roslyn workspace tracks documents, not the `-r:` references an F# project compiles against. +When a referenced assembly is rebuilt outside VS nothing tells the F# language service; FCS only +notices because it stats every reference again on the next request (`IsReferencesInvalidated` on +the incremental builder, `ReferencesOnDisk` when a snapshot is reused). The watcher turns that into +a push: one notification per changed path, delivered to the projects that reference it. + +## Shape + +- **Service.** `IVsAsyncFileChangeEx2`, obtained asynchronously; nothing ever blocks on the UI + thread. Callbacks arrive through `IVsFreeThreadedFileChangeEvents2` and stay on background + threads. +- **Batching.** Subscribe/unsubscribe operations go through a single-consumer queue with a 500 ms + window (Roslyn's empirical value for solution open/close). Consecutive operations of the same + kind, and for file watches the same sink, are coalesced into one service call. +- **Directory watches.** Each context starts with recursive `.dll` watches on the places + reference assemblies live: `DOTNET_ROOT/packs` and the machine-wide `dotnet/packs`, the .NET + Framework reference assemblies, and the NuGet cache (`NUGET_PACKAGES` or `~/.nuget/packages`). + A file under one of them costs no cookie of its own. Roslyn does not watch the NuGet cache; we + do because every `-r:` is watched uniformly and package assemblies are the bulk of them, so the + alternative is a per-file advise for each. +- **Per-file watches.** Paths outside those directories (project outputs, loose assemblies) get + an individual advise, ref-counted across projects by `FSharpReferenceChangeTracker`. +- **Debounce.** A rebuild writes a temp file and renames it, producing several notifications; the + tracker fires one callback per path after 2 s of quiet. + +## Consumer + +`FSharpProjectOptionsReactor` watches the `-r:` set of every project it computes options for and +calls `FSharpChecker.InvalidateConfiguration` for each project that references a changed path. +The cached options stay valid (same paths); only the FCS build behind them is stale. Watch sets +are diffed on recompute, so an unchanged reference list touches nothing. + +## Follow-ups + +1. A reference-change notification for the incremental builder on the FCS side, the analogue of + `useChangeNotifications` for sources, so `IsReferencesInvalidated` stops stat'ing every + reference on every request. +2. A watcher-invalidated timestamp cache for snapshot reuse (`ReferencesOnDisk`). +3. Scripts: watch `#r` references and `#load` sources the same way. diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 2af5298b7c7..094efb986e9 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -129,15 +129,14 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch let cache = ConcurrentDictionary() - // Push invalidation for on-disk '-r:' reference assemblies (not tracked by the Roslyn - // workspace): when one changes after an external rebuild, drop the cached options of every - // project referencing it instead of waiting for a timestamp poll to notice. + // Push invalidation for on-disk '-r:' reference assemblies, which the Roslyn workspace does not + // track. The cached options stay valid (same paths); only the FCS build behind them goes stale. let referenceWatches = ConcurrentDictionary>() let onWatchedReferenceChanged (path: string) = for KeyValue(projectId, paths) in referenceWatches do if paths.Contains path then - match cache.TryRemove projectId with + match cache.TryGetValue projectId with | true, (_, _, projectOptions) -> checker.InvalidateConfiguration(projectOptions, userOpName = "onWatchedReferenceChanged") | _ -> () @@ -152,19 +151,29 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch | _ -> () let watchReferenceFiles (projectId: ProjectId) (projectOptions: FSharpProjectOptions) = - clearReferenceWatches projectId - let paths = HashSet(StringComparer.OrdinalIgnoreCase) for option in projectOptions.OtherOptions do if option.StartsWithOrdinal "-r:" then paths.Add(option.Substring "-r:".Length) |> ignore - if paths.Count > 0 then + match referenceWatches.TryGetValue projectId with + | true, previous -> + for path in previous do + if not (paths.Contains path) then + referenceChangeTracker.StopWatchingReference path + + for path in paths do + if not (previous.Contains path) then + referenceChangeTracker.StartWatchingReference path + | _ -> for path in paths do referenceChangeTracker.StartWatchingReference path + if paths.Count > 0 then referenceWatches[projectId] <- paths + else + referenceWatches.TryRemove projectId |> ignore let singleFileCache = ConcurrentDictionary() diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index eb1d3e621e1..5512c748741 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -14,6 +14,8 @@ open Microsoft.VisualStudio.Shell.Interop open Internal.Utilities.Library +open Microsoft.VisualStudio.FSharp.Editor.DebugHelpers + open CancellableTasks // Push-based file watching for FSharp.Editor, modelled on Roslyn's @@ -79,7 +81,12 @@ module private FileChangeWatcherImpl = /// Empirically strong batching window during high activity (solution open/close); see /// Roslyn's FileChangeWatcher. - let batchingDelay = TimeSpan.FromMilliseconds 500. + let defaultBatchingDelay = TimeSpan.FromMilliseconds 500. + + let noOpWatchedFile = + { new IFSharpWatchedFile with + member _.Dispose() = () + } [] type internal FSharpWatchedFileToken() = @@ -93,7 +100,9 @@ type private WatcherOperation = | UnwatchDirs of cookies: List [] -type internal FSharpFileChangeWatcher(fileChangeService: Task) = +type internal FSharpFileChangeWatcher(fileChangeService: Task, ?batchingDelay: TimeSpan) = + + let batchingDelay = defaultArg batchingDelay defaultBatchingDelay let applyBatch (service: IVsAsyncFileChangeEx2) (ops: WatcherOperation list) = cancellableTask { @@ -118,7 +127,7 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task Seq.takeWhile (function - | WatchFiles _ -> true + | WatchFiles(_, _, s) -> obj.ReferenceEquals(s, sink) | _ -> false) |> Seq.toArray @@ -155,6 +164,7 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task List.skip batch.Length + // A token whose watch never got a cookie (or was already unadvised) is a no-op. let cookies = [| for op in batch do @@ -162,7 +172,9 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task for token in tokens do match token.Cookie with - | ValueSome cookie -> cookie + | ValueSome cookie -> + token.Cookie <- ValueNone + cookie | ValueNone -> () | _ -> () |] @@ -197,12 +209,10 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task ops.Add op - | None -> draining <- false + while inbox.CurrentQueueLength > 0 do + let! op = inbox.Receive() + ops.Add op let! service = fileChangeService |> Async.AwaitTask @@ -213,7 +223,7 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task // Never let a failed advise/unadvise (e.g. non-existent path) kill the // subscription loop; we simply won't get events for that path. - () + FSharpOutputPane.logExceptionWithContext (ex, nameof FSharpFileChangeWatcher) }), cancellationTokenSource.Token ) @@ -277,10 +287,7 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc member _.EnqueueWatchingFile filePath = if WatchedDirectory.FilePathCoveredByWatchedDirectories(watchedDirectories, filePath) then - // Covered by a directory watch; nothing extra to subscribe. - { new IFSharpWatchedFile with - member _.Dispose() = () - } + noOpWatchedFile else let token = FSharpWatchedFileToken() lock gate (fun () -> activeFileTokens.Add token |> ignore) @@ -342,13 +349,13 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on let watchedFiles = Dictionary(StringComparer.OrdinalIgnoreCase) - let pendingTimers = - ConcurrentDictionary(StringComparer.OrdinalIgnoreCase) + let pendingTimers = Dictionary(StringComparer.OrdinalIgnoreCase) // On each platform there is a place framework reference assemblies live; these rarely change // but account for most watched paths, so cover them with directory watches up front. static let defaultWatchedDirectories () = let dotnetRoot = Environment.GetEnvironmentVariable "DOTNET_ROOT" + let nugetPackages = Environment.GetEnvironmentVariable "NUGET_PACKAGES" let directories = seq { @@ -364,7 +371,10 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on "Framework" ) - IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.UserProfile, ".nuget", "packages") + if String.IsNullOrEmpty nugetPackages then + IO.Path.Combine(Environment.GetFolderPath Environment.SpecialFolder.UserProfile, ".nuget", "packages") + else + nugetPackages } |> Seq.distinct |> Seq.map (fun d -> WatchedDirectory(d, ImmutableArray.Create ".dll")) @@ -377,22 +387,31 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on ctx.FileChanged.Add(fun path -> let fire (_: obj) = - pendingTimers.TryRemove path - |> function - | true, timer -> timer.Dispose() - | _ -> () + let isWatched = + lock gate (fun () -> + match pendingTimers.TryGetValue path with + | true, timer -> + pendingTimers.Remove path |> ignore + timer.Dispose() + | _ -> () - // Only notify for paths someone is actually watching; directory watches - // cover whole trees. - let isWatched = lock gate (fun () -> watchedFiles.ContainsKey path) + watchedFiles.ContainsKey path) if isWatched then onChanged path - let timer = - pendingTimers.GetOrAdd(path, fun _ -> new Timer(fire, null, Timeout.Infinite, Timeout.Infinite)) - - timer.Change(notificationDelay, Timeout.InfiniteTimeSpan) |> ignore) + lock gate (fun () -> + // Directory watches cover whole trees; only debounce paths someone watches. + if not disposed && watchedFiles.ContainsKey path then + let timer = + match pendingTimers.TryGetValue path with + | true, timer -> timer + | _ -> + let timer = new Timer(fire, null, Timeout.Infinite, Timeout.Infinite) + pendingTimers[path] <- timer + timer + + timer.Change(notificationDelay, Timeout.InfiniteTimeSpan) |> ignore)) ctx) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs index 579997ddae9..ca6a4ac026b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -5,7 +5,9 @@ namespace FSharp.Editor.Tests open System open System.Collections.Immutable open System.Threading +open System.Threading.Tasks open Xunit +open Microsoft.VisualStudio.Shell open Microsoft.VisualStudio.FSharp.Editor type private MockFileChangeContext() = @@ -39,10 +41,79 @@ type private MockFileChangeWatcher() = context <- ValueSome ctx ctx :> IFSharpFileChangeContext +type private ServiceCall = + | AdvisedDir of path: string * cookie: uint32 + | FilteredDir of cookie: uint32 * extensions: string list + | AdvisedFiles of paths: string list * sink: obj * cookies: uint32 list + | UnadvisedFiles of cookies: uint32 list + | UnadvisedDirs of cookies: uint32 list + +/// Stands in for IVsAsyncFileChangeEx2: hands out sequential cookies and records every call, so a +/// test can see how the watcher turned its queue into service calls. +type private RecordingFileChangeService() = + let calls = ResizeArray() + let mutable nextCookie = 0u + + let record call = lock calls (fun () -> calls.Add call) + + let newCookie () = + nextCookie <- nextCookie + 1u + nextCookie + + member _.Calls = lock calls (fun () -> List.ofSeq calls) + + member this.WaitForCalls(count: int) = + let deadline = DateTime.UtcNow + TimeSpan.FromSeconds 10. + + while lock calls (fun () -> calls.Count) < count && DateTime.UtcNow < deadline do + Thread.Sleep 10 + + this.Calls + + interface IVsAsyncFileChangeEx2 with + member _.AdviseFileChangesAsync(filenames, _, sink, _) = + let cookies = [| for _ in filenames -> newCookie () |] + record (AdvisedFiles(List.ofSeq filenames, box sink, List.ofArray cookies)) + Task.FromResult cookies + + interface IVsAsyncFileChangeEx with + member _.AdviseFileChangeAsync(_, _, _, _) = Task.FromResult(newCookie ()) + member _.UnadviseFileChangeAsync(_, _) = Task.FromResult "" + + member _.UnadviseFileChangesAsync(cookies, _) = + record (UnadvisedFiles(List.ofSeq cookies)) + Task.FromResult Array.empty + + member _.AdviseDirChangeAsync(directory, _, _, _) = + let cookie = newCookie () + record (AdvisedDir(directory, cookie)) + Task.FromResult cookie + + member _.UnadviseDirChangeAsync(_, _) = Task.FromResult "" + + member _.UnadviseDirChangesAsync(cookies, _) = + record (UnadvisedDirs(List.ofSeq cookies)) + Task.FromResult Array.empty + + member _.SyncFileAsync(_, _) = Task.CompletedTask + member _.IgnoreFileAsync(_, _, _, _) = Task.CompletedTask + member _.IgnoreDirAsync(_, _, _) = Task.CompletedTask + + member _.FilterDirectoryChangesAsync(cookie, extensions, _) = + record (FilteredDir(cookie, List.ofArray extensions)) + Task.CompletedTask + module FileChangeWatcherTests = let private testDelay = TimeSpan.FromMilliseconds 50. + let private batchDelay = TimeSpan.FromMilliseconds 100. + + let private noDirectories = ImmutableArray.Empty + + let private createWatcher (service: RecordingFileChangeService) = + new FSharpFileChangeWatcher(Task.FromResult(service :> IVsAsyncFileChangeEx2), batchDelay) + [] let ``WatchedDirectory covers files under it matching the extension filter`` () = let dirs = @@ -122,3 +193,92 @@ module FileChangeWatcherTests = Thread.Sleep(testDelay + testDelay) Assert.False called + + [] + let ``Consecutive file watches are advised in one service call`` () = + let service = RecordingFileChangeService() + use watcher = createWatcher service + use context = (watcher :> IFSharpFileChangeWatcher).CreateContext noDirectories + + for path in [ @"C:\x\a.dll"; @"C:\x\b.dll"; @"C:\x\c.dll" ] do + context.EnqueueWatchingFile path |> ignore + + match service.WaitForCalls 1 with + | [ AdvisedFiles(paths, _, cookies) ] -> + Assert.Equal([ @"C:\x\a.dll"; @"C:\x\b.dll"; @"C:\x\c.dll" ], paths) + Assert.Equal([ 1u; 2u; 3u ], cookies) + | calls -> failwith $"Unexpected calls: %A{calls}" + + [] + let ``A run of file watches is split when the sink changes`` () = + let service = RecordingFileChangeService() + use watcher = createWatcher service + let factory = watcher :> IFSharpFileChangeWatcher + use first = factory.CreateContext noDirectories + use second = factory.CreateContext noDirectories + + first.EnqueueWatchingFile @"C:\x\a.dll" |> ignore + first.EnqueueWatchingFile @"C:\x\b.dll" |> ignore + second.EnqueueWatchingFile @"C:\x\c.dll" |> ignore + + match service.WaitForCalls 2 with + | [ AdvisedFiles(firstPaths, firstSink, [ 1u; 2u ]); AdvisedFiles(secondPaths, secondSink, [ 3u ]) ] -> + Assert.Equal([ @"C:\x\a.dll"; @"C:\x\b.dll" ], firstPaths) + Assert.Equal([ @"C:\x\c.dll" ], secondPaths) + Assert.False(obj.ReferenceEquals(firstSink, secondSink)) + | calls -> failwith $"Unexpected calls: %A{calls}" + + [] + let ``A watch followed by an unwatch in the same batch unadvises the cookie the watch received`` () = + let service = RecordingFileChangeService() + use watcher = createWatcher service + use context = (watcher :> IFSharpFileChangeWatcher).CreateContext noDirectories + + let watched = context.EnqueueWatchingFile @"C:\x\a.dll" + watched.Dispose() + + match service.WaitForCalls 2 with + | [ AdvisedFiles(_, _, [ advised ]); UnadvisedFiles [ unadvised ] ] -> Assert.Equal(advised, unadvised) + | calls -> failwith $"Unexpected calls: %A{calls}" + + [] + let ``Unwatching a token that holds no cookie is a no-op`` () = + let service = RecordingFileChangeService() + use watcher = createWatcher service + use context = (watcher :> IFSharpFileChangeWatcher).CreateContext noDirectories + + let watched = context.EnqueueWatchingFile @"C:\x\a.dll" + service.WaitForCalls 1 |> ignore + + watched.Dispose() + service.WaitForCalls 2 |> ignore + + watched.Dispose() + Thread.Sleep(batchDelay + batchDelay + batchDelay) + + match service.Calls with + | [ AdvisedFiles(_, _, [ advised ]); UnadvisedFiles [ unadvised ] ] -> Assert.Equal(advised, unadvised) + | calls -> failwith $"Unexpected calls: %A{calls}" + + [] + let ``Disposing a context unadvises its directory and remaining file cookies`` () = + let service = RecordingFileChangeService() + use watcher = createWatcher service + + let directories = + ImmutableArray.Create(WatchedDirectory(@"C:\refs", ImmutableArray.Create ".dll")) + + let context = (watcher :> IFSharpFileChangeWatcher).CreateContext directories + context.EnqueueWatchingFile @"C:\refs\covered.dll" |> ignore + context.EnqueueWatchingFile @"C:\other\a.dll" |> ignore + service.WaitForCalls 3 |> ignore + + context.Dispose() + + match service.WaitForCalls 5 with + | [ AdvisedDir(directory, 1u) + FilteredDir(1u, [ ".dll" ]) + AdvisedFiles([ @"C:\other\a.dll" ], _, [ 2u ]) + UnadvisedDirs [ 1u ] + UnadvisedFiles [ 2u ] ] -> Assert.Equal(@"C:\refs\", directory) + | calls -> failwith $"Unexpected calls: %A{calls}" From f699bac52b0af69801c57c6b8de3798f8635e9c7 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 00:59:54 +0200 Subject: [PATCH 11/25] Use a second constructor instead of an optional delay parameter 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. --- .../LanguageService/FileChangeWatcher.fs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 5512c748741..284cbad8bee 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -83,6 +83,10 @@ module private FileChangeWatcherImpl = /// Roslyn's FileChangeWatcher. let defaultBatchingDelay = TimeSpan.FromMilliseconds 500. + /// Delay between the last observed change to a path and the callback: a rebuild typically + /// writes a temp file then renames, producing several rapid notifications. + let defaultNotificationDelay = TimeSpan.FromSeconds 2. + let noOpWatchedFile = { new IFSharpWatchedFile with member _.Dispose() = () @@ -100,9 +104,7 @@ type private WatcherOperation = | UnwatchDirs of cookies: List [] -type internal FSharpFileChangeWatcher(fileChangeService: Task, ?batchingDelay: TimeSpan) = - - let batchingDelay = defaultArg batchingDelay defaultBatchingDelay +type internal FSharpFileChangeWatcher(fileChangeService: Task, batchingDelay: TimeSpan) = let applyBatch (service: IVsAsyncFileChangeEx2) (ops: WatcherOperation list) = cancellableTask { @@ -228,6 +230,8 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task] private FileChangeContext(enqueue: WatcherOperation -> unit, watc /// modelled on Roslyn's ReferenceFileChangeTracker. Multiple projects watching the same dll /// share one subscription; bursts of writes produce a single callback per path. [] -type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, onChanged: string -> unit, ?notificationDelay: TimeSpan) = - - /// Delay between the last observed change to a path and the callback: a rebuild typically - /// writes a temp file then renames, producing several rapid notifications. - let notificationDelay = defaultArg notificationDelay (TimeSpan.FromSeconds 2.) +type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, onChanged: string -> unit, notificationDelay: TimeSpan) = let gate = obj () let mutable disposed = false @@ -415,6 +415,8 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on ctx) + new(watcher, onChanged) = new FSharpReferenceChangeTracker(watcher, onChanged, defaultNotificationDelay) + /// Starts watching a path, ref-counted. Call StopWatchingReference exactly once per start. member _.StartWatchingReference(fullFilePath: string) = lock gate (fun () -> From 04fba82c8d8334d8912504ba7beeb3abd7a9ca48 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 01:19:32 +0200 Subject: [PATCH 12/25] Wrap the recording service's doc comment in summary for the cref --- .../tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs index ca6a4ac026b..d3df726b7b0 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -48,8 +48,10 @@ type private ServiceCall = | UnadvisedFiles of cookies: uint32 list | UnadvisedDirs of cookies: uint32 list -/// Stands in for IVsAsyncFileChangeEx2: hands out sequential cookies and records every call, so a +/// +/// Stands in for : hands out sequential cookies and records every call, so a /// test can see how the watcher turned its queue into service calls. +/// type private RecordingFileChangeService() = let calls = ResizeArray() let mutable nextCookie = 0u From 2b8b2e6e252b50e4d1256ddcaf95581ddb0d3137 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:02:38 +0200 Subject: [PATCH 13/25] Drop the reference subscription: Roslyn already covers it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/ide/file-watching.md | 57 ++++++++++-------- .../FSharpProjectOptionsManager.fs | 60 +------------------ .../LanguageService/LanguageService.fs | 6 +- 3 files changed, 36 insertions(+), 87 deletions(-) diff --git a/docs/ide/file-watching.md b/docs/ide/file-watching.md index 2e6073f3113..b9172c01165 100644 --- a/docs/ide/file-watching.md +++ b/docs/ide/file-watching.md @@ -4,13 +4,24 @@ Roslyn's `FileChangeWatcher` and `ReferenceFileChangeTracker` (both internal to `Microsoft.VisualStudio.LanguageServices` and not exposed through `ExternalAccess.FSharp`). -## Why a watcher +## What the workspace already gives us -The Roslyn workspace tracks documents, not the `-r:` references an F# project compiles against. -When a referenced assembly is rebuilt outside VS nothing tells the F# language service; FCS only -notices because it stats every reference again on the next request (`IsReferencesInvalidated` on -the incremental builder, `ReferencesOnDisk` when a snapshot is reused). The watcher turns that into -a push: one notification per changed path, delivered to the projects that reference it. +Not every on-disk change needs this watcher. A `-r:` that the Roslyn workspace holds as a +`MetadataReference` is already watched by Roslyn: `ProjectSystemProjectFactory` advises every +reference path, and when one changes it swaps the reference on the solution, which bumps +`Project.Version`. `FSharpProjectOptionsReactor` sees that version through `isProjectInvalidated`, +recomputes, and calls `InvalidateConfiguration` — measured in VS, that path wins the race against +a watcher subscribed to the same file, because Roslyn batches over 500 ms where this tracker +additionally debounces for 2 s. + +So a second subscription to the same reference set buys nothing. What the workspace does *not* +cover is everything it has no document or reference for, and every stat FCS still performs +internally: + +- `#load` sources of a script: not documents, not references, invisible to the workspace. +- `IsReferencesInvalidated` on the incremental builder, which stats every reference on every + request. +- `ReferencesOnDisk` on snapshot reuse, which does the same per comparison. ## Shape @@ -20,28 +31,24 @@ a push: one notification per changed path, delivered to the projects that refere - **Batching.** Subscribe/unsubscribe operations go through a single-consumer queue with a 500 ms window (Roslyn's empirical value for solution open/close). Consecutive operations of the same kind, and for file watches the same sink, are coalesced into one service call. -- **Directory watches.** Each context starts with recursive `.dll` watches on the places - reference assemblies live: `DOTNET_ROOT/packs` and the machine-wide `dotnet/packs`, the .NET - Framework reference assemblies, and the NuGet cache (`NUGET_PACKAGES` or `~/.nuget/packages`). - A file under one of them costs no cookie of its own. Roslyn does not watch the NuGet cache; we - do because every `-r:` is watched uniformly and package assemblies are the bulk of them, so the - alternative is a per-file advise for each. +- **Directory watches.** A context starts with recursive `.dll` watches on the places reference + assemblies live: `DOTNET_ROOT/packs` and the machine-wide `dotnet/packs`, the .NET Framework + reference assemblies, and the NuGet cache (`NUGET_PACKAGES` or `~/.nuget/packages`). A file + under one of them costs no cookie of its own. Roslyn does not watch the NuGet cache; a consumer + that watches every `-r:` uniformly wants it, since package assemblies are the bulk of them and + the alternative is a per-file advise for each. - **Per-file watches.** Paths outside those directories (project outputs, loose assemblies) get - an individual advise, ref-counted across projects by `FSharpReferenceChangeTracker`. + an individual advise, ref-counted across consumers by `FSharpReferenceChangeTracker`. - **Debounce.** A rebuild writes a temp file and renames it, producing several notifications; the tracker fires one callback per path after 2 s of quiet. -## Consumer - -`FSharpProjectOptionsReactor` watches the `-r:` set of every project it computes options for and -calls `FSharpChecker.InvalidateConfiguration` for each project that references a changed path. -The cached options stay valid (same paths); only the FCS build behind them is stale. Watch sets -are diffed on recompute, so an unchanged reference list touches nothing. +## Consumers -## Follow-ups +None yet — this is the transport, added on its own so the changes that need it stay reviewable: -1. A reference-change notification for the incremental builder on the FCS side, the analogue of - `useChangeNotifications` for sources, so `IsReferencesInvalidated` stops stat'ing every - reference on every request. -2. A watcher-invalidated timestamp cache for snapshot reuse (`ReferencesOnDisk`). -3. Scripts: watch `#r` references and `#load` sources the same way. +1. Scripts: watch `#load` sources (and the script's own `#r` set) so an edit outside the editor + drops the cached options for that document. +2. A watcher-invalidated timestamp cache serving `ReferencesOnDisk`, replacing the stat per + reference per snapshot comparison. +3. A reference-change notification for the incremental builder on the FCS side, the analogue of + `useChangeNotifications` for sources, so `IsReferencesInvalidated` stops stat'ing at all. diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 094efb986e9..db73206996b 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -21,7 +21,6 @@ open System.Windows open Microsoft.VisualStudio open FSharp.Compiler.Text open Microsoft.VisualStudio.TextManager.Interop -open Internal.Utilities.Library #nowarn "57" @@ -117,7 +116,7 @@ type private FSharpProjectOptionsMessage = | ClearSingleFileOptionsCache of DocumentId [] -type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatcher: IFSharpFileChangeWatcher) = +type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let cancellationTokenSource = new CancellationTokenSource() // Store command line options @@ -129,52 +128,6 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch let cache = ConcurrentDictionary() - // Push invalidation for on-disk '-r:' reference assemblies, which the Roslyn workspace does not - // track. The cached options stay valid (same paths); only the FCS build behind them goes stale. - let referenceWatches = ConcurrentDictionary>() - - let onWatchedReferenceChanged (path: string) = - for KeyValue(projectId, paths) in referenceWatches do - if paths.Contains path then - match cache.TryGetValue projectId with - | true, (_, _, projectOptions) -> checker.InvalidateConfiguration(projectOptions, userOpName = "onWatchedReferenceChanged") - | _ -> () - - let referenceChangeTracker = - new FSharpReferenceChangeTracker(fileChangeWatcher, onWatchedReferenceChanged) - - let clearReferenceWatches (projectId: ProjectId) = - match referenceWatches.TryRemove projectId with - | true, paths -> - for path in paths do - referenceChangeTracker.StopWatchingReference path - | _ -> () - - let watchReferenceFiles (projectId: ProjectId) (projectOptions: FSharpProjectOptions) = - let paths = HashSet(StringComparer.OrdinalIgnoreCase) - - for option in projectOptions.OtherOptions do - if option.StartsWithOrdinal "-r:" then - paths.Add(option.Substring "-r:".Length) |> ignore - - match referenceWatches.TryGetValue projectId with - | true, previous -> - for path in previous do - if not (paths.Contains path) then - referenceChangeTracker.StopWatchingReference path - - for path in paths do - if not (previous.Contains path) then - referenceChangeTracker.StartWatchingReference path - | _ -> - for path in paths do - referenceChangeTracker.StartWatchingReference path - - if paths.Count > 0 then - referenceWatches[projectId] <- paths - else - referenceWatches.TryRemove projectId |> ignore - let singleFileCache = ConcurrentDictionary() @@ -478,8 +431,6 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch cache.[projectId] <- struct (project, parsingOptions, projectOptions) - watchReferenceFiles projectId projectOptions - return ValueSome struct (parsingOptions, projectOptions) | true, struct (oldProject, parsingOptions, projectOptions) -> @@ -565,7 +516,6 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch | _ -> () legacyProjectSites.TryRemove(projectId) |> ignore - clearReferenceWatches projectId | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> @@ -608,22 +558,18 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch singleFileCache.Clear() lastSuccessfulCompilations.Clear() - for projectId in referenceWatches.Keys |> Array.ofSeq do - clearReferenceWatches projectId - member _.ScriptUpdated = scriptUpdatedEvent.Publish interface IDisposable with member _.Dispose() = - (referenceChangeTracker :> IDisposable).Dispose() cancellationTokenSource.Cancel() cancellationTokenSource.Dispose() (agent :> IDisposable).Dispose() /// Manages mappings of Roslyn workspace Projects/Documents to FCS. -type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace, fileChangeWatcher: IFSharpFileChangeWatcher) = +type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace) = - let reactor = new FSharpProjectOptionsReactor(checker, fileChangeWatcher) + let reactor = new FSharpProjectOptionsReactor(checker) do // We need to listen to this event for lifecycle purposes. diff --git a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs index 810861270d8..427baf0c6ab 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs @@ -205,11 +205,7 @@ type internal FSharpWorkspaceServiceFactory |> CancellableTask.startAsTask CancellationToken.None |> ignore) - let fileChangeWatcher = - new FSharpFileChangeWatcher(FSharpFileChangeWatcher.CreateDefaultServiceTask()) - - let optionsManager = - FSharpProjectOptionsManager(checker, workspace, fileChangeWatcher) + let optionsManager = FSharpProjectOptionsManager(checker, workspace) { new IFSharpWorkspaceService with member _.Checker = checker From 8b8b21ef5da224745f22527824b9ffb34efde8a2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:48 +0200 Subject: [PATCH 14/25] Keep reference stamps inside the tracker's watch entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../LanguageService/FileChangeWatcher.fs | 57 ++++++++++++-- .../FileChangeWatcherTests.fs | 74 +++++++++++++++++++ 2 files changed, 123 insertions(+), 8 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 284cbad8bee..dd29d6f5cf9 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -66,6 +66,18 @@ type internal IFSharpFileChangeContext = type internal IFSharpFileChangeWatcher = abstract CreateContext: watchedDirectories: ImmutableArray -> IFSharpFileChangeContext +/// Last-write stamps of watched reference files; a path nobody watches is stat'd directly. +type internal IReferenceStamps = + abstract GetLastWriteTimeUtc: fullFilePath: string -> DateTime + abstract Invalidate: fullFilePath: string -> unit + +type private WatchedReference = + { + Token: IFSharpWatchedFile + mutable Count: int + mutable Stamp: DateTime voption + } + [] module private FileChangeWatcherImpl = @@ -339,7 +351,9 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc /// Ref-counted, debounced watching of reference assemblies (or any other off-workspace files), /// modelled on Roslyn's ReferenceFileChangeTracker. Multiple projects watching the same dll -/// share one subscription; bursts of writes produce a single callback per path. +/// share one subscription; bursts of writes produce a single callback per path. The last-write +/// stamp of a path lives inside its watch entry, so a cached stamp is only ever served while a +/// change notification can still reach it. [] type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, onChanged: string -> unit, notificationDelay: TimeSpan) = @@ -347,7 +361,7 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on let mutable disposed = false let watchedFiles = - Dictionary(StringComparer.OrdinalIgnoreCase) + Dictionary(StringComparer.OrdinalIgnoreCase) let pendingTimers = Dictionary(StringComparer.OrdinalIgnoreCase) @@ -402,7 +416,10 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on lock gate (fun () -> // Directory watches cover whole trees; only debounce paths someone watches. - if not disposed && watchedFiles.ContainsKey path then + match watchedFiles.TryGetValue path with + | true, entry when not disposed -> + entry.Stamp <- ValueNone + let timer = match pendingTimers.TryGetValue path with | true, timer -> timer @@ -411,7 +428,8 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on pendingTimers[path] <- timer timer - timer.Change(notificationDelay, Timeout.InfiniteTimeSpan) |> ignore)) + timer.Change(notificationDelay, Timeout.InfiniteTimeSpan) |> ignore + | _ -> ())) ctx) @@ -422,17 +440,40 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on lock gate (fun () -> if not disposed then match watchedFiles.TryGetValue fullFilePath with - | true, (token, count) -> watchedFiles[fullFilePath] <- (token, count + 1) - | _ -> watchedFiles[fullFilePath] <- (context.Value.EnqueueWatchingFile fullFilePath, 1)) + | true, entry -> entry.Count <- entry.Count + 1 + | _ -> + watchedFiles[fullFilePath] <- + { + Token = context.Value.EnqueueWatchingFile fullFilePath + Count = 1 + Stamp = ValueNone + }) member _.StopWatchingReference(fullFilePath: string) = lock gate (fun () -> if not disposed then match watchedFiles.TryGetValue fullFilePath with - | true, (token, 1) -> + | true, { Count = 1; Token = token } -> watchedFiles.Remove fullFilePath |> ignore token.Dispose() - | true, (token, count) -> watchedFiles[fullFilePath] <- (token, count - 1) + | true, entry -> entry.Count <- entry.Count - 1 + | _ -> ()) + + interface IReferenceStamps with + member _.GetLastWriteTimeUtc fullFilePath = + lock gate (fun () -> + match watchedFiles.TryGetValue fullFilePath with + | true, { Stamp = ValueSome stamp } -> stamp + | true, entry -> + let stamp = IO.File.GetLastWriteTimeUtc fullFilePath + entry.Stamp <- ValueSome stamp + stamp + | _ -> IO.File.GetLastWriteTimeUtc fullFilePath) + + member _.Invalidate fullFilePath = + lock gate (fun () -> + match watchedFiles.TryGetValue fullFilePath with + | true, entry -> entry.Stamp <- ValueNone | _ -> ()) interface IDisposable with diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs index d3df726b7b0..269f5476603 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -4,6 +4,7 @@ namespace FSharp.Editor.Tests open System open System.Collections.Immutable +open System.IO open System.Threading open System.Threading.Tasks open Xunit @@ -284,3 +285,76 @@ module FileChangeWatcherTests = UnadvisedDirs [ 1u ] UnadvisedFiles [ 2u ] ] -> Assert.Equal(@"C:\refs\", directory) | calls -> failwith $"Unexpected calls: %A{calls}" + + let private t0 = DateTime(2020, 1, 1, 0, 0, 0, DateTimeKind.Utc) + let private t1 = t0.AddHours 1. + + let private withTempFile (test: string -> unit) = + let path = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid()}.dll") + File.WriteAllBytes(path, Array.empty) + File.SetLastWriteTimeUtc(path, t0) + + try + test path + finally + File.Delete path + + [] + let ``Watched path is served from the cache until a change notification`` () = + withTempFile (fun path -> + let watcher = MockFileChangeWatcher() + use tracker = new FSharpReferenceChangeTracker(watcher, ignore, testDelay) + let stamps = tracker :> IReferenceStamps + + tracker.StartWatchingReference path + Assert.Equal(t0, stamps.GetLastWriteTimeUtc path) + + File.SetLastWriteTimeUtc(path, t1) + Assert.Equal(t0, stamps.GetLastWriteTimeUtc path) + + watcher.Context.Value.Fire path + Assert.Equal(t1, stamps.GetLastWriteTimeUtc path)) + + [] + let ``Unwatched path is stat'd on every read`` () = + withTempFile (fun path -> + let watcher = MockFileChangeWatcher() + use tracker = new FSharpReferenceChangeTracker(watcher, ignore, testDelay) + let stamps = tracker :> IReferenceStamps + + Assert.Equal(t0, stamps.GetLastWriteTimeUtc path) + + File.SetLastWriteTimeUtc(path, t1) + Assert.Equal(t1, stamps.GetLastWriteTimeUtc path)) + + [] + let ``Invalidate drops the cached stamp`` () = + withTempFile (fun path -> + let watcher = MockFileChangeWatcher() + use tracker = new FSharpReferenceChangeTracker(watcher, ignore, testDelay) + let stamps = tracker :> IReferenceStamps + + tracker.StartWatchingReference path + Assert.Equal(t0, stamps.GetLastWriteTimeUtc path) + + File.SetLastWriteTimeUtc(path, t1) + stamps.Invalidate path + Assert.Equal(t1, stamps.GetLastWriteTimeUtc path)) + + [] + let ``Stopping the last watch on a path falls back to stat`` () = + withTempFile (fun path -> + let watcher = MockFileChangeWatcher() + use tracker = new FSharpReferenceChangeTracker(watcher, ignore, testDelay) + let stamps = tracker :> IReferenceStamps + + tracker.StartWatchingReference path + tracker.StartWatchingReference path + Assert.Equal(t0, stamps.GetLastWriteTimeUtc path) + + File.SetLastWriteTimeUtc(path, t1) + tracker.StopWatchingReference path + Assert.Equal(t0, stamps.GetLastWriteTimeUtc path) + + tracker.StopWatchingReference path + Assert.Equal(t1, stamps.GetLastWriteTimeUtc path)) From 45f8373ad425a81522e31665cc149ecd8a2c9048 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:48 +0200 Subject: [PATCH 15/25] Watch each project's reference set for its stamps 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. --- .../FSharpProjectOptionsManager.fs | 66 +++++++++++++++++-- .../LanguageService/LanguageService.fs | 6 +- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index db73206996b..20f1d88c348 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -21,6 +21,7 @@ open System.Windows open Microsoft.VisualStudio open FSharp.Compiler.Text open Microsoft.VisualStudio.TextManager.Interop +open Internal.Utilities.Library #nowarn "57" @@ -116,7 +117,7 @@ type private FSharpProjectOptionsMessage = | ClearSingleFileOptionsCache of DocumentId [] -type private FSharpProjectOptionsReactor(checker: FSharpChecker) = +type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatcher: IFSharpFileChangeWatcher) = let cancellationTokenSource = new CancellationTokenSource() // Store command line options @@ -137,6 +138,51 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let scriptUpdatedEvent = Event() + // The '-r:' set of each project is watched for the reference stamps the snapshot-reuse + // guard reads; invalidating the FCS build on a change is Roslyn's job (it swaps the + // MetadataReference and bumps Project.Version, which reaches tryComputeOptions). + let referenceChangeTracker = + new FSharpReferenceChangeTracker(fileChangeWatcher, ignore) + + let referenceWatches = ConcurrentDictionary>() + + let referencePaths (projectOptions: FSharpProjectOptions) = + let paths = HashSet(StringComparer.OrdinalIgnoreCase) + + for option in projectOptions.OtherOptions do + if option.StartsWithOrdinal "-r:" then + paths.Add(option.Substring "-r:".Length) |> ignore + + paths + + let watchReferenceFiles (projectId: ProjectId) (projectOptions: FSharpProjectOptions) = + let paths = referencePaths projectOptions + + match referenceWatches.TryGetValue projectId with + | true, previous -> + for path in previous do + if not (paths.Contains path) then + referenceChangeTracker.StopWatchingReference path + + for path in paths do + if not (previous.Contains path) then + referenceChangeTracker.StartWatchingReference path + | _ -> + for path in paths do + referenceChangeTracker.StartWatchingReference path + + if paths.Count > 0 then + referenceWatches[projectId] <- paths + else + referenceWatches.TryRemove projectId |> ignore + + let clearReferenceWatches (projectId: ProjectId) = + match referenceWatches.TryRemove projectId with + | true, paths -> + for path in paths do + referenceChangeTracker.StopWatchingReference path + | _ -> () + let createPEReference (referencedProject: Project) (comp: Compilation) = let projectId = referencedProject.Id @@ -410,7 +456,9 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = if not (Seq.isEmpty projectsToClearCache) then projectsToClearCache - |> Seq.iter (fun pair -> cache.TryRemove pair.Key |> ignore) + |> Seq.iter (fun pair -> + cache.TryRemove pair.Key |> ignore + clearReferenceWatches pair.Key) let options = projectsToClearCache @@ -430,6 +478,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let parsingOptions, _ = checker.GetParsingOptionsFromProjectOptions(projectOptions) cache.[projectId] <- struct (project, parsingOptions, projectOptions) + watchReferenceFiles projectId projectOptions return ValueSome struct (parsingOptions, projectOptions) @@ -516,6 +565,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = | _ -> () legacyProjectSites.TryRemove(projectId) |> ignore + clearReferenceWatches projectId | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> @@ -558,18 +608,24 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = singleFileCache.Clear() lastSuccessfulCompilations.Clear() + for projectId in referenceWatches.Keys |> Array.ofSeq do + clearReferenceWatches projectId + member _.ScriptUpdated = scriptUpdatedEvent.Publish + member _.ReferenceStamps = referenceChangeTracker :> IReferenceStamps + interface IDisposable with member _.Dispose() = + (referenceChangeTracker :> IDisposable).Dispose() cancellationTokenSource.Cancel() cancellationTokenSource.Dispose() (agent :> IDisposable).Dispose() /// Manages mappings of Roslyn workspace Projects/Documents to FCS. -type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace) = +type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace, fileChangeWatcher: IFSharpFileChangeWatcher) = - let reactor = new FSharpProjectOptionsReactor(checker) + let reactor = new FSharpProjectOptionsReactor(checker, fileChangeWatcher) do // We need to listen to this event for lifecycle purposes. @@ -638,4 +694,6 @@ type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Wor member _.ClearAllCaches() = reactor.ClearAllCaches() + member _.ReferenceStamps = reactor.ReferenceStamps + member _.Checker = checker diff --git a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs index 427baf0c6ab..810861270d8 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs @@ -205,7 +205,11 @@ type internal FSharpWorkspaceServiceFactory |> CancellableTask.startAsTask CancellationToken.None |> ignore) - let optionsManager = FSharpProjectOptionsManager(checker, workspace) + let fileChangeWatcher = + new FSharpFileChangeWatcher(FSharpFileChangeWatcher.CreateDefaultServiceTask()) + + let optionsManager = + FSharpProjectOptionsManager(checker, workspace, fileChangeWatcher) { new IFSharpWorkspaceService with member _.Checker = checker From 1d9adecfbf3a5768dac1c507bee1c333ff3d94cb Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:49 +0200 Subject: [PATCH 16/25] Read snapshot reference stamps from the tracker instead of stat'ing 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. --- .../LanguageService/WorkspaceExtensions.fs | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs index 2406f3a6e32..d5deef16547 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -18,6 +18,7 @@ open CancellableTasks open System.IO open Internal.Utilities.Collections +open Internal.Utilities.Library open Newtonsoft.Json open Newtonsoft.Json.Linq open System.Text.Json.Nodes @@ -212,17 +213,33 @@ module private CheckerExtensions = |> CancellableTask.whenAll |> CancellableTask.map (Seq.map (fun x -> x.ToString()) >> Set) - let getOnDiskReferences (options: FSharpProjectOptions) = - options.OtherOptions - |> Seq.filter (fun x -> x.StartsWith("-r:")) - |> Seq.map (fun x -> - let path = x.Substring(3) + let getOnDiskReferences (stamps: IReferenceStamps) (options: FSharpProjectOptions) = + [ + for option in options.OtherOptions do + if option.StartsWithOrdinal "-r:" then + let path = option.Substring "-r:".Length - { - Path = path - LastModified = System.IO.File.GetLastWriteTimeUtc path - }) - |> Seq.toList + { + Path = path + LastModified = stamps.GetLastWriteTimeUtc path + } + ] + + // A snapshot's own ReferencesOnDisk come from FCS stat'ing the files, so a mismatch with the + // cached stamps means one side is behind; dropping the stamps costs one re-stat instead of a + // rebuild on every future Project instance after a missed notification. + let referencesOnDiskChanged (project: Project) (oldSnapshot: FSharpProjectSnapshot) options = + let stamps = + project.Solution.GetFSharpWorkspaceService().FSharpProjectOptionsManager.ReferenceStamps + + let current = getOnDiskReferences stamps options + let changed = current <> oldSnapshot.ProjectSnapshot.ReferencesOnDisk + + if changed then + for reference in current do + stamps.Invalidate reference.Path + + changed let createProjectSnapshot (snapshotAccumulatorOpt) (project: Project) (options: FSharpProjectOptions option) = cancellableTask { @@ -246,9 +263,7 @@ module private CheckerExtensions = System.Diagnostics.Trace.TraceWarning "Reference versions changed" None - | true, (true, (_, _, _, _, oldSnapshot: FSharpProjectSnapshot)) when - oldSnapshot.ProjectSnapshot.ReferencesOnDisk <> (getOnDiskReferences options) - -> + | true, (true, (_, _, _, _, oldSnapshot: FSharpProjectSnapshot)) when referencesOnDiskChanged project oldSnapshot options -> System.Diagnostics.Trace.TraceWarning "References on disk changed" None @@ -294,8 +309,8 @@ module private CheckerExtensions = | _ -> None - let! newSnapshot = + let! newSnapshot = match updatedSnapshot with | Some snapshot -> snapshot | _ -> @@ -615,8 +630,8 @@ type Document with cancellableTask { let! checker, _, _, projectOptions = this.GetFSharpCompilationOptionsAsync(userOpName) - let! symbolUses = + let! symbolUses = if this.Project.UseTransparentCompiler then checker.FindBackgroundReferencesInFile(this.FilePath, projectSnapshot, symbol) else From 09eb9750be3caa63d859f8faf1f340b78b9a8b2d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:49 +0200 Subject: [PATCH 17/25] Document the stamp consumer and add its release note --- docs/ide/file-watching.md | 26 ++++++++++++++------ docs/release-notes/.VisualStudio/18.vNext.md | 1 + 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/docs/ide/file-watching.md b/docs/ide/file-watching.md index b9172c01165..53c00399a63 100644 --- a/docs/ide/file-watching.md +++ b/docs/ide/file-watching.md @@ -21,7 +21,7 @@ internally: - `#load` sources of a script: not documents, not references, invisible to the workspace. - `IsReferencesInvalidated` on the incremental builder, which stats every reference on every request. -- `ReferencesOnDisk` on snapshot reuse, which does the same per comparison. +- `ReferencesOnDisk` on snapshot reuse, which did the same per comparison — the consumer below. ## Shape @@ -38,17 +38,29 @@ internally: that watches every `-r:` uniformly wants it, since package assemblies are the bulk of them and the alternative is a per-file advise for each. - **Per-file watches.** Paths outside those directories (project outputs, loose assemblies) get - an individual advise, ref-counted across consumers by `FSharpReferenceChangeTracker`. + an individual advise, ref-counted across consumers by `FSharpReferenceChangeTracker`. The + tracker also keeps the last-write stamp of each watched path (see Consumers). - **Debounce.** A rebuild writes a temp file and renames it, producing several notifications; the tracker fires one callback per path after 2 s of quiet. ## Consumers -None yet — this is the transport, added on its own so the changes that need it stay reviewable: +**Snapshot reference stamps.** `FSharpProjectOptionsReactor` watches the `-r:` set of every +project it computes options for, diffed on recompute so an unchanged set touches nothing. The +tracker keeps each path's last-write stamp inside its watch entry and drops it on the raw change +notification, before the debounce. The `ReferencesOnDisk` guard in `createProjectSnapshot` reads +stamps through `IReferenceStamps`, so the comparison that runs for every new `Project` instance is +a dictionary read per reference instead of a stat. A path nobody watches is stat'd directly. A +mismatch against the snapshot's own stamps (FCS stats when it builds a snapshot) drops the +project's stamps, so a missed notification costs one re-stat pass rather than a rebuild per +`Project` instance. The reactor watch exists for these stamps, not to invalidate the FCS build — +Roslyn already does that, as the previous section says. -1. Scripts: watch `#load` sources (and the script's own `#r` set) so an edit outside the editor - drops the cached options for that document. -2. A watcher-invalidated timestamp cache serving `ReferencesOnDisk`, replacing the stat per - reference per snapshot comparison. +Still to come: + +1. Scripts: watch `#load` sources so an edit outside the editor drops the cached options for + that document. +2. `FSharpProjectSnapshot.FromOptions` stats every `-r:` when a snapshot is built from scratch; + an overload taking host-supplied stamps lets it read the same cache. 3. A reference-change notification for the incremental builder on the FCS side, the analogue of `useChangeNotifications` for sources, so `IsReferencesInvalidated` stops stat'ing at all. diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index dbb51e259f2..a5a46d74641 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -4,6 +4,7 @@ * Expand `` in IDE tooltips, completion, and signature help, inheriting XML documentation from base classes, interfaces, overridden members, and constructors. ([Issue #19175](https://github.com/dotnet/fsharp/issues/19175), [PR #19188](https://github.com/dotnet/fsharp/pull/19188)) ### Fixed +* Transparent compiler snapshot reuse no longer stats every `-r:` reference on each project change; reference timestamps are cached and invalidated by `IVsAsyncFileChangeEx2` notifications. ([PR #20457](https://github.com/dotnet/fsharp/pull/20457)) * Go To Definition no longer blocks the UI thread with a bare `Task.Wait`: the synchronous `IFSharpGoToDefinitionService` call now waits through the cancellable threaded-wait dialog, and the editor's `TaskCompletionSource` bridges run their continuations on the thread pool instead of inline on whichever thread finished the check, so repeated F12 on a large solution no longer starves semantic classification and other main-thread work. ([PR #20482](https://github.com/dotnet/fsharp/pull/20482)) * Peek Definition on an F# symbol whose definition lives in metadata no longer deadlocks Visual Studio. Peek holds the main thread in `JoinableTaskFactory.Run` without pumping messages while it asks the language service for the definition, and generating the metadata document needs that same thread; Peek now stops at definitions that already have a document, and Go To Definition, which owns the wait it makes, still opens the generated one. ([PR #20503](https://github.com/dotnet/fsharp/pull/20503)) From fa1910f1a21bc41acde87855d77d2522438ad520 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 04:10:17 +0200 Subject: [PATCH 18/25] Expose Dispose directly on the tracker and sort the reactor's opens 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. --- .../FSharpProjectOptionsManager.fs | 19 ++++++++------- .../LanguageService/FileChangeWatcher.fs | 24 ++++++++++--------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 20f1d88c348..1b1846e1036 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -8,20 +8,21 @@ open System.Collections.Concurrent open System.Collections.Immutable open System.IO open System.Linq -open Microsoft.CodeAnalysis +open System.Runtime.CompilerServices +open System.Threading +open System.Windows open FSharp.Compiler open FSharp.Compiler.CodeAnalysis +open FSharp.Compiler.Text +open Microsoft.CodeAnalysis +open Microsoft.VisualStudio 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 Internal.Utilities.Library +open CancellableTasks #nowarn "57" @@ -617,10 +618,10 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch interface IDisposable with member _.Dispose() = - (referenceChangeTracker :> IDisposable).Dispose() + referenceChangeTracker.Dispose() cancellationTokenSource.Cancel() cancellationTokenSource.Dispose() - (agent :> IDisposable).Dispose() + agent.Dispose() /// Manages mappings of Roslyn workspace Projects/Documents to FCS. type internal FSharpProjectOptionsManager(checker: FSharpChecker, workspace: Workspace, fileChangeWatcher: IFSharpFileChangeWatcher) = diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index dd29d6f5cf9..e4ae0dbee22 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -476,17 +476,19 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on | true, entry -> entry.Stamp <- ValueNone | _ -> ()) - interface IDisposable with - member _.Dispose() = - lock gate (fun () -> - if not disposed then - disposed <- true - watchedFiles.Clear() + member _.Dispose() = + lock gate (fun () -> + if not disposed then + disposed <- true + watchedFiles.Clear() - for KeyValue(_, timer) in pendingTimers do - timer.Dispose() + for KeyValue(_, timer) in pendingTimers do + timer.Dispose() - pendingTimers.Clear() + pendingTimers.Clear() - if context.IsValueCreated then - context.Value.Dispose()) + if context.IsValueCreated then + context.Value.Dispose()) + + interface IDisposable with + member this.Dispose() = this.Dispose() From 8d75f80862718183502a21c16b15fe96352a335e Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 05:19:19 +0200 Subject: [PATCH 19/25] Dispose the watcher's agent directly --- .../src/FSharp.Editor/LanguageService/FileChangeWatcher.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index e4ae0dbee22..a714280fbb2 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -262,7 +262,7 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task IDisposable).Dispose() + agent.Dispose() and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watchedDirectories: ImmutableArray) as this = From 1c3d942dd9cef9d44eb954ade72aeac6aa78916c Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 05:54:15 +0200 Subject: [PATCH 20/25] Call String.StartsWith/EndsWith directly instead of the illib helpers 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 --- .../LanguageService/FSharpProjectOptionsManager.fs | 3 +-- .../LanguageService/FileChangeWatcher.fs | 11 +++++------ .../LanguageService/WorkspaceExtensions.fs | 5 +---- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index 1b1846e1036..49499802b2f 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -21,7 +21,6 @@ open Microsoft.VisualStudio.FSharp.Interactive.Session open Microsoft.VisualStudio.FSharp.Editor.Extensions open Microsoft.VisualStudio.TextManager.Interop -open Internal.Utilities.Library open CancellableTasks #nowarn "57" @@ -151,7 +150,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch let paths = HashSet(StringComparer.OrdinalIgnoreCase) for option in projectOptions.OtherOptions do - if option.StartsWithOrdinal "-r:" then + if option.StartsWith("-r:", StringComparison.Ordinal) then paths.Add(option.Substring "-r:".Length) |> ignore paths diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index a714280fbb2..399f24464ed 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -12,8 +12,6 @@ open Microsoft.VisualStudio open Microsoft.VisualStudio.Shell open Microsoft.VisualStudio.Shell.Interop -open Internal.Utilities.Library - open Microsoft.VisualStudio.FSharp.Editor.DebugHelpers open CancellableTasks @@ -28,14 +26,14 @@ open CancellableTasks [] type internal WatchedDirectory(path: string, extensionFilters: ImmutableArray) = let path = - if path.EndsWithOrdinal(string IO.Path.DirectorySeparatorChar) then + if path.EndsWith(string IO.Path.DirectorySeparatorChar, StringComparison.Ordinal) then path else $"{path}{IO.Path.DirectorySeparatorChar}" do for filter in extensionFilters do - if not (filter.StartsWithOrdinal ".") then + if not (filter.StartsWith(".", StringComparison.Ordinal)) then invalidArg (nameof extensionFilters) $"Filter '{filter}' must start with a period." member _.Path = path @@ -44,9 +42,10 @@ type internal WatchedDirectory(path: string, extensionFilters: ImmutableArray, filePath: string) = watchedDirectories |> Seq.exists (fun w -> - filePath.StartsWithOrdinalIgnoreCase w.Path + filePath.StartsWith(w.Path, StringComparison.OrdinalIgnoreCase) && (w.ExtensionFilters.IsEmpty - || w.ExtensionFilters |> Seq.exists filePath.EndsWithOrdinalIgnoreCase)) + || w.ExtensionFilters + |> Seq.exists (fun filter -> filePath.EndsWith(filter, StringComparison.OrdinalIgnoreCase)))) /// A single watched file; disposing stops watching. type internal IFSharpWatchedFile = diff --git a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs index d5deef16547..4bdd9d432cb 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -18,7 +18,6 @@ open CancellableTasks open System.IO open Internal.Utilities.Collections -open Internal.Utilities.Library open Newtonsoft.Json open Newtonsoft.Json.Linq open System.Text.Json.Nodes @@ -216,7 +215,7 @@ module private CheckerExtensions = let getOnDiskReferences (stamps: IReferenceStamps) (options: FSharpProjectOptions) = [ for option in options.OtherOptions do - if option.StartsWithOrdinal "-r:" then + if option.StartsWith("-r:", StringComparison.Ordinal) then let path = option.Substring "-r:".Length { @@ -309,7 +308,6 @@ module private CheckerExtensions = | _ -> None - let! newSnapshot = match updatedSnapshot with | Some snapshot -> snapshot @@ -630,7 +628,6 @@ type Document with cancellableTask { let! checker, _, _, projectOptions = this.GetFSharpCompilationOptionsAsync(userOpName) - let! symbolUses = if this.Project.UseTransparentCompiler then checker.FindBackgroundReferencesInFile(this.FilePath, projectSnapshot, symbol) From f7ab57f2f80e25bfc75e9bd82715a8b29629a9cc Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 9 Sep 2026 20:20:38 +0200 Subject: [PATCH 21/25] Never cache a reference stamp before its watch is confirmed active 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 https://github.com/dotnet/fsharp/pull/20457#discussion_r3965934540 Co-Authored-By: Claude Sonnet 5 --- .../LanguageService/FileChangeWatcher.fs | 117 ++++++++++++------ .../FileChangeWatcherTests.fs | 100 +++++++++++++-- 2 files changed, 173 insertions(+), 44 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 399f24464ed..09c5516688f 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -39,18 +39,25 @@ type internal WatchedDirectory(path: string, extensionFilters: ImmutableArray Seq.exists (fun filter -> filePath.EndsWith(filter, StringComparison.OrdinalIgnoreCase))) + static member FilePathCoveredByWatchedDirectories(watchedDirectories: ImmutableArray, filePath: string) = - watchedDirectories - |> Seq.exists (fun w -> - filePath.StartsWith(w.Path, StringComparison.OrdinalIgnoreCase) - && (w.ExtensionFilters.IsEmpty - || w.ExtensionFilters - |> Seq.exists (fun filter -> filePath.EndsWith(filter, StringComparison.OrdinalIgnoreCase)))) + watchedDirectories |> Seq.exists (fun w -> WatchedDirectory.Covers(w, filePath)) /// A single watched file; disposing stops watching. type internal IFSharpWatchedFile = inherit IDisposable + /// True once the underlying advise has succeeded; false while queued or after it has + /// failed. A consumer that caches something derived from a watch must gate the cache on + /// this, not on the watch merely existing - there is no gap in which a change could be + /// missed once it is true. + abstract IsActive: bool + /// A group of file/directory watches sharing one event sink. Disposing unsubscribes everything. type internal IFSharpFileChangeContext = inherit IDisposable @@ -98,18 +105,18 @@ module private FileChangeWatcherImpl = /// writes a temp file then renames, producing several rapid notifications. let defaultNotificationDelay = TimeSpan.FromSeconds 2. - let noOpWatchedFile = - { new IFSharpWatchedFile with - member _.Dispose() = () - } - [] type internal FSharpWatchedFileToken() = member val Cookie: uint32 voption = ValueNone with get, set /// Subscription operations queued for batched application against the file change service. type private WatcherOperation = - | WatchDir of path: string * filters: ImmutableArray * sink: IVsFreeThreadedFileChangeEvents2 * cookies: List + | WatchDir of + path: string * + filters: ImmutableArray * + sink: IVsFreeThreadedFileChangeEvents2 * + cookies: List * + active: bool ref | WatchFiles of paths: string list * tokens: FSharpWatchedFileToken list * sink: IVsFreeThreadedFileChangeEvents2 | UnwatchFiles of tokens: FSharpWatchedFileToken list | UnwatchDirs of cookies: List @@ -128,13 +135,19 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task () - | WatchDir(path, filters, sink, cookies) :: rest -> + | WatchDir(path, filters, sink, cookies, active) :: rest -> pending <- rest - let! cookie = service.AdviseDirChangeAsync(path, true, sink, ct) - cookies.Add cookie - if not filters.IsEmpty then - do! service.FilterDirectoryChangesAsync(cookie, Seq.toArray filters, ct) + try + let! cookie = service.AdviseDirChangeAsync(path, true, sink, ct) + cookies.Add cookie + + if not filters.IsEmpty then + do! service.FilterDirectoryChangesAsync(cookie, Seq.toArray filters, ct) + + active.Value <- true + with ex when not (ex :? OperationCanceledException) -> + FSharpOutputPane.logExceptionWithContext (ex, nameof FSharpFileChangeWatcher) | WatchFiles(_, _, sink) :: _ -> let batch = @@ -162,10 +175,13 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task () |] - let! cookies = service.AdviseFileChangesAsync(paths, watchFlags, sink, ct) + try + let! cookies = service.AdviseFileChangesAsync(paths, watchFlags, sink, ct) - (tokens, cookies) - ||> Array.iter2 (fun token cookie -> token.Cookie <- ValueSome cookie) + (tokens, cookies) + ||> Array.iter2 (fun token cookie -> token.Cookie <- ValueSome cookie) + with ex when not (ex :? OperationCanceledException) -> + FSharpOutputPane.logExceptionWithContext (ex, nameof FSharpFileChangeWatcher) | UnwatchFiles _ :: _ -> let batch = @@ -192,16 +208,22 @@ type internal FSharpFileChangeWatcher(fileChangeService: Task () |] - if cookies.Length > 0 then - let! _ = service.UnadviseFileChangesAsync(cookies, ct) - () + try + if cookies.Length > 0 then + let! _ = service.UnadviseFileChangesAsync(cookies, ct) + () + with ex when not (ex :? OperationCanceledException) -> + FSharpOutputPane.logExceptionWithContext (ex, nameof FSharpFileChangeWatcher) | UnwatchDirs cookies :: rest -> pending <- rest - if cookies.Count > 0 then - let! _ = service.UnadviseDirChangesAsync(cookies.ToArray(), ct) - () + try + if cookies.Count > 0 then + let! _ = service.UnadviseDirChangesAsync(cookies.ToArray(), ct) + () + with ex when not (ex :? OperationCanceledException) -> + FSharpOutputPane.logExceptionWithContext (ex, nameof FSharpFileChangeWatcher) } let cancellationTokenSource = new CancellationTokenSource() @@ -271,6 +293,19 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc let directoryCookies = List() let fileChanged = Event() + // One activation flag per watched directory: a path it covers is only ever cached once that + // directory's own advise has succeeded, never while still queued or after it has failed. + let directoryActive = + watchedDirectories |> Seq.map (fun _ -> ref false) |> Seq.toArray + + let directoryWatchedFiles = + directoryActive + |> Array.map (fun active -> + { new IFSharpWatchedFile with + member _.IsActive = active.Value + member _.Dispose() = () + }) + let raiseChanges (count: uint32) (files: string[]) (changeFlags: uint32[]) = for i in 0 .. int count - 1 do if @@ -282,15 +317,17 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc VSConstants.S_OK do - for watchedDirectory in watchedDirectories do + watchedDirectories + |> Seq.iteri (fun i watchedDirectory -> enqueue ( WatchDir( watchedDirectory.Path, watchedDirectory.ExtensionFilters, this :> IVsFreeThreadedFileChangeEvents2, - directoryCookies + directoryCookies, + directoryActive[i] ) - ) + )) member private _.StopWatchingFile(token: FSharpWatchedFileToken) = lock gate (fun () -> activeFileTokens.Remove token |> ignore) @@ -301,14 +338,18 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc member _.FileChanged = fileChanged.Publish member _.EnqueueWatchingFile filePath = - if WatchedDirectory.FilePathCoveredByWatchedDirectories(watchedDirectories, filePath) then - noOpWatchedFile - else + match + watchedDirectories + |> Seq.tryFindIndex (fun w -> WatchedDirectory.Covers(w, filePath)) + with + | Some i -> directoryWatchedFiles[i] + | None -> let token = FSharpWatchedFileToken() lock gate (fun () -> activeFileTokens.Add token |> ignore) enqueue (WatchFiles([ filePath ], [ token ], this :> IVsFreeThreadedFileChangeEvents2)) { new IFSharpWatchedFile with + member _.IsActive = token.Cookie.IsSome member _.Dispose() = this.StopWatchingFile token } @@ -462,11 +503,13 @@ type internal FSharpReferenceChangeTracker(watcher: IFSharpFileChangeWatcher, on member _.GetLastWriteTimeUtc fullFilePath = lock gate (fun () -> match watchedFiles.TryGetValue fullFilePath with - | true, { Stamp = ValueSome stamp } -> stamp - | true, entry -> - let stamp = IO.File.GetLastWriteTimeUtc fullFilePath - entry.Stamp <- ValueSome stamp - stamp + | true, entry when entry.Token.IsActive -> + match entry.Stamp with + | ValueSome stamp -> stamp + | ValueNone -> + let stamp = IO.File.GetLastWriteTimeUtc fullFilePath + entry.Stamp <- ValueSome stamp + stamp | _ -> IO.File.GetLastWriteTimeUtc fullFilePath) member _.Invalidate fullFilePath = diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs index 269f5476603..75314543970 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -3,6 +3,7 @@ namespace FSharp.Editor.Tests open System +open System.Collections.Generic open System.Collections.Immutable open System.IO open System.Threading @@ -11,12 +12,26 @@ open Xunit open Microsoft.VisualStudio.Shell open Microsoft.VisualStudio.FSharp.Editor +/// Stands in for the token EnqueueWatchingFile hands back: IsActive defaults to true (the mock's +/// watch is live the moment it is created) so most tests do not need to think about it; tests +/// exercising the pending/failed states flip it through the owning context's SetActive. +type private MockWatchedFile(onDispose: unit -> unit) = + let mutable isActive = true + + member _.SetActive value = isActive <- value + + interface IFSharpWatchedFile with + member _.IsActive = isActive + member _.Dispose() = onDispose () + type private MockFileChangeContext() = let fileChanged = Event() let watched = ResizeArray() + let tokens = Dictionary(StringComparer.OrdinalIgnoreCase) member _.WatchedFiles = List.ofSeq watched member _.Fire path = fileChanged.Trigger path + member _.SetActive(path, active) = tokens[path].SetActive active interface IFSharpFileChangeContext with [] @@ -24,10 +39,9 @@ type private MockFileChangeContext() = member _.EnqueueWatchingFile path = watched.Add path - - { new IFSharpWatchedFile with - member _.Dispose() = watched.Remove path |> ignore - } + let token = MockWatchedFile(fun () -> watched.Remove path |> ignore) + tokens[path] <- token + token :> IFSharpWatchedFile member _.Dispose() = watched.Clear() @@ -56,6 +70,7 @@ type private ServiceCall = type private RecordingFileChangeService() = let calls = ResizeArray() let mutable nextCookie = 0u + let mutable failAdviseDirForPath: string option = None let record call = lock calls (fun () -> calls.Add call) @@ -65,6 +80,12 @@ type private RecordingFileChangeService() = member _.Calls = lock calls (fun () -> List.ofSeq calls) + /// Every AdviseDirChangeAsync for this exact (already directory-separator-normalized) path + /// throws instead of succeeding, so a test can exercise what happens to the rest of a batch + /// when one operation in it fails. + member _.FailAdviseDirForPath + with set value = failAdviseDirForPath <- value + member this.WaitForCalls(count: int) = let deadline = DateTime.UtcNow + TimeSpan.FromSeconds 10. @@ -88,9 +109,13 @@ type private RecordingFileChangeService() = Task.FromResult Array.empty member _.AdviseDirChangeAsync(directory, _, _, _) = - let cookie = newCookie () - record (AdvisedDir(directory, cookie)) - Task.FromResult cookie + match failAdviseDirForPath with + | Some p when String.Equals(p, directory, StringComparison.OrdinalIgnoreCase) -> + Task.FromException(InvalidOperationException "simulated advise failure") + | _ -> + let cookie = newCookie () + record (AdvisedDir(directory, cookie)) + Task.FromResult cookie member _.UnadviseDirChangeAsync(_, _) = Task.FromResult "" @@ -286,6 +311,43 @@ module FileChangeWatcherTests = UnadvisedFiles [ 2u ] ] -> Assert.Equal(@"C:\refs\", directory) | calls -> failwith $"Unexpected calls: %A{calls}" + [] + let ``A directory-covered watch becomes active once the directory's advise succeeds`` () = + let service = RecordingFileChangeService() + use watcher = createWatcher service + + let directories = + ImmutableArray.Create(WatchedDirectory(@"C:\refs", ImmutableArray.Create ".dll")) + + use context = (watcher :> IFSharpFileChangeWatcher).CreateContext directories + let watched = context.EnqueueWatchingFile @"C:\refs\a.dll" + + Assert.False watched.IsActive + service.WaitForCalls 2 |> ignore + Assert.True watched.IsActive + + [] + let ``A failing directory advise does not block the rest of the batch`` () = + let service = RecordingFileChangeService() + service.FailAdviseDirForPath <- Some @"C:\bad\" + use watcher = createWatcher service + + let directories = + ImmutableArray.Create( + WatchedDirectory(@"C:\bad", ImmutableArray.Create ".dll"), + WatchedDirectory(@"C:\good", ImmutableArray.Create ".dll") + ) + + use context = (watcher :> IFSharpFileChangeWatcher).CreateContext directories + let badWatch = context.EnqueueWatchingFile @"C:\bad\a.dll" + let goodWatch = context.EnqueueWatchingFile @"C:\good\a.dll" + + // The failing directory's advise never gets recorded; the good one still does. + service.WaitForCalls 2 |> ignore + + Assert.False badWatch.IsActive + Assert.True goodWatch.IsActive + let private t0 = DateTime(2020, 1, 1, 0, 0, 0, DateTimeKind.Utc) let private t1 = t0.AddHours 1. @@ -358,3 +420,27 @@ module FileChangeWatcherTests = tracker.StopWatchingReference path Assert.Equal(t1, stamps.GetLastWriteTimeUtc path)) + + [] + let ``A pending watch always stats fresh; caching begins only once it becomes active`` () = + withTempFile (fun path -> + let watcher = MockFileChangeWatcher() + use tracker = new FSharpReferenceChangeTracker(watcher, ignore, testDelay) + let stamps = tracker :> IReferenceStamps + + tracker.StartWatchingReference path + let context = watcher.Context.Value + context.SetActive(path, false) + + Assert.Equal(t0, stamps.GetLastWriteTimeUtc path) + + // Still pending: an external change is visible on the very next read, nothing cached yet. + File.SetLastWriteTimeUtc(path, t1) + Assert.Equal(t1, stamps.GetLastWriteTimeUtc path) + + // Once the advise is confirmed, this read is the one that gets trusted from here on. + context.SetActive(path, true) + Assert.Equal(t1, stamps.GetLastWriteTimeUtc path) + + File.SetLastWriteTimeUtc(path, t0) + Assert.Equal(t1, stamps.GetLastWriteTimeUtc path)) From 41eec4aa561eec9b25708f04be20b5ed5c74bd0e Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 9 Sep 2026 20:36:42 +0200 Subject: [PATCH 22/25] Use Seq.tryFindIndexV instead of the option-returning Seq.tryFindIndex 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 --- .../src/FSharp.Editor/LanguageService/FileChangeWatcher.fs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 09c5516688f..232751e0c0f 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -340,10 +340,10 @@ and [] private FileChangeContext(enqueue: WatcherOperation -> unit, watc member _.EnqueueWatchingFile filePath = match watchedDirectories - |> Seq.tryFindIndex (fun w -> WatchedDirectory.Covers(w, filePath)) + |> Seq.tryFindIndexV (fun w -> WatchedDirectory.Covers(w, filePath)) with - | Some i -> directoryWatchedFiles[i] - | None -> + | ValueSome i -> directoryWatchedFiles[i] + | ValueNone -> let token = FSharpWatchedFileToken() lock gate (fun () -> activeFileTokens.Add token |> ignore) enqueue (WatchFiles([ filePath ], [ token ], this :> IVsFreeThreadedFileChangeEvents2)) From cdf35a78d7d9fb426369db1842bb94613f77655b Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 11 Sep 2026 17:49:18 +0200 Subject: [PATCH 23/25] Fix the FS0760 that broke every Windows leg, and stop timing the IsActive 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 --- .../FileChangeWatcherTests.fs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs index 75314543970..2620d7d255b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/FileChangeWatcherTests.fs @@ -39,7 +39,7 @@ type private MockFileChangeContext() = member _.EnqueueWatchingFile path = watched.Add path - let token = MockWatchedFile(fun () -> watched.Remove path |> ignore) + let token = new MockWatchedFile(fun () -> watched.Remove path |> ignore) tokens[path] <- token token :> IFSharpWatchedFile @@ -142,6 +142,9 @@ module FileChangeWatcherTests = let private createWatcher (service: RecordingFileChangeService) = new FSharpFileChangeWatcher(Task.FromResult(service :> IVsAsyncFileChangeEx2), batchDelay) + let private becomesActive (watched: IFSharpWatchedFile) = + SpinWait.SpinUntil((fun () -> watched.IsActive), TimeSpan.FromSeconds 10.) + [] let ``WatchedDirectory covers files under it matching the extension filter`` () = let dirs = @@ -313,8 +316,10 @@ module FileChangeWatcherTests = [] let ``A directory-covered watch becomes active once the directory's advise succeeds`` () = - let service = RecordingFileChangeService() - use watcher = createWatcher service + let serviceAvailable = + TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) + + use watcher = new FSharpFileChangeWatcher(serviceAvailable.Task, batchDelay) let directories = ImmutableArray.Create(WatchedDirectory(@"C:\refs", ImmutableArray.Create ".dll")) @@ -322,9 +327,11 @@ module FileChangeWatcherTests = use context = (watcher :> IFSharpFileChangeWatcher).CreateContext directories let watched = context.EnqueueWatchingFile @"C:\refs\a.dll" + // The advise is queued but cannot run until the service is available, however long that takes. Assert.False watched.IsActive - service.WaitForCalls 2 |> ignore - Assert.True watched.IsActive + + serviceAvailable.SetResult(RecordingFileChangeService()) + Assert.True(becomesActive watched) [] let ``A failing directory advise does not block the rest of the batch`` () = @@ -342,11 +349,10 @@ module FileChangeWatcherTests = let badWatch = context.EnqueueWatchingFile @"C:\bad\a.dll" let goodWatch = context.EnqueueWatchingFile @"C:\good\a.dll" - // The failing directory's advise never gets recorded; the good one still does. - service.WaitForCalls 2 |> ignore - + // Both directories are advised in the same batch, the failing one first, so once the good + // one is active the bad one has already had its only chance. + Assert.True(becomesActive goodWatch) Assert.False badWatch.IsActive - Assert.True goodWatch.IsActive let private t0 = DateTime(2020, 1, 1, 0, 0, 0, DateTimeKind.Utc) let private t1 = t0.AddHours 1. From a3ec7f5e6c6250ed3629587a5f35cdcad01aff29 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 13:47:29 +0200 Subject: [PATCH 24/25] Keep one release note, in the version in development 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) --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 1 - docs/release-notes/.FSharp.Compiler.Service/11.0.200.md | 1 + docs/release-notes/.VisualStudio/18.vNext.md | 1 - 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 337ee638886..f383908e195 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -169,7 +169,6 @@ * Fix signature generation (`fsc --sig`, `GenerateSignature`, `GetValSignatureText`) dropping the parentheses around a destructured or pattern-annotated tuple parameter when a sibling argument in the same curried group is named, so `(int * float) * z: string` no longer prints as the flat 3-tuple `int * float * z: string`. ([Issue #20397](https://github.com/dotnet/fsharp/issues/20397), [PR #20589](https://github.com/dotnet/fsharp/pull/20589)) ### Added -* `Internal.Utilities.Library`: `String.StartsWithOrdinalIgnoreCase` extension, the `StartsWith` sibling of `EndsWithOrdinalIgnoreCase`. ([PR #20457](https://github.com/dotnet/fsharp/pull/20457)) * FCS: add FSharpCheckFileResults.FileSignature ([PR #20478](https://github.com/dotnet/fsharp/pull/20478)) * Added the `ReraiseInComputationExpressions` language feature (`--langversion:preview`): `reraise ()` in the `with` handler of a computation expression is compiled to a rethrow through `ExceptionDispatchInfo` instead of being rejected with FS0413. ([Suggestion #660](https://github.com/fsharp/fslang-suggestions/issues/660), [RFC FS-1347](https://github.com/fsharp/fslang-design/pull/843), [PR #20405](https://github.com/dotnet/fsharp/pull/20405)) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md index 6212188b65f..c237683a579 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md @@ -1,5 +1,6 @@ ### Added +* `Internal.Utilities.Library`: `String.StartsWithOrdinalIgnoreCase` extension, the `StartsWith` sibling of `EndsWithOrdinalIgnoreCase`. ([PR #20457](https://github.com/dotnet/fsharp/pull/20457)) * F# Interactive gains a JSON-RPC server mode, `--fsi-server-jsonrpc:`, in which a host submits interactions over a named pipe and receives structured results — diagnostics with positions, escaping exceptions, the values each interaction bound, and the session's own process id — instead of recovering them by looking for a `SERVER-PROMPT>` marker in the output text. Program output continues to flow through the redirected console streams. The pipe admits only the user running the session; `--fsi-server-client-pid:` names the host process whose exit ends the session. `FsiEvaluationSession` exposes both options as `JsonRpcServerPipeName` and `JsonRpcClientProcessId`. The mode is part of the .NET fsi only. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396)) ### Fixed diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index a5a46d74641..4c00fcecc0d 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -13,7 +13,6 @@ * 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)) * Avoid using `cancellableTask` in `DocumentCache`; the editor cache now uses direct `CancellationToken`-aware `task` wrappers, avoiding the background `Task.Run` offload and a larger wrapper closure from the `cancellableTask` builder. ([Issue #20268](https://github.com/dotnet/fsharp/issues/20268)) * Cache document diagnostics by version stamp, so an unchanged document is not reanalyzed on every crawler pass. ([Issue #20120](https://github.com/dotnet/fsharp/issues/20120), [PR #20121](https://github.com/dotnet/fsharp/pull/20121)) -* 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)) * 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)) From c1a2d725d62665c5d518a6135c77806f53c2e56d Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 14:47:08 +0200 Subject: [PATCH 25/25] Watch a reference's appearance and removal, not only its writes 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) --- .../FSharp.Editor/LanguageService/FileChangeWatcher.fs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs index 232751e0c0f..ff04a938c52 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FileChangeWatcher.fs @@ -87,9 +87,15 @@ type private WatchedReference = [] module private FileChangeWatcherImpl = - // Same flags Roslyn uses for both subscribing and filtering callbacks. + // What a watch on a single file asks to be told about. Roslyn asks for size and time; a reference is + // also watched before the project that produces it has been built, and is replaced by a rename rather + // than a write, so its appearance and its removal have to arrive too. A directory watch takes no flags - + // `AdviseDirChange` reports every kind - which is why the callback filter below names all four. let watchFlags = - _VSFILECHANGEFLAGS.VSFILECHG_Size ||| _VSFILECHANGEFLAGS.VSFILECHG_Time + _VSFILECHANGEFLAGS.VSFILECHG_Size + ||| _VSFILECHANGEFLAGS.VSFILECHG_Time + ||| _VSFILECHANGEFLAGS.VSFILECHG_Add + ||| _VSFILECHANGEFLAGS.VSFILECHG_Del let relevantFlags = _VSFILECHANGEFLAGS.VSFILECHG_Time