Fix inefficient interop in OSX FileSystemWatcher (dotnet/corefx#34715)

* Fix inefficient interop in OSX FileSystemWatcher

Avoid unnecessary array allocations in the FileSystemWatcher callback

* Add using for FSEventStreamEventFlags

Commit migrated from https://github.com/dotnet/corefx/commit/93386f5dd75e291d7c0dbbb450f5b96659b3d58f

diff --git a/src/libraries/Common/src/Interop/OSX/Interop.EventStream.cs b/src/libraries/Common/src/Interop/OSX/Interop.EventStream.cs
index 9dbd626..fb1ae4e 100644 (file)
--- a/src/libraries/Common/src/Interop/OSX/Interop.EventStream.cs
+++ b/src/libraries/Common/src/Interop/OSX/Interop.EventStream.cs
@@ -87,10 +87,8 @@ internal static partial class Interop
             IntPtr clientCallBackInfo,
             size_t numEvents,
             byte** eventPaths,
-            [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 2)]
-            FSEventStreamEventFlags[] eventFlags,
-            [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 2)]
-            FSEventStreamEventId[] eventIds);
+            FSEventStreamEventFlags* eventFlags,
+            FSEventStreamEventId* eventIds);
 
         /// <summary>
         /// Internal wrapper to create a new EventStream to listen to events from the core OS (such as File System events).

diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs
index 309898d..b608879 100644 (file)
--- a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs
+++ b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs
@@ -13,6 +13,7 @@ using CFStringRef = System.IntPtr;
 using FSEventStreamRef = System.IntPtr;
 using size_t = System.IntPtr;
 using FSEventStreamEventId = System.UInt64;
+using FSEventStreamEventFlags = Interop.EventStream.FSEventStreamEventFlags;
 using CFRunLoopRef = System.IntPtr;
 using Microsoft.Win32.SafeHandles;
 
@@ -79,38 +80,38 @@ namespace System.IO
 
         private CancellationTokenSource _cancellation;
 
