From f2791e9eb02af21f5e5a1a99c1324fd0c9f79f64 Mon Sep 17 00:00:00 2001
From: Jason Malinowski <jason.malinowski@microsoft.com>
Date: Fri, 9 Jun 2017 16:09:40 -0700
Subject: [PATCH] Reduce contention in FileChangeTracker

We were creating a bunch of tasks on the thread pool, all of which
ultimately contended on the single lock that exists inside of the
current implementation of IVsFileChangeEx. That's silly, so let's just
only try to do one of the background subscriptions at a time.
---
 .../ProjectSystem/FileChangeTracker.cs        | 23 +++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs
index 19d8413d0a5..4d436433913 100644
--- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs
+++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs
@@ -29,6 +29,23 @@ internal sealed class FileChangeTracker : IVsFileChangeEvents, IDisposable
 
         public event EventHandler UpdatedOnDisk;
 
+        /// <summary>
+        /// Operations on <see cref="IVsFileChangeEx"/> synchronize on a single lock within that service, so there's no point
+        /// in us trying to have multiple threads all trying to use it at the same time. When we queue a new background thread operation
+        /// we'll just do a continuation after the previous one. Any callers of <see cref="EnsureSubscription"/> will bypass that queue
+        /// and ensure it happens quickly.
+        /// </summary>
+        private static Task s_lastBackgroundTask = Task.CompletedTask;
+
+        /// <summary>
+        /// The object to use as a monitor guarding <see cref="s_lastBackgroundTask"/>. This lock is not strictly necessary, since we don't need
+        /// to ensure the background tasks happen entirely sequentially -- if we just removed the lock, and two subscriptions happened, we end up with
+        /// a 'branching' set of continuations, but that's fine since we're generally not running things in parallel. But it's easy to write,
+        /// and easy to delete if this lock has contention itself. Given we tend to call <see cref="StartFileChangeListeningAsync"/> on the UI
+        /// thread, I don't expect to see contention.
+        /// </summary>
+        private static readonly object s_lastBackgroundTaskGate = new object();
+
         public FileChangeTracker(IVsFileChangeEx fileChangeService, string filePath)
         {
             _fileChangeService = fileChangeService;
@@ -77,8 +94,10 @@ public void StartFileChangeListeningAsync()
                 return newCookie;
             }, LazyThreadSafetyMode.ExecutionAndPublication);
 
-            // file change service is free-threaded. start running it in background right away
-            Task.Run(() => _fileChangeCookie.Value, CancellationToken.None);
+            lock (s_lastBackgroundTaskGate)
+            {
+                s_lastBackgroundTask = s_lastBackgroundTask.ContinueWith(_ => _fileChangeCookie.Value, CancellationToken.None, TaskContinuationOptions.None, TaskScheduler.Default);
+            }
         }
 
         public void StopFileChangeListening()
-- 
GitLab