Merge pull request #60654 from CyrusNajmabadi/todoListWait

Delay starting the work to scan for todo-items
dotnet · Apr 10, 2022 · 3e091ab · 3e091ab
2 parents 48ef538 + afa7186
commit 3e091ab
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 48 deletions.
diff --git a/src/EditorFeatures/Core/TodoComment/TodoItemsUpdatedArgs.cs b/src/EditorFeatures/Core/TodoComment/TodoItemsUpdatedArgs.cs
@@ -21,8 +21,8 @@ internal sealed class TodoItemsUpdatedArgs : UpdatedEventArgs
         public ImmutableArray<TodoCommentData> TodoItems { get; }
 
         public TodoItemsUpdatedArgs(
-            object id, Workspace workspace, Solution solution, ProjectId projectId, DocumentId documentId, ImmutableArray<TodoCommentData> todoItems)
-            : base(id, workspace, projectId, documentId)
+            object id, Solution solution, DocumentId documentId, ImmutableArray<TodoCommentData> todoItems)
+            : base(id, solution.Workspace, documentId.ProjectId, documentId)
         {
             Solution = solution;
             TodoItems = todoItems;

diff --git a/src/Features/Core/Portable/TodoComments/AbstractTodoCommentsIncrementalAnalyzer.cs b/src/Features/Core/Portable/TodoComments/AbstractTodoCommentsIncrementalAnalyzer.cs
@@ -6,7 +6,6 @@
 using System.Collections.Immutable;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.SolutionCrawler;

diff --git a/.../InProcTodoCommentsIncrementalAnalyzer.cs → .../InProcTodoCommentsIncrementalAnalyzer.cs b/.../InProcTodoCommentsIncrementalAnalyzer.cs → .../InProcTodoCommentsIncrementalAnalyzer.cs
@@ -10,9 +10,9 @@ namespace Microsoft.CodeAnalysis.TodoComments
 {
     internal sealed class InProcTodoCommentsIncrementalAnalyzer : AbstractTodoCommentsIncrementalAnalyzer
     {
-        private readonly ITodoCommentsListener _listener;
+        private readonly TodoCommentsListener _listener;
 
-        public InProcTodoCommentsIncrementalAnalyzer(ITodoCommentsListener listener)
+        public InProcTodoCommentsIncrementalAnalyzer(TodoCommentsListener listener)
             => _listener = listener;
 
         protected override ValueTask ReportTodoCommentDataAsync(DocumentId documentId, ImmutableArray<TodoCommentData> data, CancellationToken cancellationToken)

diff --git a/...odoCommentsIncrementalAnalyzerProvider.cs → ...odoCommentsIncrementalAnalyzerProvider.cs b/...odoCommentsIncrementalAnalyzerProvider.cs → ...odoCommentsIncrementalAnalyzerProvider.cs
@@ -13,9 +13,9 @@ namespace Microsoft.CodeAnalysis.TodoComments
     /// </remarks>
     internal sealed class InProcTodoCommentsIncrementalAnalyzerProvider : IIncrementalAnalyzerProvider
     {
-        private readonly ITodoCommentsListener _listener;
+        private readonly TodoCommentsListener _listener;
 
-        public InProcTodoCommentsIncrementalAnalyzerProvider(ITodoCommentsListener listener)
+        public InProcTodoCommentsIncrementalAnalyzerProvider(TodoCommentsListener listener)
             => _listener = listener;
 
         public IIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace workspace)

diff --git a/src/Features/LanguageServer/Protocol/Features/TodoComments/TodoCommentsListener.cs b/src/Features/LanguageServer/Protocol/Features/TodoComments/TodoCommentsListener.cs
@@ -36,7 +36,7 @@ internal sealed class TodoCommentsListener : ITodoCommentsListener, IDisposable
         /// <summary>
         /// Queue where we enqueue the information we get from OOP to process in batch in the future.
         /// </summary>
-        private readonly TaskCompletionSource<AsyncBatchingWorkQueue<DocumentAndComments>> _workQueueSource = new();
+        private readonly AsyncBatchingWorkQueue<DocumentAndComments> _workQueue;
 
         public TodoCommentsListener(
             IGlobalOptionService globalOptions,
@@ -50,6 +50,12 @@ public TodoCommentsListener(
             _asyncListener = asynchronousOperationListenerProvider.GetListener(FeatureAttribute.TodoCommentList);
             _onTodoCommentsUpdated = onTodoCommentsUpdated;
             _disposalToken = disposalToken;
+
+            _workQueue = new AsyncBatchingWorkQueue<DocumentAndComments>(
+                TimeSpan.FromSeconds(1),
+                ProcessTodoCommentInfosAsync,
+                _asyncListener,
+                _disposalToken);
         }
 
         public void Dispose()
@@ -64,14 +70,10 @@ public void Dispose()
 
         public async ValueTask StartAsync()
         {
-            var cancellationToken = _disposalToken;
+            // Should only be started once.
+            Contract.ThrowIfTrue(_lazyConnection != null);
 
-            _workQueueSource.SetResult(
-                new AsyncBatchingWorkQueue<DocumentAndComments>(
-                TimeSpan.FromSeconds(1),
-                ProcessTodoCommentInfosAsync,
-                _asyncListener,
-                cancellationToken));
+            var cancellationToken = _disposalToken;
 
             var client = await RemoteHostClient.TryGetClientAsync(_services, cancellationToken).ConfigureAwait(false);
             if (client == null)
@@ -120,12 +122,12 @@ private void GlobalOptionChanged(object? sender, OptionChangedEventArgs e)
         /// <summary>
         /// Callback from the OOP service back into us.
         /// </summary>
-        public async ValueTask ReportTodoCommentDataAsync(DocumentId documentId, ImmutableArray<TodoCommentData> infos, CancellationToken cancellationToken)
+        public ValueTask ReportTodoCommentDataAsync(DocumentId documentId, ImmutableArray<TodoCommentData> infos, CancellationToken cancellationToken)
         {
             try
             {
-                var workQueue = await _workQueueSource.Task.ConfigureAwait(false);
-                workQueue.AddWork(new DocumentAndComments(documentId, infos));
+                _workQueue.AddWork(new DocumentAndComments(documentId, infos));
+                return ValueTaskFactory.CompletedTask;
             }
             catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken))
             {

diff --git a/src/VisualStudio/Core/Def/TableDataSource/AbstractTable.cs b/src/VisualStudio/Core/Def/TableDataSource/AbstractTable.cs
@@ -22,6 +22,8 @@ protected AbstractTable(Workspace workspace, ITableManagerProvider provider, str
         }
 
         protected Workspace Workspace { get; }
+        internal ITableManager TableManager { get; }
+        internal abstract ImmutableArray<string> Columns { get; }
 
         protected abstract void AddTableSourceIfNecessary(Solution solution);
         protected abstract void RemoveTableSourceIfNecessary(Solution solution);
@@ -88,9 +90,5 @@ protected void AddInitialTableSource(Solution solution, ITableDataSource source)
 
         protected void AddTableSource(ITableDataSource source)
             => this.TableManager.AddSource(source, Columns);
-
-        internal ITableManager TableManager { get; }
-
-        internal abstract ImmutableArray<string> Columns { get; }
     }
 }
diff --git a/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseTodoListTable.cs b/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseTodoListTable.cs
@@ -115,18 +115,15 @@ protected override object GetOrUpdateAggregationKey(TodoItemsUpdatedArgs data)
 
             private bool CheckAggregateKey(ImmutableArray<DocumentId> key, TodoItemsUpdatedArgs args)
             {
-                if (args.DocumentId == null || args.Solution == null)
-                    return true;
-
+                Contract.ThrowIfNull(args.Solution);
+                Contract.ThrowIfNull(args.DocumentId);
                 var documents = GetDocumentsWithSameFilePath(args.Solution, args.DocumentId);
                 return key == documents;
             }
 
             private object CreateAggregationKey(TodoItemsUpdatedArgs data)
             {
-                if (data.Solution == null)
-                    return GetItemKey(data);
-
+                Contract.ThrowIfNull(data.Solution);
                 return GetDocumentsWithSameFilePath(data.Solution, data.DocumentId);
             }
 

diff --git a/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs b/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs
@@ -26,13 +26,13 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.TodoComments
 {
     [Export(typeof(IVsTypeScriptTodoCommentService))]
     [ExportEventListener(WellKnownEventListeners.Workspace, WorkspaceKind.Host), Shared]
-    internal class VisualStudioTodoCommentsService
-        : ForegroundThreadAffinitizedObject,
-          ITodoListProvider,
-          IVsTypeScriptTodoCommentService,
-          IEventListener<object>,
-          IDisposable
+    internal class VisualStudioTodoCommentsService :
+        ITodoListProvider,
+        IVsTypeScriptTodoCommentService,
+        IEventListener<object>,
+        IDisposable
     {
+        private readonly IThreadingContext _threadingContext;
         private readonly VisualStudioWorkspaceImpl _workspace;
         private readonly EventListenerTracker<ITodoListProvider> _eventListenerTracker;
         private readonly TodoCommentsListener _listener;
@@ -42,13 +42,13 @@ internal class VisualStudioTodoCommentsService
         [ImportingConstructor]
         [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
         public VisualStudioTodoCommentsService(
+            IThreadingContext threadingContext,
             VisualStudioWorkspaceImpl workspace,
             IGlobalOptionService globalOptions,
-            IThreadingContext threadingContext,
             IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider,
             [ImportMany] IEnumerable<Lazy<IEventListener, EventListenerMetadata>> eventListeners)
-            : base(threadingContext)
         {
+            _threadingContext = threadingContext;
             _workspace = workspace;
             _eventListenerTracker = new EventListenerTracker<ITodoListProvider>(eventListeners, WellKnownEventListeners.TodoListProvider);
 
@@ -59,12 +59,7 @@ public VisualStudioTodoCommentsService(
                 onTodoCommentsUpdated: (documentId, oldComments, newComments) =>
                 {
                     if (TodoListUpdated != null && !oldComments.SequenceEqual(newComments))
-                    {
-                        TodoListUpdated?.Invoke(
-                            this, new TodoItemsUpdatedArgs(
-                                documentId, _workspace, _workspace.CurrentSolution,
-                                documentId.ProjectId, documentId, newComments));
-                    }
+                        TodoListUpdated?.Invoke(this, new TodoItemsUpdatedArgs(documentId, _workspace.CurrentSolution, documentId, newComments));
                 },
                 threadingContext.DisposalToken);
         }
@@ -77,15 +72,20 @@ public void Dispose()
         void IEventListener<object>.StartListening(Workspace workspace, object _)
         {
             if (workspace is VisualStudioWorkspace)
-                _ = StartAsync();
+                _ = StartAsync(workspace);
         }
 
-        private async Task StartAsync()
+        private async Task StartAsync(Workspace workspace)
         {
             // Have to catch all exceptions coming through here as this is called from a
             // fire-and-forget method and we want to make sure nothing leaks out.
             try
             {
+                // Don't bother doing anything until the workspace has actually loaded.  We don't want to add to any
+                // startup costs by doing work too early.
+                var workspaceStatus = workspace.Services.GetRequiredService<IWorkspaceStatusService>();
+                await workspaceStatus.WaitUntilFullyLoadedAsync(_threadingContext.DisposalToken).ConfigureAwait(false);
+
                 // Now that we've started, let the VS todo list know to start listening to us
                 _eventListenerTracker.EnsureEventListener(_workspace, this);
 

diff --git a/src/VisualStudio/Core/Test/Diagnostics/TodoListTableDataSourceTests.vb b/src/VisualStudio/Core/Test/Diagnostics/TodoListTableDataSourceTests.vb
@@ -406,13 +406,13 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics
 
                 For Each group In map
                     RaiseEvent TodoListUpdated(Me, New TodoItemsUpdatedArgs(
-                        Tuple.Create(Me, group.Key), workspace, workspace.CurrentSolution, group.Key.ProjectId, group.Key, group.ToImmutableArrayOrEmpty()))
+                        Me, workspace.CurrentSolution, group.Key, group.ToImmutableArrayOrEmpty()))
                 Next
             End Sub
 
             Public Sub RaiseClearTodoListUpdated(workspace As Microsoft.CodeAnalysis.Workspace, documentId As DocumentId)
                 RaiseEvent TodoListUpdated(Me, New TodoItemsUpdatedArgs(
-                    Tuple.Create(Me, documentId), workspace, workspace.CurrentSolution, documentId.ProjectId, documentId, ImmutableArray(Of TodoCommentData).Empty))
+                    Me, workspace.CurrentSolution, documentId, ImmutableArray(Of TodoCommentData).Empty))
             End Sub
         End Class
     End Class

diff --git a/...erviceHub/Services/TodoCommentsDiscovery/RemoteTodoCommentsIncrementalAnalyzerProvider.cs b/...erviceHub/Services/TodoCommentsDiscovery/RemoteTodoCommentsIncrementalAnalyzerProvider.cs
@@ -3,8 +3,6 @@
 // See the LICENSE file in the project root for more information.
 
 using Microsoft.CodeAnalysis.SolutionCrawler;
-using Microsoft.CodeAnalysis.TodoComments;
-using Microsoft.ServiceHub.Framework;
 
 namespace Microsoft.CodeAnalysis.Remote
 {