-        private static Interop.EventStream.FSEventStreamEventFlags TranslateFlags(NotifyFilters flagsToTranslate)
+        private static FSEventStreamEventFlags TranslateFlags(NotifyFilters flagsToTranslate)
         {
-            Interop.EventStream.FSEventStreamEventFlags flags = 0;
+            FSEventStreamEventFlags flags = 0;
 
             // Always re-create the filter flags when start is called since they could have changed
             if ((flagsToTranslate & (NotifyFilters.Attributes | NotifyFilters.CreationTime | NotifyFilters.LastAccess | NotifyFilters.LastWrite | NotifyFilters.Size)) != 0)
             {
-                flags = Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemInodeMetaMod  |
-                        Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemFinderInfoMod |
-                        Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemModified      |
-                        Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemChangeOwner;
+                flags = FSEventStreamEventFlags.kFSEventStreamEventFlagItemInodeMetaMod  |
+                        FSEventStreamEventFlags.kFSEventStreamEventFlagItemFinderInfoMod |
+                        FSEventStreamEventFlags.kFSEventStreamEventFlagItemModified      |
+                        FSEventStreamEventFlags.kFSEventStreamEventFlagItemChangeOwner;
             }
             if ((flagsToTranslate & NotifyFilters.Security) != 0)
             {
-                flags |= Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemChangeOwner |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemXattrMod;
+                flags |= FSEventStreamEventFlags.kFSEventStreamEventFlagItemChangeOwner |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemXattrMod;
             }
             if ((flagsToTranslate & NotifyFilters.DirectoryName) != 0)
             {
-                flags |= Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsDir |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsSymlink |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed;
+                flags |= FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsDir |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsSymlink |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed;
             }
             if ((flagsToTranslate & NotifyFilters.FileName) != 0)
             {
-                flags |= Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsSymlink |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved |
-                         Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed;
+                flags |= FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsSymlink |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved |
+                         FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed;
             }
 
             return flags;
@@ -135,7 +136,7 @@ namespace System.IO
             private bool _includeChildren;
 
             // The bitmask of events that we want to send to the user
-            private Interop.EventStream.FSEventStreamEventFlags _filterFlags;
+            private FSEventStreamEventFlags _filterFlags;
 
             // The EventStream to listen for events on
             private SafeEventStreamHandle _eventStream;
@@ -157,7 +158,7 @@ namespace System.IO
                 FileSystemWatcher watcher,
                 string directory,
                 bool includeChildren,
-                Interop.EventStream.FSEventStreamEventFlags filter,
+                FSEventStreamEventFlags filter,
                 CancellationToken cancelToken)
             {
                 Debug.Assert(string.IsNullOrEmpty(directory) == false);
@@ -351,13 +352,9 @@ namespace System.IO
                 IntPtr clientCallBackInfo,
                 size_t numEvents,
                 byte** eventPaths,
-                [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 2)]
-                Interop.EventStream.FSEventStreamEventFlags[] eventFlags,
-                [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 2)]
-                FSEventStreamEventId[] eventIds)
+                FSEventStreamEventFlags* eventFlags,
+                FSEventStreamEventId* eventIds)
             {
-                Debug.Assert((eventPaths != null) && (numEvents.ToInt32() == eventFlags.Length) && (numEvents.ToInt32() == eventIds.Length));
-
                 // Try to get the actual watcher from our weak reference.  We maintain a weak reference most of the time
                 // so as to avoid a rooted cycle that would prevent our processing loop from ever ending
                 // if the watcher is dropped by the user without being disposed. If we can't get the watcher,
@@ -386,8 +383,8 @@ namespace System.IO
 
             private unsafe void ProcessEvents(int numEvents,
                 byte** eventPaths,
-                Interop.EventStream.FSEventStreamEventFlags[] eventFlags,
-                FSEventStreamEventId[] eventIds,
+                FSEventStreamEventFlags* eventFlags,
+                FSEventStreamEventId* eventIds,
                 FileSystemWatcher watcher)
             {
                 // Since renames come in pairs, when we find the first we need to search for the next one. Once we find it, we'll add it to this
@@ -396,7 +393,7 @@ namespace System.IO
                 Memory<char>[] events = new Memory<char>[numEvents];
                 ParseEvents();
 
-                for (long i = 0; i < numEvents; i++)
+                for (int i = 0; i < numEvents; i++)
                 {
                     ReadOnlySpan<char> path = events[i].Span;
                     Debug.Assert(path[path.Length - 1] != '/', "Trailing slashes on events is not supported");
@@ -446,14 +443,14 @@ namespace System.IO
                         if (((eventType & WatcherChangeTypes.Renamed) > 0))
                         {
                             // Find the rename that is paired to this rename, which should be the next rename in the list
-                            long pairedId = FindRenameChangePairedChange(i, eventFlags);
-                            if (pairedId == long.MinValue)
+                            int pairedId = FindRenameChangePairedChange(i, eventFlags, numEvents);
+                            if (pairedId == int.MinValue)
                             {
                                 // Getting here means we have a rename without a pair, meaning it should be a create for the 
                                 // move from unwatched folder to watcher folder scenario or a move from the watcher folder out.
                                 // Check if the item exists on disk to check which it is
                                 // Don't send a new notification if we already sent one for this event.
-                                if (DoesItemExist(path, IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile)))
+                                if (DoesItemExist(path, IsFlagSet(eventFlags[i], FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile)))
                                 {
                                     if ((eventType & WatcherChangeTypes.Created) == 0)
                                     {
@@ -519,13 +516,13 @@ namespace System.IO
             /// Compares the given event flags to the filter flags and returns which event (if any) corresponds
             /// to those flags.
             /// </summary>
-            private WatcherChangeTypes FilterEvents(Interop.EventStream.FSEventStreamEventFlags eventFlags)
+            private WatcherChangeTypes FilterEvents(FSEventStreamEventFlags eventFlags)
             {
-                const Interop.EventStream.FSEventStreamEventFlags changedFlags = Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemInodeMetaMod |
-                                                                                 Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemFinderInfoMod |
-                                                                                 Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemModified |
-                                                                                 Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemChangeOwner |
-                                                                                 Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemXattrMod;
+                const FSEventStreamEventFlags changedFlags = FSEventStreamEventFlags.kFSEventStreamEventFlagItemInodeMetaMod |
+                                                                                 FSEventStreamEventFlags.kFSEventStreamEventFlagItemFinderInfoMod |
+                                                                                 FSEventStreamEventFlags.kFSEventStreamEventFlagItemModified |
+                                                                                 FSEventStreamEventFlags.kFSEventStreamEventFlagItemChangeOwner |
+                                                                                 FSEventStreamEventFlags.kFSEventStreamEventFlagItemXattrMod;
                 WatcherChangeTypes eventType = 0;
                 // If any of the Changed flags are set in both Filter and Event then a Changed event has occurred.
                 if (((_filterFlags & changedFlags) & (eventFlags & changedFlags)) > 0)
@@ -534,25 +531,25 @@ namespace System.IO
                 }
 
                 // Notify created/deleted/renamed events if they pass through the filters
-                bool allowDirs = (_filterFlags & Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsDir) > 0;
-                bool allowFiles = (_filterFlags & Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile) > 0;
-                bool isDir = (eventFlags & Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsDir) > 0;
-                bool isFile = (eventFlags & Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile) > 0;
+                bool allowDirs = (_filterFlags & FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsDir) > 0;
+                bool allowFiles = (_filterFlags & FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile) > 0;
+                bool isDir = (eventFlags & FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsDir) > 0;
+                bool isFile = (eventFlags & FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsFile) > 0;
                 bool eventIsCorrectType = (isDir && allowDirs) || (isFile && allowFiles);
-                bool eventIsLink = (eventFlags & (Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsHardlink | Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsSymlink | Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsLastHardlink)) > 0;
+                bool eventIsLink = (eventFlags & (FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsHardlink | FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsSymlink | FSEventStreamEventFlags.kFSEventStreamEventFlagItemIsLastHardlink)) > 0;
 
                 if (eventIsCorrectType || ((allowDirs || allowFiles) && (eventIsLink)))
                 {
                     // Notify Created/Deleted/Renamed events.
-                    if (IsFlagSet(eventFlags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed))
+                    if (IsFlagSet(eventFlags, FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed))
                     {
                         eventType |= WatcherChangeTypes.Renamed;
                     }
-                    if (IsFlagSet(eventFlags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated))
+                    if (IsFlagSet(eventFlags, FSEventStreamEventFlags.kFSEventStreamEventFlagItemCreated))
                     {
                         eventType |= WatcherChangeTypes.Created;
                     }
-                    if (IsFlagSet(eventFlags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved))
+                    if (IsFlagSet(eventFlags, FSEventStreamEventFlags.kFSEventStreamEventFlagItemRemoved))
                     {
                         eventType |= WatcherChangeTypes.Deleted;
                     }
@@ -560,15 +557,15 @@ namespace System.IO
                 return eventType;
             }
 
-            private bool ShouldRescanOccur(Interop.EventStream.FSEventStreamEventFlags flags)
+            private bool ShouldRescanOccur(FSEventStreamEventFlags flags)
             {
                 // Check if any bit is set that signals that the caller should rescan
-                return (IsFlagSet(flags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagMustScanSubDirs) ||
-                        IsFlagSet(flags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagUserDropped)     ||
-                        IsFlagSet(flags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagKernelDropped)   ||
-                        IsFlagSet(flags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagRootChanged)     ||
-                        IsFlagSet(flags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagMount)           ||
-                        IsFlagSet(flags, Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagUnmount));
+                return (IsFlagSet(flags, FSEventStreamEventFlags.kFSEventStreamEventFlagMustScanSubDirs) ||
+                        IsFlagSet(flags, FSEventStreamEventFlags.kFSEventStreamEventFlagUserDropped)     ||
+                        IsFlagSet(flags, FSEventStreamEventFlags.kFSEventStreamEventFlagKernelDropped)   ||
+                        IsFlagSet(flags, FSEventStreamEventFlags.kFSEventStreamEventFlagRootChanged)     ||
+                        IsFlagSet(flags, FSEventStreamEventFlags.kFSEventStreamEventFlagMount)           ||
+                        IsFlagSet(flags, FSEventStreamEventFlags.kFSEventStreamEventFlagUnmount));
             }
 
             private bool CheckIfPathIsNested(ReadOnlySpan<char> eventPath)
@@ -579,24 +576,25 @@ namespace System.IO
                 return _includeChildren || _fullDirectory.AsSpan().StartsWith(System.IO.Path.GetDirectoryName(eventPath), StringComparison.OrdinalIgnoreCase);
             }
 
-            private long FindRenameChangePairedChange(
-                long currentIndex, 
-                Interop.EventStream.FSEventStreamEventFlags[] eventFlags)
+            private unsafe int FindRenameChangePairedChange(
+                int currentIndex, 
+                FSEventStreamEventFlags* eventFlags,
+                int numEvents)
             {
                 // Start at one past the current index and try to find the next Rename item, which should be the old path.
-                for (long i = currentIndex + 1; i < eventFlags.Length; i++)
+                for (int i = currentIndex + 1; i < numEvents; i++)
                 {
-                    if (IsFlagSet(eventFlags[i], Interop.EventStream.FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed))
+                    if (IsFlagSet(eventFlags[i], FSEventStreamEventFlags.kFSEventStreamEventFlagItemRenamed))
                     {
                         // We found match, stop looking
                         return i;
                     }
                 }
 
-                return long.MinValue;
+                return int.MinValue;
             }
 
-            private static bool IsFlagSet(Interop.EventStream.FSEventStreamEventFlags flags, Interop.EventStream.FSEventStreamEventFlags value)
+            private static bool IsFlagSet(FSEventStreamEventFlags flags, FSEventStreamEventFlags value)
             {
                 return (value & flags) == value;
             }