From 9d995a65fec9188ebe9afb7e732e4e7a02db0992 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 17 Aug 2026 22:35:05 +0200 Subject: [PATCH 01/20] 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 fef605b8c3a1a40637339a29df2f1b5a3aaac5d2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 17 Aug 2026 23:31:12 +0200 Subject: [PATCH 02/20] 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 9629f738f1c9ede47143f4b488f9f68f9643848a Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 17 Aug 2026 23:36:03 +0200 Subject: [PATCH 03/20] 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 08bfbbddaa8..e8e7e4683b2 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -113,7 +113,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 @@ -124,6 +124,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() @@ -429,6 +467,8 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = cache.[projectId] <- (project, parsingOptions, projectOptions) + watchReferenceFiles projectId projectOptions + return ValueSome(parsingOptions, projectOptions) | true, (oldProject, parsingOptions, projectOptions) -> @@ -514,6 +554,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = | _ -> () legacyProjectSites.TryRemove(projectId) |> ignore + clearReferenceWatches projectId | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> @@ -556,18 +597,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 48d274274386ba688d356de7c1df4d6ffd3bbff8 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Tue, 18 Aug 2026 00:01:53 +0200 Subject: [PATCH 04/20] 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 ef64e1a75c4..2151f05220e 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -9,6 +9,7 @@ * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * 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)) +* 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 2c1ab982676ab4aadabea286f7ecd25d10fba5dc Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 21:26:38 +0200 Subject: [PATCH 05/20] Link the file-watching release note to its PR --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2151f05220e..430e7660b2b 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -9,7 +9,7 @@ * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * 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)) -* 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 57f2bb0ce454db0a1dbd5581d70e725b76a36d9e Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 22:34:55 +0200 Subject: [PATCH 06/20] 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 e8e7e4683b2..52058aae139 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" @@ -113,7 +114,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 @@ -127,40 +128,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() @@ -604,15 +604,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 4b204823ddd3275d0d2551c494a8855fb7ef58ce Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 22:56:13 +0200 Subject: [PATCH 07/20] 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 439e7ce72b80e68b1025eba2e6c042d8299978cc Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 23:25:30 +0200 Subject: [PATCH 08/20] 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 ba2667f476411a20a6a9925924486f28c2621815 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 5 Sep 2026 23:42:15 +0200 Subject: [PATCH 09/20] 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 9441f8587e0..e7651463bd5 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -148,6 +148,7 @@ * IL: fix leaking binary view ([PR #20250](https://github.com/dotnet/fsharp/pull/20250)) ### Added +* `Internal.Utilities.Library`: `String.StartsWithOrdinalIgnoreCase` extension, the `StartsWith` sibling of `EndsWithOrdinalIgnoreCase`. ([PR #20457](https://github.com/dotnet/fsharp/pull/20457)) * Added a "most concrete" tiebreaker for overload resolution (`--langversion:preview`). ([RFC FS-1340](https://github.com/fsharp/fslang-design/pull/834), [PR #19277](https://github.com/dotnet/fsharp/pull/19277)) * Added support for `OverloadResolutionPriorityAttribute` in overload resolution (`--langversion:preview`). ([RFC FS-1338](https://github.com/fsharp/fslang-design/pull/828), [PR #19277](https://github.com/dotnet/fsharp/pull/19277)) diff --git a/src/Compiler/Utilities/illib.fs b/src/Compiler/Utilities/illib.fs index 5842492b31d..d84eccf6dae 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 7c598ffa7a2..a216da229e0 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 d8dce7ac9473973410ad3d58a58fe9ce26157e0d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 00:47:47 +0200 Subject: [PATCH 10/20] 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 52058aae139..c92fc4e9eed 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs @@ -125,15 +125,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") | _ -> () @@ -148,19 +147,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 704a0adc7ab7d9d8c1a57545a5fbdfe168504214 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 00:59:54 +0200 Subject: [PATCH 11/20] 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 761e069ee13be510be0caefd6a6279baa87bc266 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 01:19:32 +0200 Subject: [PATCH 12/20] 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 c15f47bd52b14356fb7b523a47b60fb5f0d42d3e Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:02:38 +0200 Subject: [PATCH 13/20] 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 ++++++++++-------- docs/release-notes/.VisualStudio/18.vNext.md | 1 - .../FSharpProjectOptionsManager.fs | 60 +------------------ .../LanguageService/LanguageService.fs | 6 +- 4 files changed, 36 insertions(+), 88 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/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 430e7660b2b..ef64e1a75c4 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -9,7 +9,6 @@ * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * 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)) -* 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)) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs b/vsintegration/src/FSharp.Editor/LanguageService/FSharpProjectOptionsManager.fs index c92fc4e9eed..08bfbbddaa8 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" @@ -114,7 +113,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 @@ -125,52 +124,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() @@ -476,8 +429,6 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch cache.[projectId] <- (project, parsingOptions, projectOptions) - watchReferenceFiles projectId projectOptions - return ValueSome(parsingOptions, projectOptions) | true, (oldProject, parsingOptions, projectOptions) -> @@ -563,7 +514,6 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker, fileChangeWatch | _ -> () legacyProjectSites.TryRemove(projectId) |> ignore - clearReferenceWatches projectId | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> @@ -606,22 +556,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 dadf230f5af78909d9d2e1a02933a07e4a1d0722 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:48 +0200 Subject: [PATCH 14/20] 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 744355228a5d7890422d6bb8f1f194a7e5db3564 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:48 +0200 Subject: [PATCH 15/20] 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 08bfbbddaa8..553132a86fb 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" @@ -113,7 +114,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 @@ -133,6 +134,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 @@ -408,7 +454,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 @@ -428,6 +476,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = let parsingOptions, _ = checker.GetParsingOptionsFromProjectOptions(projectOptions) cache.[projectId] <- (project, parsingOptions, projectOptions) + watchReferenceFiles projectId projectOptions return ValueSome(parsingOptions, projectOptions) @@ -514,6 +563,7 @@ type private FSharpProjectOptionsReactor(checker: FSharpChecker) = | _ -> () legacyProjectSites.TryRemove(projectId) |> ignore + clearReferenceWatches projectId | FSharpProjectOptionsMessage.ClearSingleFileOptionsCache(documentId) -> match singleFileCache.TryRemove(documentId) with | true, (_, _, _, projectOptions, subscription) -> @@ -556,18 +606,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. @@ -636,4 +692,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 7153224f14aa4ef5175c551459815d18fca99f40 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:49 +0200 Subject: [PATCH 16/20] 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 15bd8ce03ef6fd15f9258cda4f27690bebda099c Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 03:53:49 +0200 Subject: [PATCH 17/20] 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 ef64e1a75c4..5b22655b08e 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)) * Improve Find All References performance by throttling parallel typechecks. ([PR #20128](https://github.com/dotnet/fsharp/pull/20128)) * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) From c5739d9120416dd4d2618b30f70f8302b4c3cca2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 04:10:17 +0200 Subject: [PATCH 18/20] 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 553132a86fb..b068c68544c 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" @@ -615,10 +616,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 bc53ca8cb03f74da0d2341ef56c40090dce1a91a Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 05:19:19 +0200 Subject: [PATCH 19/20] 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 6f7e4be11499b3880e957dc37157b55c47361b61 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 05:54:15 +0200 Subject: [PATCH 20/20] 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 b068c68544c..cecf4306e84 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" @@ -147,7 +146,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)