Skip to content

Commit

Permalink
Fix inefficient interop in OSX FileSystemWatcher (dotnet/corefx#34715)
Browse files Browse the repository at this point in the history 
* Fix inefficient interop in OSX FileSystemWatcher

Avoid unnecessary array allocations in the FileSystemWatcher callback

* Add using for FSEventStreamEventFlags


Commit migrated from dotnet/corefx@93386f5
  • Loading branch information
@jkotas @stephentoub
jkotas authored and stephentoub committed Jan 21, 2019
1 parent ba6b2d0 commit c600501
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 64 deletions.
6 changes: 2 additions & 4 deletions src/libraries/Common/src/Interop/OSX/Interop.EventStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,8 @@ internal unsafe delegate void FSEventStreamCallback(
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).
Expand Down
118 changes: 58 additions & 60 deletions src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
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;

Expand Down Expand Up @@ -79,38 +80,38 @@ private void StopRaisingEvents()

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;
Expand All @@ -135,7 +136,7 @@ private sealed class RunningInstance
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;
Expand All @@ -157,7 +158,7 @@ internal RunningInstance(
FileSystemWatcher watcher,
string directory,
bool includeChildren,
Interop.EventStream.FSEventStreamEventFlags filter,
FSEventStreamEventFlags filter,
CancellationToken cancelToken)
{
Debug.Assert(string.IsNullOrEmpty(directory) == false);
Expand Down Expand Up @@ -351,13 +352,9 @@ private unsafe void FileSystemEventCallback(
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,
Expand Down Expand Up @@ -386,8 +383,8 @@ private unsafe void FileSystemEventCallback(

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
Expand All @@ -396,7 +393,7 @@ private unsafe void ProcessEvents(int numEvents,
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");
Expand Down Expand Up @@ -446,14 +443,14 @@ private unsafe void ProcessEvents(int numEvents,
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)
{
Expand Down Expand Up @@ -519,13 +516,13 @@ void ParseEvents()
/// 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)
Expand All @@ -534,41 +531,41 @@ private WatcherChangeTypes FilterEvents(Interop.EventStream.FSEventStreamEventFl
}

// 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;
}
}
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)
Expand All @@ -579,24 +576,25 @@ private bool CheckIfPathIsNested(ReadOnlySpan<char> eventPath)
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;
}
Expand Down

0 comments on commit c600501

Please sign in to comment.