Merge pull request #42180 from jasonmalinowski/switch-to-directory-watching

Watch directories for source files and some metadata references

src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeWatcher.cs

@@ -6,6 +6,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Threading;
 using System.Threading.Tasks;
@@ -76,20 +77,49 @@ public void WaitForQueue_TestOnly()
             queue.Wait();
         }
 
-        public IContext CreateContext()
-            => new Context(this, null);
+        public IContext CreateContext(params WatchedDirectory[] watchedDirectories)
+        {
+            return new Context(this, watchedDirectories.ToImmutableArray());
+        }
 
         /// <summary>
-        /// Creates an <see cref="IContext"/> that watches all files in a directory, in addition to any files explicitly requested by <see cref="IContext.EnqueueWatchingFile(string)"/>.
+        /// Gives a hint to the <see cref="IContext"/> that we should watch a top-level directory for all changes in addition
+        /// to any files called by <see cref="IContext.EnqueueWatchingFile(string)"/>.
         /// </summary>
-        public IContext CreateContextForDirectory(string directoryFilePath)
+        /// <remarks>
+        /// This is largely intended as an optimization; consumers should still call <see cref="IContext.EnqueueWatchingFile(string)" />
+        /// for files they want to watch. This allows the caller to give a hint that it is expected that most of the files being
+        /// watched is under this directory, and so it's more efficient just to watch _all_ of the changes in that directory
+        /// rather than creating and tracking a bunch of file watcher state for each file separately. A good example would be
+        /// just creating a single directory watch on the root of a project for source file changes: rather than creating a file watcher
+        /// for each individual file, we can just watch the entire directory and that's it.
+        /// </remarks>
+        public sealed class WatchedDirectory
         {
-            if (directoryFilePath == null)
+            public WatchedDirectory(string path, string? extensionFilter)
             {
-                throw new ArgumentNullException(nameof(directoryFilePath));
+                // We are doing string comparisons with this path, so ensure it has a trailing \ so we don't get confused with sibling
+                // paths that won't actually be covered.
+                if (!path.EndsWith(System.IO.Path.DirectorySeparatorChar.ToString()))
+                {
+                    path += System.IO.Path.DirectorySeparatorChar;
+                }
+
+                if (extensionFilter != null && !extensionFilter.StartsWith("."))
+                {
+                    throw new ArgumentException($"{nameof(extensionFilter)} should start with a period.", nameof(extensionFilter));
+                }
+
+                Path = path;
+                ExtensionFilter = extensionFilter;
             }
 
-            return new Context(this, directoryFilePath);
+            public string Path { get; }
+
+            /// <summary>
+            /// If non-null, only watch the directory for changes to a specific extension. String always starts with a period.
+            /// </summary>
+            public string? ExtensionFilter { get; }
         }
 
         /// <summary>
