Fixes issue #6898 FileWatcherService locks UI Thread on linux. In #6900
Conversation
conjunction with the mono patch: mono/mono#12432 I think the mono patch is the real fix here but I optimized the UpdateWatchers.
| // First remove the watchers, so we don't spin too many threads. | ||
| foreach (var directory in toRemove) { | ||
| RemoveWatcher_NoLock (directory); | ||
| if (newWatchers.Count == 0) { |
There was a problem hiding this comment.
Do we ever hit this case as a fast path?
There was a problem hiding this comment.
That would mean that we cleared all the filewatchers, which I don't think will happen often. I would go for simplicity of code here and merge the loop of this and the one below.
newWatchers.Contains (directory) in the second loop below will exit early as the collection has no elements.
There was a problem hiding this comment.
It was in the original code :), therefore I decided not to remove it. But y doesn't make much difference here. The original code over evaluated the cost of Contains/foreach etc. and underestimated LINQ and memory usage. Usually the # of watchers to suspect here is low - way below 1 million where that foreach could be of any impact :).
removed that fast path case
There was a problem hiding this comment.
We can have up to 8 watchers, due to the algorithm.
|
Okay, I remember why I added the extra list. See test failures: Also, it looks like the line endings were changed. |
The toRemove list can easily be converted to a static field as well - only used in a static method inside a lock without a way of recursion.
| static HashSet<FilePath> newWatchers = new HashSet<FilePath>(); | ||
|
|
||
| static HashSet<FilePath> newWatchers = new HashSet<FilePath>(); | ||
| static List<FilePath> toRemove = new List<FilePath> (); |
conjunction with the mono patch:
mono/mono#12432
I think the mono patch is the real fix here but I optimized the
UpdateWatchers.