From d86751e655321f354bed7d3d6e1e315e4cff1114 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 8 Apr 2022 12:43:15 -0700 Subject: [PATCH 1/5] Delay starting the work to scan for todo-items --- ...AbstractTodoCommentsIncrementalAnalyzer.cs | 1 - .../InProcTodoCommentsIncrementalAnalyzer.cs | 4 +-- ...TodoCommentsIncrementalAnalyzerProvider.cs | 4 +-- .../TodoComments/TodoCommentsListener.cs | 24 ++++++++++-------- .../VisualStudioTodoCommentsService.cs | 25 +++++++++++-------- ...TodoCommentsIncrementalAnalyzerProvider.cs | 2 -- 6 files changed, 32 insertions(+), 28 deletions(-) rename src/Features/{Core/Portable => LanguageServer/Protocol/Features}/TodoComments/InProcTodoCommentsIncrementalAnalyzer.cs (86%) rename src/Features/{Core/Portable => LanguageServer/Protocol/Features}/TodoComments/InProcTodoCommentsIncrementalAnalyzerProvider.cs (86%) diff --git a/src/Features/Core/Portable/TodoComments/AbstractTodoCommentsIncrementalAnalyzer.cs b/src/Features/Core/Portable/TodoComments/AbstractTodoCommentsIncrementalAnalyzer.cs index 2895d456b6899..e2440ebf21fe5 100644 --- 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/src/Features/Core/Portable/TodoComments/InProcTodoCommentsIncrementalAnalyzer.cs b/src/Features/LanguageServer/Protocol/Features/TodoComments/InProcTodoCommentsIncrementalAnalyzer.cs similarity index 86% rename from src/Features/Core/Portable/TodoComments/InProcTodoCommentsIncrementalAnalyzer.cs rename to src/Features/LanguageServer/Protocol/Features/TodoComments/InProcTodoCommentsIncrementalAnalyzer.cs index 6d5d9f20a9f62..8b7e079edfdf5 100644 --- a/src/Features/Core/Portable/TodoComments/InProcTodoCommentsIncrementalAnalyzer.cs +++ b/src/Features/LanguageServer/Protocol/Features/TodoComments/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 data, CancellationToken cancellationToken) diff --git a/src/Features/Core/Portable/TodoComments/InProcTodoCommentsIncrementalAnalyzerProvider.cs b/src/Features/LanguageServer/Protocol/Features/TodoComments/InProcTodoCommentsIncrementalAnalyzerProvider.cs similarity index 86% rename from src/Features/Core/Portable/TodoComments/InProcTodoCommentsIncrementalAnalyzerProvider.cs rename to src/Features/LanguageServer/Protocol/Features/TodoComments/InProcTodoCommentsIncrementalAnalyzerProvider.cs index ce8d70ee76300..1aab3600ae490 100644 --- a/src/Features/Core/Portable/TodoComments/InProcTodoCommentsIncrementalAnalyzerProvider.cs +++ b/src/Features/LanguageServer/Protocol/Features/TodoComments/InProcTodoCommentsIncrementalAnalyzerProvider.cs @@ -13,9 +13,9 @@ namespace Microsoft.CodeAnalysis.TodoComments /// 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 index 2b30b5fafe2c2..c67e827cfeb02 100644 --- 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 /// /// Queue where we enqueue the information we get from OOP to process in batch in the future. /// - private readonly TaskCompletionSource> _workQueueSource = new(); + private readonly AsyncBatchingWorkQueue _workQueue; public TodoCommentsListener( IGlobalOptionService globalOptions, @@ -50,6 +50,12 @@ public TodoCommentsListener( _asyncListener = asynchronousOperationListenerProvider.GetListener(FeatureAttribute.TodoCommentList); _onTodoCommentsUpdated = onTodoCommentsUpdated; _disposalToken = disposalToken; + + _workQueue = new AsyncBatchingWorkQueue( + 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( - 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) /// /// Callback from the OOP service back into us. /// - public async ValueTask ReportTodoCommentDataAsync(DocumentId documentId, ImmutableArray infos, CancellationToken cancellationToken) + public ValueTask ReportTodoCommentDataAsync(DocumentId documentId, ImmutableArray 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/TodoComments/VisualStudioTodoCommentsService.cs b/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs index 6c4115790edf2..acc3068399578 100644 --- 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, - IDisposable + internal class VisualStudioTodoCommentsService : + ITodoListProvider, + IVsTypeScriptTodoCommentService, + IEventListener, + IDisposable { + private readonly IThreadingContext _threadingContext; private readonly VisualStudioWorkspaceImpl _workspace; private readonly EventListenerTracker _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> eventListeners) - : base(threadingContext) { + _threadingContext = threadingContext; _workspace = workspace; _eventListenerTracker = new EventListenerTracker(eventListeners, WellKnownEventListeners.TodoListProvider); @@ -77,15 +77,20 @@ public void Dispose() void IEventListener.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 workspae has actually loaded. We don't want to add to any + // startup costs by doing work too early. + var workspaceStatus = workspace.Services.GetRequiredService(); + 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/Workspaces/Remote/ServiceHub/Services/TodoCommentsDiscovery/RemoteTodoCommentsIncrementalAnalyzerProvider.cs b/src/Workspaces/Remote/ServiceHub/Services/TodoCommentsDiscovery/RemoteTodoCommentsIncrementalAnalyzerProvider.cs index 835589453b331..d68dc3891dcf5 100644 --- a/src/Workspaces/Remote/ServiceHub/Services/TodoCommentsDiscovery/RemoteTodoCommentsIncrementalAnalyzerProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/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 { From 260a72d3efe56eaf8191c2b03dc433980c90d1f1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 8 Apr 2022 12:57:07 -0700 Subject: [PATCH 2/5] move properties --- src/VisualStudio/Core/Def/TableDataSource/AbstractTable.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/VisualStudio/Core/Def/TableDataSource/AbstractTable.cs b/src/VisualStudio/Core/Def/TableDataSource/AbstractTable.cs index 80644f07b9b6c..80dd0e59bc155 100644 --- 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 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 Columns { get; } } } From e502dd9b1b63208f08c8f33277687aa4fcf60c23 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 8 Apr 2022 13:10:19 -0700 Subject: [PATCH 3/5] Cleanup --- .../Core/TodoComment/TodoItemsUpdatedArgs.cs | 4 ++-- .../Def/TableDataSource/VisualStudioBaseTodoListTable.cs | 9 +++------ .../Def/TodoComments/VisualStudioTodoCommentsService.cs | 7 +------ 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/EditorFeatures/Core/TodoComment/TodoItemsUpdatedArgs.cs b/src/EditorFeatures/Core/TodoComment/TodoItemsUpdatedArgs.cs index f3d88dc59636a..d5e64e9b33546 100644 --- 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 TodoItems { get; } public TodoItemsUpdatedArgs( - object id, Workspace workspace, Solution solution, ProjectId projectId, DocumentId documentId, ImmutableArray todoItems) - : base(id, workspace, projectId, documentId) + object id, Solution solution, DocumentId documentId, ImmutableArray todoItems) + : base(id, solution.Workspace, documentId.ProjectId, documentId) { Solution = solution; TodoItems = todoItems; diff --git a/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseTodoListTable.cs b/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseTodoListTable.cs index 3f5fa0edc2dd7..e1254cc022f8f 100644 --- 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 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 index acc3068399578..69942164ca96f 100644 --- a/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs +++ b/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs @@ -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); } From a0bb05a567404906bbb7e4d580dd812eea3698b4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 8 Apr 2022 13:11:18 -0700 Subject: [PATCH 4/5] Cleanup --- .../Core/Def/TodoComments/VisualStudioTodoCommentsService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs b/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs index 69942164ca96f..8cddda082e7d5 100644 --- a/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs +++ b/src/VisualStudio/Core/Def/TodoComments/VisualStudioTodoCommentsService.cs @@ -81,7 +81,7 @@ private async Task StartAsync(Workspace workspace) // fire-and-forget method and we want to make sure nothing leaks out. try { - // Don't bother doing anything until the workspae has actually loaded. We don't want to add to any + // 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(); await workspaceStatus.WaitUntilFullyLoadedAsync(_threadingContext.DisposalToken).ConfigureAwait(false); From afa7186c499b6eafa43c3bdbf4fa66d40daa753c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 8 Apr 2022 14:48:33 -0700 Subject: [PATCH 5/5] Fixup --- .../Core/Test/Diagnostics/TodoListTableDataSourceTests.vb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/VisualStudio/Core/Test/Diagnostics/TodoListTableDataSourceTests.vb b/src/VisualStudio/Core/Test/Diagnostics/TodoListTableDataSourceTests.vb index c4f826b90b42b..2d8973a21ec27 100644 --- 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