@@ -124,7 +154,7 @@ public interface IFileWatchingToken
         private sealed class Context : IVsFreeThreadedFileChangeEvents2, IContext
         {
             private readonly FileChangeWatcher _fileChangeWatcher;
-            private readonly string? _directoryFilePath;
+            private readonly ImmutableArray<WatchedDirectory> _watchedDirectories;
             private readonly IFileWatchingToken _noOpFileWatchingToken;
 
             /// <summary>
@@ -133,26 +163,41 @@ private sealed class Context : IVsFreeThreadedFileChangeEvents2, IContext
             private readonly object _gate = new object();
             private bool _disposed = false;
             private readonly HashSet<FileWatchingToken> _activeFileWatchingTokens = new HashSet<FileWatchingToken>();
-            private uint _directoryWatchCookie;
 
-            public Context(FileChangeWatcher fileChangeWatcher, string? directoryFilePath)
+            /// <summary>
+            /// The list of cookies we used to make watchers for <see cref="_watchedDirectories"/>.
+            /// </summary>
+            /// <remarks>
+            /// This does not need to be used under <see cref="_gate"/>, as it's only used inside the actual queue of file watcher
+            /// actions.
+            /// </remarks>
+            private readonly List<uint> _directoryWatchCookies = new List<uint>();
+
+            public Context(FileChangeWatcher fileChangeWatcher, ImmutableArray<WatchedDirectory> watchedDirectories)
             {
                 _fileChangeWatcher = fileChangeWatcher;
+                _watchedDirectories = watchedDirectories;
                 _noOpFileWatchingToken = new FileWatchingToken();
 
-                if (directoryFilePath != null)
+                foreach (var watchedDirectory in watchedDirectories)
                 {
-                    if (!directoryFilePath.EndsWith("\\"))
+                    _fileChangeWatcher.EnqueueWork(
+                        async service =>
                         {
-                        directoryFilePath += "\\";
-                    }
+                            var cookie = await service.AdviseDirChangeAsync(watchedDirectory.Path, watchSubdirectories: true, this).ConfigureAwait(false);
+                            _directoryWatchCookies.Add(cookie);
 
-                    _directoryFilePath = directoryFilePath;
+                            if (watchedDirectory.ExtensionFilter != null)
+                            {
+                                // TODO: switch to proper reference assemblies
+                                var filterDirectoryChangesAsyncMethod = service.GetType().GetMethod("FilterDirectoryChangesAsync");
 
-                    _fileChangeWatcher.EnqueueWork(
-                        async service =>
+                                if (filterDirectoryChangesAsyncMethod != null)
                                 {
-                            _directoryWatchCookie = await service.AdviseDirChangeAsync(_directoryFilePath, watchSubdirectories: true, this).ConfigureAwait(false);
+                                    var arguments = new object[] { cookie, new string[] { watchedDirectory.ExtensionFilter }, CancellationToken.None };
+                                    await ((Task)filterDirectoryChangesAsyncMethod.Invoke(service, arguments)).ConfigureAwait(false);
+                                }
+                            }
                         });
                 }
             }
@@ -172,14 +217,20 @@ public void Dispose()
                 _fileChangeWatcher.EnqueueWork(
                     async service =>
                     {
-                        // Since we put all of our work in a queue, we know that if we had tried to advise file or directory changes,
-                        // it must have happened before now
-                        if (_directoryFilePath != null)
+                        // This cleanup code all runs in the single queue that we push usages of the file change service into.
+                        // Therefore, we know that any advise operations we had done have ran in that queue by now. Since this is also
+                        // running after dispose, we don't need to take any locks at this point, since we're taking the general policy
+                        // that any use of the type after it's been disposed is simply undefined behavior.
+
+                        // We don't use IAsyncDisposable here simply because we don't ever want to block on the queue if we're
+                        // able to avoid it, since that would potentially cause a stall or UI delay on shutting down.
+
+                        foreach (var cookie in _directoryWatchCookies)
                         {
-                            await service.UnadviseDirChangeAsync(_directoryWatchCookie).ConfigureAwait(false);
+                            await service.UnadviseDirChangeAsync(cookie).ConfigureAwait(false);
                         }
 
-                        // it runs after disposed. so no lock is needed for _activeFileWatchingTokens
+                        // Since this runs after disposal, no lock is needed for _activeFileWatchingTokens
                         foreach (var token in _activeFileWatchingTokens)
                         {
                             await UnsubscribeFileChangeEventsAsync(service, token).ConfigureAwait(false);
@@ -189,11 +240,20 @@ public void Dispose()
 
             public IFileWatchingToken EnqueueWatchingFile(string filePath)
             {
-                // If we already have this file under our path, we don't have to do additional watching
-                if (_directoryFilePath != null && filePath.StartsWith(_directoryFilePath))
+                // If we already have this file under our path, we may not have to do additional watching
+                foreach (var watchedDirectory in _watchedDirectories)
+                {
+                    if (watchedDirectory != null && filePath.StartsWith(watchedDirectory.Path))
+                    {
+                        // If ExtensionFilter is null, then we're watching for all files in the directory so the prior check
+                        // of the directory containment was sufficient. If it isn't null, then we have to check the extension
+                        // matches.
+                        if (watchedDirectory.ExtensionFilter == null || filePath.EndsWith(watchedDirectory.ExtensionFilter))
                         {
                             return _noOpFileWatchingToken;
                         }
+                    }
+                }
 
                 var token = new FileWatchingToken();
 

src/VisualStudio/Core/Def/Implementation/ProjectSystem/MetadataReferences/FileWatchedPortableExecutableReferenceFactory.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel.Composition;
+using System.IO;
 using System.Linq;
 using System.Text;
 using System.Threading;
@@ -53,9 +54,15 @@ internal sealed class FileWatchedPortableExecutableReferenceFactory
         {
             _visualStudioWorkspace = visualStudioWorkspace;
 
-            // TODO: set this to watch the NuGet directory or the reference assemblies directory; since those change rarely and most references
-            // will come from them, we can avoid creating a bunch of explicit file watchers.
-            _fileReferenceChangeContext = fileChangeWatcherProvider.Watcher.CreateContext();
+            // We will do a single directory watch on the Reference Assemblies folder to avoid having to create separate file
+            // watches on individual .dlls that effectively never change.
+            var referenceAssembliesPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86), "Reference Assemblies", "Microsoft", "Framework");
+            var referenceAssemblies = new FileChangeWatcher.WatchedDirectory(referenceAssembliesPath, ".dll");
+
+            // TODO: set this to watch the NuGet directory as well; there's some concern that watching the entire directory
+            // might make restores take longer because we'll be watching changes that may not impact your project.
+
+            _fileReferenceChangeContext = fileChangeWatcherProvider.Watcher.CreateContext(referenceAssemblies);
             _fileReferenceChangeContext.FileChanged += FileReferenceChangeContext_FileChanged;
         }
 

src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs

@@ -8,6 +8,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics.CodeAnalysis;
+using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -16,9 +17,7 @@
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
-using Microsoft.VisualStudio.Debugger.Interop;
 using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;
-using Microsoft.VisualStudio.PlatformUI;
 using Roslyn.Utilities;
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
@@ -144,11 +143,14 @@ internal sealed class VisualStudioProject
             Language = language;
             _displayName = displayName;
 
-            if (filePath != null)
+            string? fileExtensionToWatch = language switch { LanguageNames.CSharp => ".cs", LanguageNames.VisualBasic => ".vb", _ => null };
+
+            if (filePath != null && fileExtensionToWatch != null)
             {
-                // TODO: use filePath to create a directory watcher. For now, there's perf hits due to the flood of events we'll need to sort out later.
-                // _documentFileChangeContext = _workspace.FileChangeWatcher.CreateContextForDirectory(Path.GetDirectoryName(filePath));
-                _documentFileChangeContext = workspace.FileChangeWatcher.CreateContext();
+                // Since we have a project directory, we'll just watch all the files under that path; that'll avoid extra overhead of
+                // having to add explicit file watches everywhere.
+                var projectDirectoryToWatch = new FileChangeWatcher.WatchedDirectory(Path.GetDirectoryName(filePath), fileExtensionToWatch);
+                _documentFileChangeContext = _workspace.FileChangeWatcher.CreateContext(projectDirectoryToWatch);
             }
             else
             {

src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/MockVsFileChangeEx.vb

@@ -14,10 +14,16 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
         Implements IVsAsyncFileChangeEx
 
         Private ReadOnly _lock As New Object
-        Private ReadOnly _watchedFiles As List(Of Tuple(Of UInteger, String, IVsFreeThreadedFileChangeEvents2)) = New List(Of Tuple(Of UInteger, String, IVsFreeThreadedFileChangeEvents2))
+        Private ReadOnly _watchedFiles As New List(Of WatchedEntity)
+        Private ReadOnly _watchedDirectories As New List(Of WatchedEntity)
         Private _nextCookie As UInteger = 0UI
 
         Public Function AdviseDirChange(pszDir As String, fWatchSubDir As Integer, pFCE As IVsFileChangeEvents, ByRef pvsCookie As UInteger) As Integer Implements IVsFileChangeEx.AdviseDirChange
+            If fWatchSubDir = 0 Then
+                Throw New NotImplementedException("Watching a single directory but not subdirectories is not implemented in this mock.")
+            End If
+
+            pvsCookie = AdviseDirectoryOrFileChange(_watchedDirectories, pszDir, pFCE)
             Return VSConstants.S_OK
         End Function
 
@@ -26,12 +32,20 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
                 Throw New NotImplementedException()
             End If
 
+            pvsCookie = AdviseDirectoryOrFileChange(_watchedFiles, pszMkDocument, pFCE)
+            Return VSConstants.S_OK
+        End Function
+
+        Private Function AdviseDirectoryOrFileChange(watchedList As List(Of WatchedEntity),
+                                                     pszMkDocument As String,
+                                                     pFCE As IVsFileChangeEvents) As UInteger
+
             SyncLock _lock
-                pvsCookie = _nextCookie
-                _watchedFiles.Add(Tuple.Create(pvsCookie, pszMkDocument, DirectCast(pFCE, IVsFreeThreadedFileChangeEvents2)))
+                Dim cookie = _nextCookie
+                watchedList.Add(New WatchedEntity(cookie, pszMkDocument, DirectCast(pFCE, IVsFreeThreadedFileChangeEvents2)))
                 _nextCookie += 1UI
 
-                Return VSConstants.S_OK
+                Return cookie
             End SyncLock
         End Function
 
@@ -44,33 +58,71 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
         End Function
 
         Public Function UnadviseDirChange(VSCOOKIE As UInteger) As Integer Implements IVsFileChangeEx.UnadviseDirChange
+            SyncLock _lock
+                _watchedDirectories.RemoveAll(Function(t) t.Cookie = VSCOOKIE)
+
                 Return VSConstants.S_OK
+            End SyncLock
         End Function
 
         Public Function UnadviseFileChange(VSCOOKIE As UInteger) As Integer Implements IVsFileChangeEx.UnadviseFileChange
             SyncLock _lock
-                Dim index = _watchedFiles.FindIndex(Function(t) t.Item1 = VSCOOKIE)
-                _watchedFiles.RemoveAt(index)
+                _watchedFiles.RemoveAll(Function(t) t.Cookie = VSCOOKIE)
 
                 Return VSConstants.S_OK
             End SyncLock
         End Function
 
         Public Sub FireUpdate(filename As String)
-            Dim subscription As Tuple(Of UInteger, String, IVsFreeThreadedFileChangeEvents2) = Nothing
+            Dim actionsToFire As List(Of Action) = New List(Of Action)()
 
             SyncLock _lock
-                subscription = _watchedFiles.First(Function(t) String.Equals(t.Item2, filename, StringComparison.OrdinalIgnoreCase))
-            End SyncLock
+                For Each watchedFile In _watchedFiles
+                    If String.Equals(watchedFile.Path, filename, StringComparison.OrdinalIgnoreCase) Then
+                        actionsToFire.Add(Sub()
+                                              watchedFile.Sink.FilesChanged(1, {watchedFile.Path}, {CType(_VSFILECHANGEFLAGS.VSFILECHG_Time, UInteger)})
+                                          End Sub)
+                    End If
+                Next
 
-            If subscription IsNot Nothing Then
+                For Each watchedDirectory In _watchedDirectories
+                    If FileNameMatchesFilter(filename, watchedDirectory) Then
+                        actionsToFire.Add(Sub()
+                                              watchedDirectory.Sink.DirectoryChangedEx2(watchedDirectory.Path, 1, {filename}, {CType(_VSFILECHANGEFLAGS.VSFILECHG_Time, UInteger)})
+                                          End Sub)
+                    End If
+                Next
+            End SyncLock
 
-                subscription.Item3.FilesChanged(1, {subscription.Item2}, {CType(_VSFILECHANGEFLAGS.VSFILECHG_Time, UInteger)})
+            If actionsToFire.Count > 0 Then
+                For Each actionToFire In actionsToFire
+                    actionToFire()
+                Next
             Else
-                Throw New InvalidOperationException("There is no subscription for file " + filename)
+                Throw New InvalidOperationException($"There is no subscription for file {filename}. Is the test authored correctly?")
             End If
         End Sub
 
+        Private Function FileNameMatchesFilter(filename As String, watchedDirectory As WatchedEntity) As Boolean
+            If Not filename.StartsWith(watchedDirectory.Path, StringComparison.OrdinalIgnoreCase) Then
+                Return False
+            End If
+
+            If watchedDirectory.ExtensionFilters Is Nothing Then
+                ' We have no extension filter, so good
+                Return True
+            End If
+
+            For Each extension In watchedDirectory.ExtensionFilters
+                If filename.EndsWith(extension) Then
+                    Return True
+                End If
+            Next
+
+            ' Didn't match the extension
+            Return False
+        End Function
+
         Public Function AdviseFileChangeAsync(filename As String, filter As _VSFILECHANGEFLAGS, sink As IVsFreeThreadedFileChangeEvents2, Optional cancellationToken As CancellationToken = Nothing) As Task(Of UInteger) Implements IVsAsyncFileChangeEx.AdviseFileChangeAsync
             Dim cookie As UInteger
             Marshal.ThrowExceptionForHR(AdviseFileChange(filename, CType(filter, UInteger), sink, cookie))
@@ -79,7 +131,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
 
         Public Function UnadviseFileChangeAsync(cookie As UInteger, Optional cancellationToken As CancellationToken = Nothing) As Task(Of String) Implements IVsAsyncFileChangeEx.UnadviseFileChangeAsync
             SyncLock _lock
-                Dim path = _watchedFiles.FirstOrDefault(Function(t) t.Item1 = cookie).Item2
+                Dim path = _watchedFiles.FirstOrDefault(Function(t) t.Cookie = cookie).Path
 
                 Marshal.ThrowExceptionForHR(UnadviseFileChange(cookie))
 
@@ -88,11 +140,19 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
         End Function
 
         Public Function AdviseDirChangeAsync(directory As String, watchSubdirectories As Boolean, sink As IVsFreeThreadedFileChangeEvents2, Optional cancellationToken As CancellationToken = Nothing) As Task(Of UInteger) Implements IVsAsyncFileChangeEx.AdviseDirChangeAsync
-            Throw New NotImplementedException()
+            Dim cookie As UInteger
+            Marshal.ThrowExceptionForHR(AdviseDirChange(directory, If(watchSubdirectories, 1, 0), sink, cookie))
+            Return Task.FromResult(cookie)
         End Function
 
         Public Function UnadviseDirChangeAsync(cookie As UInteger, Optional cancellationToken As CancellationToken = Nothing) As Task(Of String) Implements IVsAsyncFileChangeEx.UnadviseDirChangeAsync
-            Throw New NotImplementedException()
+            SyncLock _lock
+                Dim path = _watchedFiles.FirstOrDefault(Function(t) t.Cookie = cookie).Path
+
+                Marshal.ThrowExceptionForHR(UnadviseFileChange(cookie))
+
+                Return Task.FromResult(path)
+            End SyncLock
         End Function
 
         Public Function SyncFileAsync(filename As String, Optional cancellationToken As CancellationToken = Nothing) As Tasks.Task Implements IVsAsyncFileChangeEx.SyncFileAsync
@@ -107,6 +167,11 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
             Throw New NotImplementedException()
         End Function
 
+        Public Function FilterDirectoryChangesAsync(cookie As UInteger, extensions As String(), Optional cancellationToken As CancellationToken = Nothing) As Task
+            _watchedDirectories.FirstOrDefault(Function(t) t.Cookie = cookie).ExtensionFilters = extensions
+            Return Task.CompletedTask
+        End Function
+
         Public ReadOnly Property WatchedFileCount As Integer
             Get
                 SyncLock _lock
@@ -115,4 +180,17 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
             End Get
         End Property
     End Class
+
+    Friend Class WatchedEntity
+        Public ReadOnly Cookie As UInteger
+        Public ReadOnly Path As String
+        Public ReadOnly Sink As IVsFreeThreadedFileChangeEvents2
+        Public ExtensionFilters As String()
+
+        Public Sub New(cookie As UInteger, path As String, sink As IVsFreeThreadedFileChangeEvents2)
+            Me.Cookie = cookie
+            Me.Path = path
+            Me.Sink = sink
+        End Sub
+    End Class
 End Namespace