From 7a035feaacee7969f9574eb3ea480747d0a3316e Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 7 Jun 2021 15:56:14 -0700 Subject: [PATCH 01/19] Address prototype comments in inheritdoc --- .../Core/Portable/Shared/Extensions/ISymbolExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs index 85ea4c5dc0d83..d37f7db440246 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs @@ -487,7 +487,8 @@ private static XNode[] RewriteMany(ISymbol symbol, HashSet? visitedSymb { if (typeSymbol.TypeKind == TypeKind.Class) { - // prototype(inheritdoc): when does base class take precedence over interface? + // Classes use the base type as the default inheritance candidate. A different target (e.g. an + // interface) can be provided via the 'path' attribute. return typeSymbol.BaseType; } else if (typeSymbol.TypeKind == TypeKind.Interface) From 4f4fd5c5c506c3afd673723235b95a77cbff0600 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 14 Jun 2021 08:19:17 -0700 Subject: [PATCH 02/19] Remove unnecessary nested inheritdoc rewrite The input to the nested rewrite already had inheritdoc elements rewritten, so the only elements remaining are ones which failed to resolve an inheritance target or 'cref' attribute. For the former case, preservation of the inheritdoc element would produce an invalid rewrite if nested rewriting were performed. --- .../QuickInfo/SemanticQuickInfoSourceTests.cs | 40 +++++++++++++++++++ .../Shared/Extensions/ISymbolExtensions.cs | 22 +--------- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs b/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs index 76b55b3a69915..48c5183661dcd 100644 --- a/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs +++ b/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs @@ -6853,6 +6853,46 @@ public async Task TestInheritdocInlineSummary() /// Remarks documentation void M(int x) { } +/// +void $$M(int x, int y) { }"; + + await TestInClassAsync(markup, + MainDescription("void C.M(int x, int y)"), + Documentation("Summary documentation")); + } + + [Fact, Trait(Traits.Feature, Traits.Features.QuickInfo)] + public async Task TestInheritdocTwoLevels1() + { + var markup = +@" +/// Summary documentation +/// Remarks documentation +void M() { } + +/// +void M(int x) { } + +/// +void $$M(int x, int y) { }"; + + await TestInClassAsync(markup, + MainDescription("void C.M(int x, int y)"), + Documentation("Summary documentation")); + } + + [Fact, Trait(Traits.Feature, Traits.Features.QuickInfo)] + public async Task TestInheritdocTwoLevels2() + { + var markup = +@" +/// Summary documentation +/// Remarks documentation +void M() { } + +/// +void M(int x) { } + /// void $$M(int x, int y) { }"; diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs index d37f7db440246..fb0ef16010a09 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs @@ -425,27 +425,7 @@ private static XNode[] RewriteMany(ISymbol symbol, HashSet? visitedSymb } var loadedElements = TrySelectNodes(document, xpathValue); - if (loadedElements is null) - { - return Array.Empty(); - } - - if (loadedElements?.Length > 0) - { - // change the current XML file path for nodes contained in the document: - // prototype(inheritdoc): what should the file path be? - var result = RewriteMany(symbol, visitedSymbols, compilation, loadedElements, cancellationToken); - - // The elements could be rewritten away if they are includes that refer to invalid - // (but existing and accessible) XML files. If this occurs, behave as if we - // had failed to find any XPath results (as in Dev11). - if (result.Length > 0) - { - return result; - } - } - - return null; + return loadedElements ?? Array.Empty(); } catch (XmlException) { From a828e1861aaffa39c69493b465281f1f5f38e5f1 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 14 Jun 2021 08:19:33 -0700 Subject: [PATCH 03/19] Report telemetry for XML rewrite failures --- .../Core/Portable/Shared/Extensions/ISymbolExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs index fb0ef16010a09..46a8ab030a6c6 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs @@ -15,6 +15,7 @@ using System.Xml.Linq; using System.Xml.XPath; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Shared.Utilities; using Roslyn.Utilities; @@ -269,7 +270,7 @@ private static DocumentationComment GetDocumentationComment(ISymbol symbol, Hash element.ReplaceNodes(RewriteMany(symbol, visitedSymbols, compilation, element.Nodes().ToArray(), cancellationToken)); xmlText = element.ToString(SaveOptions.DisableFormatting); } - catch + catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e, cancellationToken)) { } } From 3ca27cd51934d0a48339f1e508dc9afa2281bccd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 27 Jun 2021 13:18:06 -0700 Subject: [PATCH 04/19] Full simplification --- .../NavigationBar/NavigationBarController.cs | 196 ++++++---------- ...avigationBarController_ModelComputation.cs | 210 ++++-------------- .../NavigationBar/NavigationBarModel.cs | 9 +- 3 files changed, 119 insertions(+), 296 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs index a645f439539ea..7869b82232dd4 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs @@ -3,13 +3,16 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Editor.Implementation.Classification; using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Editor.Shared.Tagging; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; +using Microsoft.CodeAnalysis.Editor.Tagging; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -36,12 +39,16 @@ internal partial class NavigationBarController : ForegroundThreadAffinitizedObje private readonly IAsynchronousOperationListener _asyncListener; private bool _disconnected = false; - private Workspace? _workspace; /// - /// Latest model and selected items produced once completes and - /// presents the single item to the view. These can then be read in when the dropdown is expanded and we want - /// to show all items. + /// The last fully computed model. + /// + private NavigationBarModel _model_OnlyAccessOnUIThread; + + /// + /// Latest model and selected items produced once completes and presents the + /// single item to the view. These can then be read in when the dropdown is expanded and we want to show all + /// items. /// private (NavigationBarModel model, NavigationBarSelectedTypeAndMember selectedInfo) _latestModelAndSelectedInfo_OnlyAccessOnUIThread; @@ -51,6 +58,24 @@ internal partial class NavigationBarController : ForegroundThreadAffinitizedObje /// private (ImmutableArray projectItems, NavigationBarProjectItem? selectedProjectItem, NavigationBarModel model, NavigationBarSelectedTypeAndMember selectedInfo) _lastPresentedInfo; + private readonly ITaggerEventSource _eventSource; + + private readonly CancellationTokenSource _cancellationTokenSource = new(); + + /// + /// Queue to batch up work to do to compute the current model. Used so we can batch up a lot of events and only + /// compute the model once for every batch. The bool type parameter isn't used, but is provided as this + /// type is generic. + /// + private readonly AsyncBatchingWorkQueue _computeModelQueue; + + /// + /// Queue to batch up work to do to determine the selected item. Used so we can batch up a lot of events and + /// only compute the selected item once for every batch. The bool type parameter isn't used, but is + /// provided as this type is generic. + /// + private readonly AsyncBatchingWorkQueue _selectItemQueue; + public NavigationBarController( IThreadingContext threadingContext, INavigationBarPresenter presenter, @@ -64,86 +89,47 @@ public NavigationBarController( _uiThreadOperationExecutor = uiThreadOperationExecutor; _asyncListener = asyncListener; + _computeModelQueue = new AsyncBatchingWorkQueue( + TimeSpan.FromMilliseconds(TaggerConstants.ShortDelay), + ComputeModelAndSelectItemAsync, + EqualityComparer.Default, + asyncListener, + _cancellationTokenSource.Token); + + _selectItemQueue = new AsyncBatchingWorkQueue( + TimeSpan.FromMilliseconds(TaggerConstants.NearImmediateDelay), + SelectItemAsync, + EqualityComparer.Default, + asyncListener, + _cancellationTokenSource.Token); + presenter.CaretMoved += OnCaretMoved; presenter.ViewFocused += OnViewFocused; presenter.DropDownFocused += OnDropDownFocused; presenter.ItemSelected += OnItemSelected; - subjectBuffer.PostChanged += OnSubjectBufferPostChanged; - // Initialize the tasks to be an empty model so we never have to deal with a null case. _latestModelAndSelectedInfo_OnlyAccessOnUIThread.model = new( ImmutableArray.Empty, - semanticVersionStamp: default, itemService: null!); _latestModelAndSelectedInfo_OnlyAccessOnUIThread.selectedInfo = new(typeItem: null, memberItem: null); - - _modelTask = Task.FromResult(_latestModelAndSelectedInfo_OnlyAccessOnUIThread.model); - } - - public void SetWorkspace(Workspace? newWorkspace) - { - DisconnectFromWorkspace(); - - if (newWorkspace != null) - { - ConnectToWorkspace(newWorkspace); - } - } - - private void ConnectToWorkspace(Workspace workspace) - { - // If we disconnected before the workspace ever connected, just disregard - if (_disconnected) - { - return; - } - - _workspace = workspace; - _workspace.WorkspaceChanged += this.OnWorkspaceChanged; - _workspace.DocumentActiveContextChanged += this.OnDocumentActiveContextChanged; - - if (IsForeground()) - { - ConnectToNewWorkspace(); - } - else - { - var asyncToken = _asyncListener.BeginAsyncOperation(nameof(ConnectToWorkspace)); - Task.Run(async () => - { - await ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(); - - ConnectToNewWorkspace(); - }).CompletesAsyncOperation(asyncToken); - } - - return; - - void ConnectToNewWorkspace() - { - // For the first time you open the file, we'll start immediately - StartModelUpdateAndSelectedItemUpdateTasks(modelUpdateDelay: 0); - } - } - - private void DisconnectFromWorkspace() - { - if (_workspace != null) - { - _workspace.DocumentActiveContextChanged -= this.OnDocumentActiveContextChanged; - _workspace.WorkspaceChanged -= this.OnWorkspaceChanged; - _workspace = null; - } + _model_OnlyAccessOnUIThread = _latestModelAndSelectedInfo_OnlyAccessOnUIThread.model; + + _eventSource = new CompilationAvailableTaggerEventSource( + subjectBuffer, + asyncListener, + TaggerEventSources.OnTextChanged(subjectBuffer), + TaggerEventSources.OnDocumentActiveContextChanged(subjectBuffer), + TaggerEventSources.OnWorkspaceRegistrationChanged(subjectBuffer), + TaggerEventSources.OnWorkspaceChanged(subjectBuffer, asyncListener)); + _eventSource.Changed += OnEventSourceChanged; + _eventSource.Connect(); } public void Disconnect() { AssertIsForeground(); - DisconnectFromWorkspace(); - - _subjectBuffer.PostChanged -= OnSubjectBufferPostChanged; _presenter.CaretMoved -= OnCaretMoved; _presenter.ViewFocused -= OnViewFocused; @@ -153,81 +139,45 @@ public void Disconnect() _presenter.Disconnect(); + _eventSource.Changed -= OnEventSourceChanged; + _eventSource.Disconnect(); + _disconnected = true; // Cancel off any remaining background work - _modelTaskCancellationSource.Cancel(); - _selectedItemInfoTaskCancellationSource.Cancel(); + _cancellationTokenSource.Cancel(); } - private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs args) + private void OnEventSourceChanged(object? sender, TaggerEventArgs e) { - // We're getting an event for a workspace we already disconnected from - if (args.NewSolution.Workspace != _workspace) - { - return; - } - - // If the displayed project is being renamed, retrigger the update - if (args.Kind == WorkspaceChangeKind.ProjectChanged && args.ProjectId != null) - { - var oldProject = args.OldSolution.GetRequiredProject(args.ProjectId); - var newProject = args.NewSolution.GetRequiredProject(args.ProjectId); - - if (oldProject.Name != newProject.Name) - { - var currentContextDocumentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); - - if (currentContextDocumentId != null && currentContextDocumentId.ProjectId == args.ProjectId) - { - StartModelUpdateAndSelectedItemUpdateTasks(modelUpdateDelay: 0); - } - } - } - - if (args.Kind == WorkspaceChangeKind.DocumentChanged && - args.OldSolution == args.NewSolution) - { - var currentContextDocumentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); - if (currentContextDocumentId != null && currentContextDocumentId == args.DocumentId) - { - // The context has changed, so update everything. - StartModelUpdateAndSelectedItemUpdateTasks(modelUpdateDelay: 0); - } - } + StartModelUpdateAndSelectedItemUpdateTasks(); } - private void OnDocumentActiveContextChanged(object? sender, DocumentActiveContextChangedEventArgs args) + public void SetWorkspace(Workspace? newWorkspace) { - AssertIsForeground(); - if (args.Solution.Workspace != _workspace) - return; - - var currentContextDocumentId = _workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); - if (args.NewActiveContextDocumentId == currentContextDocumentId || - args.OldActiveContextDocumentId == currentContextDocumentId) - { - // if the active context changed, recompute the types/member as they may be changed as well. - StartModelUpdateAndSelectedItemUpdateTasks(modelUpdateDelay: 0); - } + StartModelUpdateAndSelectedItemUpdateTasks(); } - private void OnSubjectBufferPostChanged(object? sender, EventArgs e) + private void StartModelUpdateAndSelectedItemUpdateTasks() { - AssertIsForeground(); - StartModelUpdateAndSelectedItemUpdateTasks(modelUpdateDelay: TaggerConstants.MediumDelay); + // If we disconnected already, just disregard + if (_disconnected) + return; + + // 'true' value is unused. this just signals to the queue that we have work to do. + _computeModelQueue.AddWork(true); } private void OnCaretMoved(object? sender, EventArgs e) { AssertIsForeground(); - StartSelectedItemUpdateTask(delay: TaggerConstants.NearImmediateDelay); + StartSelectedItemUpdateTask(); } private void OnViewFocused(object? sender, EventArgs e) { AssertIsForeground(); - StartSelectedItemUpdateTask(delay: TaggerConstants.ShortDelay); + StartSelectedItemUpdateTask(); } private void OnDropDownFocused(object? sender, EventArgs e) @@ -393,9 +343,7 @@ private async Task ProcessItemSelectionAsync(NavigationBarItem item, Cancellatio } // Now that the edit has been done, refresh to make sure everything is up-to-date. - // Have to make sure we come back to the main thread for this. - AssertIsForeground(); - StartModelUpdateAndSelectedItemUpdateTasks(modelUpdateDelay: 0); + StartModelUpdateAndSelectedItemUpdateTasks(); } } } diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs index cc6ddd26ee71f..7a0c0d882a853 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs @@ -4,17 +4,14 @@ #nullable disable -using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Editor.Shared.Extensions; -using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Threading; @@ -23,155 +20,90 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.NavigationBar { internal partial class NavigationBarController { - /// - /// The computation of the last model. - /// - private Task _modelTask; - - private CancellationTokenSource _modelTaskCancellationSource = new(); - private CancellationTokenSource _selectedItemInfoTaskCancellationSource = new(); - /// /// Starts a new task to compute the model based on the current text. /// - private void StartModelUpdateAndSelectedItemUpdateTasks(int modelUpdateDelay) + private async Task ComputeModelAndSelectItemAsync(ImmutableArray unused, CancellationToken cancellationToken) { - AssertIsForeground(); - + // Jump back to the UI thread to determine what snapshot the user is processing. + await this.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); var textSnapshot = _subjectBuffer.CurrentSnapshot; - // Cancel off any existing work - _modelTaskCancellationSource.Cancel(); + // Ensure we switch to the threadpool before calling GetDocumentWithFrozenPartialSemantics. It ensures + // that any IO that performs is not potentially on the UI thread. + await TaskScheduler.Default; - _modelTaskCancellationSource = new CancellationTokenSource(); - var cancellationToken = _modelTaskCancellationSource.Token; + var model = await ComputeModelAsync(textSnapshot, cancellationToken).ConfigureAwait(false); - // Enqueue a new computation for the model - var asyncToken = _asyncListener.BeginAsyncOperation(GetType().Name + ".StartModelUpdateTask"); - _modelTask = ComputeModelAfterDelayAsync(_modelTask, textSnapshot, modelUpdateDelay, cancellationToken); - _modelTask.CompletesAsyncOperation(asyncToken); + await this.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); - StartSelectedItemUpdateTask(delay: 0); - } + _model_OnlyAccessOnUIThread = model; - private static async Task ComputeModelAfterDelayAsync( - Task modelTask, ITextSnapshot textSnapshot, int modelUpdateDelay, CancellationToken cancellationToken) - { - var previousModel = await modelTask.ConfigureAwait(false); - if (!cancellationToken.IsCancellationRequested) - { - try - { - await Task.Delay(modelUpdateDelay, cancellationToken).ConfigureAwait(false); - return await ComputeModelAsync(previousModel, textSnapshot, cancellationToken).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - } - catch (Exception e) when (FatalError.ReportAndCatch(e)) - { - } - } + // Now, enqueue work to select the right item in this new model. + StartSelectedItemUpdateTask(); - // If we canceled, then just return along whatever we have computed so far. Note: this means the - // _modelTask task will never enter the canceled state. It always represents the last successfully - // computed model. - return previousModel; - } + return; - /// - /// Computes a model for the given snapshot. - /// - private static async Task ComputeModelAsync( - NavigationBarModel lastCompletedModel, ITextSnapshot snapshot, CancellationToken cancellationToken) - { - // Ensure we switch to the threadpool before calling GetDocumentWithFrozenPartialSemantics. It ensures - // that any IO that performs is not potentially on the UI thread. - await TaskScheduler.Default; - - // When computing items just get the partial semantics workspace. This will ensure we can get data for this - // file, and hopefully have enough loaded to get data for other files in the case of partial types. In the - // event the other files aren't available, then partial-type information won't be correct. That's ok though - // as this is just something that happens during solution load and will pass once that is over. By using - // partial semantics, we can ensure we don't spend an inordinate amount of time computing and using full - // compilation data (like skeleton assemblies). - var document = snapshot.AsText().GetDocumentWithFrozenPartialSemantics(cancellationToken); - if (document == null) - return null; - - var languageService = document.GetLanguageService(); - if (languageService != null) + static async Task ComputeModelAsync(ITextSnapshot textSnapshot, CancellationToken cancellationToken) { - // check whether we can re-use lastCompletedModel. otherwise, update lastCompletedModel here. - // the model should be only updated here - if (lastCompletedModel != null) + // When computing items just get the partial semantics workspace. This will ensure we can get data for this + // file, and hopefully have enough loaded to get data for other files in the case of partial types. In the + // event the other files aren't available, then partial-type information won't be correct. That's ok though + // as this is just something that happens during solution load and will pass once that is over. By using + // partial semantics, we can ensure we don't spend an inordinate amount of time computing and using full + // compilation data (like skeleton assemblies). + var document = textSnapshot.AsText().GetDocumentWithFrozenPartialSemantics(cancellationToken); + if (document == null) + return null; + + var itemService = document.GetLanguageService(); + if (itemService != null) { - var semanticVersion = await document.Project.GetDependentSemanticVersionAsync(CancellationToken.None).ConfigureAwait(false); - if (lastCompletedModel.SemanticVersionStamp == semanticVersion && SpanStillValid(lastCompletedModel, snapshot, cancellationToken)) + using (Logger.LogBlock(FunctionId.NavigationBar_ComputeModelAsync, cancellationToken)) { - // it looks like we can re-use previous model - return lastCompletedModel; + var items = await itemService.GetItemsAsync(document, textSnapshot, cancellationToken).ConfigureAwait(false); + return new NavigationBarModel(items, itemService); } } - using (Logger.LogBlock(FunctionId.NavigationBar_ComputeModelAsync, cancellationToken)) - { - var items = await languageService.GetItemsAsync(document, snapshot, cancellationToken).ConfigureAwait(false); - var version = await document.Project.GetDependentSemanticVersionAsync(cancellationToken).ConfigureAwait(false); - return new NavigationBarModel(items, version, languageService); - } + return new NavigationBarModel(ImmutableArray.Empty, itemService: null); } - - return new NavigationBarModel(ImmutableArray.Empty, new VersionStamp(), null); } /// /// Starts a new task to compute what item should be selected. /// - private void StartSelectedItemUpdateTask(int delay) + private void StartSelectedItemUpdateTask() { - AssertIsForeground(); - - var currentView = _presenter.TryGetCurrentView(); - var subjectBufferCaretPosition = currentView?.GetCaretPoint(_subjectBuffer); - if (!subjectBufferCaretPosition.HasValue) - return; - - // Cancel off any existing work - _selectedItemInfoTaskCancellationSource.Cancel(); - _selectedItemInfoTaskCancellationSource = new CancellationTokenSource(); - var cancellationToken = _selectedItemInfoTaskCancellationSource.Token; - - var asyncToken = _asyncListener.BeginAsyncOperation(GetType().Name + ".StartSelectedItemUpdateTask"); - var selectedItemInfoTask = DetermineSelectedItemInfoAsync(_modelTask, delay, subjectBufferCaretPosition.Value, cancellationToken); - selectedItemInfoTask.CompletesAsyncOperation(asyncToken); + // 'true' value is unused. this just signals to the queue that we have work to do. + _selectItemQueue.AddWork(true); } - private async Task DetermineSelectedItemInfoAsync( - Task lastModelTask, - int delay, - SnapshotPoint caretPosition, - CancellationToken cancellationToken) + private async Task SelectItemAsync(ImmutableArray unused, CancellationToken cancellationToken) { - // First wait the delay before doing any other work. That way if we get canceled due to other events (like - // the user moving around), we don't end up doing anything, and the next task can take over. - await Task.Delay(delay, cancellationToken).ConfigureAwait(false); + // Switch to the UI so we can determine where the user is. + await this.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); - var lastModel = await lastModelTask.ConfigureAwait(false); - if (cancellationToken.IsCancellationRequested) + var currentView = _presenter.TryGetCurrentView(); + var caretPosition = currentView?.GetCaretPoint(_subjectBuffer); + if (!caretPosition.HasValue) return; - var currentSelectedItem = ComputeSelectedTypeAndMember(lastModel, caretPosition, cancellationToken); + // Grab the last computed model while there. + var model = _model_OnlyAccessOnUIThread; - await ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); + // Jump back to the BG to do any expensive work walking the entire model + await TaskScheduler.Default; + var currentSelectedItem = ComputeSelectedTypeAndMember(model, caretPosition.Value, cancellationToken); - AssertIsForeground(); + // Finally, switch back to the UI to update our state and UI. + await ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); // Update the UI to show *just* the type/member that was selected. We don't need it to know about all items // as the user can only see one at a time as they're editing in a document. However, once we've done this, // store the full list of items as well so that if the user expands the dropdown, we can take all those // values and shove them in so it appears as if the lists were always fully realized. - _latestModelAndSelectedInfo_OnlyAccessOnUIThread = (lastModel, currentSelectedItem); + _latestModelAndSelectedInfo_OnlyAccessOnUIThread = (model, currentSelectedItem); PushSelectedItemsToPresenter(currentSelectedItem); } @@ -247,55 +179,5 @@ private static (T item, bool gray) GetMatchingItem(IEnumerable items, Snap return (itemToGray, gray: itemToGray != null); } } - - private static bool SpanStillValid(NavigationBarModel model, ITextSnapshot snapshot, CancellationToken cancellationToken) - { - // even if semantic version is same, portion of text could have been copied & pasted with - // exact same top level content. - // go through spans to see whether this happened. - // - // paying cost of moving spans forward shouldn't be matter since we need to pay that - // price soon or later to figure out selected item. - foreach (var type in model.Types) - { - if (!SpansStillValid(type, snapshot)) - return false; - - foreach (var member in type.ChildItems) - { - cancellationToken.ThrowIfCancellationRequested(); - - if (!SpansStillValid(member, snapshot)) - return false; - } - } - - return true; - } - - private static bool SpansStillValid(NavigationBarItem item, ITextSnapshot snapshot) - { - if (item.NavigationTrackingSpan != null) - { - var currentSpan = item.NavigationTrackingSpan.GetSpan(snapshot); - if (currentSpan.IsEmpty) - return false; - } - - foreach (var span in item.TrackingSpans) - { - var currentSpan = span.GetSpan(snapshot); - if (currentSpan.IsEmpty) - return false; - } - - foreach (var childItem in item.ChildItems) - { - if (!SpansStillValid(childItem, snapshot)) - return false; - } - - return true; - } } } diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarModel.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarModel.cs index 99faf5fad32cd..3e4ede44c6f1b 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarModel.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarModel.cs @@ -10,20 +10,13 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.NavigationBar internal sealed class NavigationBarModel { public ImmutableArray Types { get; } - - /// - /// The VersionStamp of the project when this model was computed. - /// - public VersionStamp SemanticVersionStamp { get; } - public INavigationBarItemService ItemService { get; } - public NavigationBarModel(ImmutableArray types, VersionStamp semanticVersionStamp, INavigationBarItemService itemService) + public NavigationBarModel(ImmutableArray types, INavigationBarItemService itemService) { Contract.ThrowIfNull(types); this.Types = types; - this.SemanticVersionStamp = semanticVersionStamp; this.ItemService = itemService; } } From 0bf4294da5aa621d177a485bc2218a2a74a6b29e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 09:15:06 -0700 Subject: [PATCH 05/19] Work on latest model --- .../NavigationBar/NavigationBarController_ModelComputation.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs index 7d48c82105eab..daf53af1493a6 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs @@ -91,11 +91,13 @@ private async Task SelectItemAsync(ImmutableArray unused, CancellationToke if (!caretPosition.HasValue) return; - // Grab the last computed model while there. + // Ensure the latest model is computed and grab it while on the UI thread. + await _computeModelQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(true); var model = _model_OnlyAccessOnUIThread; // Jump back to the BG to do any expensive work walking the entire model await TaskScheduler.Default; + var currentSelectedItem = ComputeSelectedTypeAndMember(model, caretPosition.Value, cancellationToken); // Finally, switch back to the UI to update our state and UI. From ce771b9f0ca77a44b755e95c9e53b843230d77a0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 10:16:24 -0700 Subject: [PATCH 06/19] Provide helpful generic overloads of AsyncBatchingWorkQueue --- ...ventSources.WorkspaceChangedEventSource.cs | 8 +-- .../Workspace/SourceGeneratedFileManager.cs | 6 +-- .../Shared/Utilities/AsyncBatchingDelay.cs | 50 ------------------- .../Utilities/AsyncBatchingWorkQueue`0.cs | 43 ++++++++++++++++ .../Utilities/AsyncBatchingWorkQueue`1.cs | 49 ++++++++++++++++++ ...rkQueue.cs => AsyncBatchingWorkQueue`2.cs} | 34 ++++++------- .../SQLite/v2/SQLitePersistentStorage.cs | 2 +- .../v2/SQLitePersistentStorage_FlushWrites.cs | 4 +- 8 files changed, 118 insertions(+), 78 deletions(-) delete mode 100644 src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingDelay.cs create mode 100644 src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs create mode 100644 src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs rename src/Workspaces/Core/Portable/Shared/Utilities/{AsyncBatchingWorkQueue.cs => AsyncBatchingWorkQueue`2.cs} (85%) diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs index 63dca1866fc32..e265ecb2f3d4d 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs @@ -15,7 +15,7 @@ internal partial class TaggerEventSources { private class WorkspaceChangedEventSource : AbstractWorkspaceTrackingTaggerEventSource { - private readonly AsyncBatchingDelay _asyncDelay; + private readonly AsyncBatchingWorkQueue _asyncDelay; public WorkspaceChangedEventSource( ITextBuffer subjectBuffer, @@ -24,9 +24,9 @@ public WorkspaceChangedEventSource( { // That will ensure that even if we get a flurry of workspace events that we // only process a tag change once. - _asyncDelay = new AsyncBatchingDelay( + _asyncDelay = new AsyncBatchingWorkQueue( TimeSpan.FromMilliseconds(250), - processAsync: cancellationToken => + processBatchAsync: cancellationToken => { RaiseChanged(); return Task.CompletedTask; @@ -48,7 +48,7 @@ protected override void DisconnectFromWorkspace(Workspace workspace) } private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs eventArgs) - => _asyncDelay.RequeueWork(); + => _asyncDelay.AddWork(); } } } diff --git a/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs b/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs index 2dad6684c85fe..708ce56de7ccc 100644 --- a/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs +++ b/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs @@ -247,7 +247,7 @@ private class OpenSourceGeneratedFile : ForegroundThreadAffinitizedObject, IDisp /// /// A queue used to batch updates to the file. /// - private readonly AsyncBatchingDelay _batchingWorkQueue; + private readonly AsyncBatchingWorkQueue _batchingWorkQueue; /// /// The of the active window. This may be null if we're in the middle of construction and @@ -284,7 +284,7 @@ public OpenSourceGeneratedFile(SourceGeneratedFileManager fileManager, ITextBuff _workspace.WorkspaceChanged += OnWorkspaceChanged; - _batchingWorkQueue = new AsyncBatchingDelay( + _batchingWorkQueue = new AsyncBatchingWorkQueue( TimeSpan.FromSeconds(1), RefreshFileAsync, asyncListener: _fileManager._listener, @@ -436,7 +436,7 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) if (await oldProject.GetDependentVersionAsync(_cancellationTokenSource.Token).ConfigureAwait(false) != await newProject.GetDependentVersionAsync(_cancellationTokenSource.Token).ConfigureAwait(false)) { - _batchingWorkQueue.RequeueWork(); + _batchingWorkQueue.AddWork(); } }, _cancellationTokenSource.Token).CompletesAsyncOperation(asyncToken); } diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingDelay.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingDelay.cs deleted file mode 100644 index ccdd3f39c2a26..0000000000000 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingDelay.cs +++ /dev/null @@ -1,50 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Shared.TestHooks; -using Roslyn.Utilities; - -namespace Roslyn.Utilities -{ - internal sealed class AsyncBatchingDelay - { - private readonly AsyncBatchingWorkQueue _workQueue; - private readonly Func _processAsync; - - public AsyncBatchingDelay( - TimeSpan delay, - Func processAsync, - IAsynchronousOperationListener? asyncListener, - CancellationToken cancellationToken) - { - _processAsync = processAsync; - - // We use an AsyncBatchingWorkQueue with a boolean, and just always add the - // same value at all times. - _workQueue = new AsyncBatchingWorkQueue( - delay, - OnNotifyAsync, - equalityComparer: EqualityComparer.Default, - asyncListener, - cancellationToken); - } - - private Task OnNotifyAsync(ImmutableArray _, CancellationToken cancellationToken) - { - return _processAsync(cancellationToken); - } - - public void RequeueWork() - { - // Value doesn't matter here as long as we're consistent. - _workQueue.AddWork(item: false); - } - } -} diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs new file mode 100644 index 0000000000000..150fa2f080981 --- /dev/null +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Shared.TestHooks; + +namespace Roslyn.Utilities +{ + /// + internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue + { + public AsyncBatchingWorkQueue( + TimeSpan delay, + Func processBatchAsync, + CancellationToken cancellationToken) + : this(delay, + processBatchAsync, + asyncListener: null, + cancellationToken) + { + } + + public AsyncBatchingWorkQueue( + TimeSpan delay, + Func processBatchAsync, + IAsynchronousOperationListener? asyncListener, + CancellationToken cancellationToken) + : base(delay, Convert(processBatchAsync), EqualityComparer.Default, asyncListener, cancellationToken) + { + } + + private static Func, CancellationToken, Task> Convert(Func processBatchAsync) + => (items, ct) => processBatchAsync(ct); + + public void AddWork() + => base.AddWork(true); + } +} diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs new file mode 100644 index 0000000000000..b500ffcbaff54 --- /dev/null +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Shared.TestHooks; + +namespace Roslyn.Utilities +{ + /// + internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue + { + public AsyncBatchingWorkQueue( + TimeSpan delay, + Func, CancellationToken, Task> processBatchAsync, + CancellationToken cancellationToken) + : this(delay, + processBatchAsync, + equalityComparer: null, + asyncListener: null, + cancellationToken) + { + } + + public AsyncBatchingWorkQueue( + TimeSpan delay, + Func, CancellationToken, Task> processBatchAsync, + IEqualityComparer? equalityComparer, + IAsynchronousOperationListener? asyncListener, + CancellationToken cancellationToken) + : base(delay, Convert(processBatchAsync), equalityComparer, asyncListener, cancellationToken) + { + } + + private static Func, CancellationToken, Task> Convert(Func, CancellationToken, Task> processBatchAsync) + => async (items, ct) => + { + await processBatchAsync(items, ct).ConfigureAwait(false); + return true; + }; + + public new Task WaitUntilCurrentBatchCompletesAsync() + => base.WaitUntilCurrentBatchCompletesAsync(); + } +} diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs similarity index 85% rename from src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue.cs rename to src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs index 125ba2db6c417..63e6df6803c7e 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs @@ -19,7 +19,7 @@ namespace Roslyn.Utilities /// along to be worked on. Rounds of processing happen serially, only starting up after a /// previous round has completed. /// - internal class AsyncBatchingWorkQueue + internal class AsyncBatchingWorkQueue { /// /// Delay we wait after finishing the processing of one batch and starting up on then. @@ -34,7 +34,7 @@ internal class AsyncBatchingWorkQueue /// /// Callback to actually perform the processing of the next batch of work. /// - private readonly Func, CancellationToken, Task> _processBatchAsync; + private readonly Func, CancellationToken, Task> _processBatchAsync; private readonly IAsynchronousOperationListener _asyncListener; private readonly CancellationToken _cancellationToken; @@ -63,7 +63,7 @@ internal class AsyncBatchingWorkQueue /// Task kicked off to do the next batch of processing of . These /// tasks form a chain so that the next task only processes when the previous one completes. /// - private Task _updateTask = Task.CompletedTask; + private Task _updateTask = Task.FromResult(default(TResult)); /// /// Whether or not there is an existing task in flight that will process the current batch @@ -76,7 +76,7 @@ internal class AsyncBatchingWorkQueue public AsyncBatchingWorkQueue( TimeSpan delay, - Func, CancellationToken, Task> processBatchAsync, + Func, CancellationToken, Task> processBatchAsync, CancellationToken cancellationToken) : this(delay, processBatchAsync, @@ -86,11 +86,9 @@ public AsyncBatchingWorkQueue( { } - /// Callback to add the new items to the current batch. It is legal to mutate - /// the current batch (for example, clearing the batch or deduplicating) public AsyncBatchingWorkQueue( TimeSpan delay, - Func, CancellationToken, Task> processBatchAsync, + Func, CancellationToken, Task> processBatchAsync, IEqualityComparer? equalityComparer, IAsynchronousOperationListener? asyncListener, CancellationToken cancellationToken) @@ -126,7 +124,7 @@ public void AddWork(IEnumerable items) } } - public Task WaitUntilCurrentBatchCompletesAsync() + public Task WaitUntilCurrentBatchCompletesAsync() { lock (_gate) return _updateTask; @@ -158,21 +156,21 @@ private void TryKickOffNextBatchTask() // No in-flight task. Kick one off to process these messages a second from now. // We always attach the task to the previous one so that notifications to the ui // follow the same order as the notification the OOP server sent to us. - var token = _asyncListener.BeginAsyncOperation(nameof(TryKickOffNextBatchTask)); + _updateTask = ContinueAfterDelay(); + _taskInFlight = true; + } - _updateTask = _updateTask.ContinueWithAfterDelayFromAsync( - _ => ProcessNextBatchAsync(_cancellationToken), - _cancellationToken, - _delay, - _asyncListener, - TaskContinuationOptions.RunContinuationsAsynchronously, - TaskScheduler.Default).CompletesAsyncOperation(token); + return; - _taskInFlight = true; + async Task ContinueAfterDelay() + { + using var _ = _asyncListener.BeginAsyncOperation(nameof(TryKickOffNextBatchTask)); + await _asyncListener.Delay(_delay, _cancellationToken).ConfigureAwait(false); + return await ProcessNextBatchAsync(_cancellationToken).ConfigureAwait(false); } } - private Task ProcessNextBatchAsync(CancellationToken cancellationToken) + private Task ProcessNextBatchAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); return _processBatchAsync(GetNextBatchAndResetQueue(), _cancellationToken); diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs index 418b29c4b8240..dd831f8993480 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs @@ -63,7 +63,7 @@ private SQLitePersistentStorage( CancellationToken.None)!; // Create a delay to batch up requests to flush. We'll won't flush more than every FlushAllDelayMS. - _flushQueue = new AsyncBatchingDelay( + _flushQueue = new AsyncBatchingWorkQueue( TimeSpan.FromMilliseconds(FlushAllDelayMS), FlushInMemoryDataToDiskIfNotShutdownAsync, asyncListener: null, diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs index 7bb665bd52a99..4772d0f77b330 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs @@ -16,11 +16,11 @@ internal partial class SQLitePersistentStorage /// A queue to batch up flush requests and ensure that we don't issue then more often than every . /// - private readonly AsyncBatchingDelay _flushQueue; + private readonly AsyncBatchingWorkQueue _flushQueue; private void EnqueueFlushTask() { - _flushQueue.RequeueWork(); + _flushQueue.AddWork(); } private Task FlushInMemoryDataToDiskIfNotShutdownAsync(CancellationToken cancellationToken) From aa44b78b0439048dfff441f43884a4e07509e0ce Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 12:35:46 -0700 Subject: [PATCH 07/19] Feedback --- .../Portable/InternalUtilities/VoidResult.cs | 14 ++++- .../CustomProtocol/FindUsagesLSPContext.cs | 4 +- .../Utilities/AsyncBatchingWorkQueue`0.cs | 21 ++----- .../Utilities/AsyncBatchingWorkQueue`1.cs | 11 ++-- .../Utilities/AsyncBatchingWorkQueue`2.cs | 63 ++++++++----------- ...DesktopPersistenceStorageServiceFactory.cs | 9 ++- .../SQLite/v2/SQLitePersistentStorage.cs | 8 ++- .../v2/SQLitePersistentStorageService.cs | 10 ++- 8 files changed, 73 insertions(+), 67 deletions(-) diff --git a/src/Compilers/Core/Portable/InternalUtilities/VoidResult.cs b/src/Compilers/Core/Portable/InternalUtilities/VoidResult.cs index 73048a55c9757..ea481f9017836 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/VoidResult.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/VoidResult.cs @@ -2,10 +2,22 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; + namespace Roslyn.Utilities { /// /// Explicitly indicates result is void /// - internal readonly struct VoidResult { } + internal readonly struct VoidResult : IEquatable + { + public override bool Equals(object? obj) + => obj is VoidResult; + + public override int GetHashCode() + => 0; + + public bool Equals(VoidResult other) + => true; + } } diff --git a/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs b/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs index b607e5ca7927c..a089ccc382b6d 100644 --- a/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs +++ b/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs @@ -17,6 +17,7 @@ using Microsoft.CodeAnalysis.FindUsages; using Microsoft.CodeAnalysis.MetadataAsSource; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.LanguageServer.Protocol; using Microsoft.VisualStudio.Text.Adornments; @@ -72,6 +73,7 @@ public FindUsagesLSPContext( Document document, int position, IMetadataAsSourceFileService metadataAsSourceFileService, + IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) { _progress = progress; @@ -79,7 +81,7 @@ public FindUsagesLSPContext( _position = position; _metadataAsSourceFileService = metadataAsSourceFileService; _workQueue = new AsyncBatchingWorkQueue( - TimeSpan.FromMilliseconds(500), ReportReferencesAsync, cancellationToken); + TimeSpan.FromMilliseconds(500), ReportReferencesAsync, asyncListener, cancellationToken); } // After all definitions/references have been found, wait here until all results have been reported. diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs index 150fa2f080981..7eb26a198630d 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs @@ -12,32 +12,21 @@ namespace Roslyn.Utilities { /// - internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue + internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue { public AsyncBatchingWorkQueue( TimeSpan delay, Func processBatchAsync, + IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) - : this(delay, - processBatchAsync, - asyncListener: null, - cancellationToken) + : base(delay, Convert(processBatchAsync), EqualityComparer.Default, asyncListener, cancellationToken) { } - public AsyncBatchingWorkQueue( - TimeSpan delay, - Func processBatchAsync, - IAsynchronousOperationListener? asyncListener, - CancellationToken cancellationToken) - : base(delay, Convert(processBatchAsync), EqualityComparer.Default, asyncListener, cancellationToken) - { - } - - private static Func, CancellationToken, Task> Convert(Func processBatchAsync) + private static Func, CancellationToken, Task> Convert(Func processBatchAsync) => (items, ct) => processBatchAsync(ct); public void AddWork() - => base.AddWork(true); + => base.AddWork(default(VoidResult)); } } diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs index b500ffcbaff54..ec22008f93e9a 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs @@ -12,16 +12,17 @@ namespace Roslyn.Utilities { /// - internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue + internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue { public AsyncBatchingWorkQueue( TimeSpan delay, Func, CancellationToken, Task> processBatchAsync, + IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) : this(delay, processBatchAsync, equalityComparer: null, - asyncListener: null, + asyncListener, cancellationToken) { } @@ -30,17 +31,17 @@ public AsyncBatchingWorkQueue( TimeSpan delay, Func, CancellationToken, Task> processBatchAsync, IEqualityComparer? equalityComparer, - IAsynchronousOperationListener? asyncListener, + IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) : base(delay, Convert(processBatchAsync), equalityComparer, asyncListener, cancellationToken) { } - private static Func, CancellationToken, Task> Convert(Func, CancellationToken, Task> processBatchAsync) + private static Func, CancellationToken, Task> Convert(Func, CancellationToken, Task> processBatchAsync) => async (items, ct) => { await processBatchAsync(items, ct).ConfigureAwait(false); - return true; + return default; }; public new Task WaitUntilCurrentBatchCompletesAsync() diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs index 63e6df6803c7e..2878b59820cee 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs @@ -74,29 +74,17 @@ internal class AsyncBatchingWorkQueue #endregion - public AsyncBatchingWorkQueue( - TimeSpan delay, - Func, CancellationToken, Task> processBatchAsync, - CancellationToken cancellationToken) - : this(delay, - processBatchAsync, - equalityComparer: null, - asyncListener: null, - cancellationToken) - { - } - public AsyncBatchingWorkQueue( TimeSpan delay, Func, CancellationToken, Task> processBatchAsync, IEqualityComparer? equalityComparer, - IAsynchronousOperationListener? asyncListener, + IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) { _delay = delay; _processBatchAsync = processBatchAsync; _equalityComparer = equalityComparer; - _asyncListener = asyncListener ?? AsynchronousOperationListenerProvider.NullListener; + _asyncListener = asyncListener; _cancellationToken = cancellationToken; _uniqueItems = new HashSet(equalityComparer); @@ -120,7 +108,29 @@ public void AddWork(IEnumerable items) { // add our work to the set we'll process in the next batch. AddItemsToBatch(items); - TryKickOffNextBatchTask(); + + if (!_taskInFlight) + { + // No in-flight task. Kick one off to process these messages a second from now. + // We always attach the task to the previous one so that notifications to the ui + // follow the same order as the notification the OOP server sent to us. + _updateTask = ContinueAfterDelay(); + _taskInFlight = true; + } + } + + return; + + async Task ContinueAfterDelay() + { + using var _ = _asyncListener.BeginAsyncOperation(nameof(AddWork)); + // Ensure that we always yield the current thread this is necessary for correctness as we are called + // inside a lock that _taskInFlight to true. We must ensure that the work to process the next batch + // must be on another thread that runs afterwards, can only grab the thread once we release it and will + // then reset that bool back to false + await Task.Yield().ConfigureAwait(false); + await _asyncListener.Delay(_delay, _cancellationToken).ConfigureAwait(false); + return await ProcessNextBatchAsync(_cancellationToken).ConfigureAwait(false); } } @@ -147,29 +157,6 @@ private void AddItemsToBatch(IEnumerable items) } } - private void TryKickOffNextBatchTask() - { - Debug.Assert(Monitor.IsEntered(_gate)); - - if (!_taskInFlight) - { - // No in-flight task. Kick one off to process these messages a second from now. - // We always attach the task to the previous one so that notifications to the ui - // follow the same order as the notification the OOP server sent to us. - _updateTask = ContinueAfterDelay(); - _taskInFlight = true; - } - - return; - - async Task ContinueAfterDelay() - { - using var _ = _asyncListener.BeginAsyncOperation(nameof(TryKickOffNextBatchTask)); - await _asyncListener.Delay(_delay, _cancellationToken).ConfigureAwait(false); - return await ProcessNextBatchAsync(_cancellationToken).ConfigureAwait(false); - } - } - private Task ProcessNextBatchAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs b/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs index 4408ca3467b3c..dd6743b406e63 100644 --- a/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs +++ b/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Shared.TestHooks; // When building for source-build, there is no sqlite dependency #if !DOTNET_BUILD_FROM_SOURCE @@ -36,12 +37,16 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) #else private readonly SQLiteConnectionPoolService _connectionPoolService; + private readonly IAsynchronousOperationListener _asyncListener; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] - public DesktopPersistenceStorageServiceFactory(SQLiteConnectionPoolService connectionPoolService) + public DesktopPersistenceStorageServiceFactory( + SQLiteConnectionPoolService connectionPoolService, + IAsynchronousOperationListenerProvider asyncOperationListenerProvider) { _connectionPoolService = connectionPoolService; + _asyncListener = asyncOperationListenerProvider.GetListener(FeatureAttribute.Workspace); } public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) @@ -55,7 +60,7 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) switch (database) { case StorageDatabase.SQLite: - return new SQLitePersistentStorageService(options, _connectionPoolService, locationService); + return new SQLitePersistentStorageService(options, _connectionPoolService, locationService, _asyncListener); case StorageDatabase.CloudCache: var factory = workspaceServices.GetService(); diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs index dd831f8993480..4b4970d5dc1e1 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.PersistentStorage; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SQLite.v2.Interop; using Microsoft.CodeAnalysis.Storage; using Roslyn.Utilities; @@ -45,6 +46,7 @@ private SQLitePersistentStorage( string workingFolderPath, string solutionFilePath, string databaseFile, + IAsynchronousOperationListener asyncListener, IPersistentStorageFaultInjector? faultInjector) : base(workingFolderPath, solutionFilePath, databaseFile) { @@ -66,7 +68,7 @@ private SQLitePersistentStorage( _flushQueue = new AsyncBatchingWorkQueue( TimeSpan.FromMilliseconds(FlushAllDelayMS), FlushInMemoryDataToDiskIfNotShutdownAsync, - asyncListener: null, + asyncListener, _shutdownTokenSource.Token); } @@ -75,9 +77,11 @@ private SQLitePersistentStorage( string workingFolderPath, string solutionFilePath, string databaseFile, + IAsynchronousOperationListener asyncListener, IPersistentStorageFaultInjector? faultInjector) { - var sqlStorage = new SQLitePersistentStorage(connectionPoolService, workingFolderPath, solutionFilePath, databaseFile, faultInjector); + var sqlStorage = new SQLitePersistentStorage( + connectionPoolService, workingFolderPath, solutionFilePath, databaseFile, asyncListener, faultInjector); if (sqlStorage._connectionPool is null) { // The connection pool failed to initialize diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs index 9baaf0089c3ba..5b19565285155 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorageService.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.PersistentStorage; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.SQLite.v2 @@ -19,25 +20,29 @@ internal class SQLitePersistentStorageService : AbstractSQLitePersistentStorageS private const string PersistentStorageFileName = "storage.ide"; private readonly SQLiteConnectionPoolService _connectionPoolService; + private readonly IAsynchronousOperationListener _asyncListener; private readonly OptionSet _options; private readonly IPersistentStorageFaultInjector? _faultInjector; public SQLitePersistentStorageService( OptionSet options, SQLiteConnectionPoolService connectionPoolService, - IPersistentStorageLocationService locationService) + IPersistentStorageLocationService locationService, + IAsynchronousOperationListener asyncListener) : base(locationService) { _options = options; _connectionPoolService = connectionPoolService; + _asyncListener = asyncListener; } public SQLitePersistentStorageService( OptionSet options, SQLiteConnectionPoolService connectionPoolService, IPersistentStorageLocationService locationService, + IAsynchronousOperationListener asyncListener, IPersistentStorageFaultInjector? faultInjector) - : this(options, connectionPoolService, locationService) + : this(options, connectionPoolService, locationService, asyncListener) { _faultInjector = faultInjector; } @@ -65,6 +70,7 @@ protected override string GetDatabaseFilePath(string workingFolderPath) workingFolderPath, solutionKey.FilePath, databaseFilePath, + _asyncListener, _faultInjector)); } From 3d093ad96b8b895bb22a993bac529350ccc2db49 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 12:39:31 -0700 Subject: [PATCH 08/19] One more use case --- .../Handler/References/FindAllReferencesHandler.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Features/LanguageServer/Protocol/Handler/References/FindAllReferencesHandler.cs b/src/Features/LanguageServer/Protocol/Handler/References/FindAllReferencesHandler.cs index a1d6b8f3ee1e1..86201184020b4 100644 --- a/src/Features/LanguageServer/Protocol/Handler/References/FindAllReferencesHandler.cs +++ b/src/Features/LanguageServer/Protocol/Handler/References/FindAllReferencesHandler.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis.LanguageServer.CustomProtocol; using Microsoft.CodeAnalysis.MetadataAsSource; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.VisualStudio.LanguageServer.Protocol; using LSP = Microsoft.VisualStudio.LanguageServer.Protocol; @@ -22,12 +23,16 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler internal class FindAllReferencesHandler : AbstractStatelessRequestHandler { private readonly IMetadataAsSourceFileService _metadataAsSourceFileService; + private readonly IAsynchronousOperationListener _asyncListener; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] - public FindAllReferencesHandler(IMetadataAsSourceFileService metadataAsSourceFileService) + public FindAllReferencesHandler( + IMetadataAsSourceFileService metadataAsSourceFileService, + IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider) { _metadataAsSourceFileService = metadataAsSourceFileService; + _asyncListener = asynchronousOperationListenerProvider.GetListener(FeatureAttribute.LanguageServer); } public override string Method => LSP.Methods.TextDocumentReferencesName; @@ -54,7 +59,7 @@ public FindAllReferencesHandler(IMetadataAsSourceFileService metadataAsSourceFil ProtocolConversions.PositionToLinePosition(referenceParams.Position), cancellationToken).ConfigureAwait(false); var findUsagesContext = new FindUsagesLSPContext( - progress, document, position, _metadataAsSourceFileService, cancellationToken); + progress, document, position, _metadataAsSourceFileService, _asyncListener, cancellationToken); // Finds the references for the symbol at the specific position in the document, reporting them via streaming to the LSP client. await findUsagesService.FindReferencesAsync(document, position, findUsagesContext, cancellationToken).ConfigureAwait(false); From ad3403d926056921c7bcdd74bef7c7892958dc8e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 13:13:38 -0700 Subject: [PATCH 09/19] Fix tests --- .../SQLitePersistentStorageBenchmark.cs | 5 ++++- .../AbstractPersistentStorageTests.cs | 1 + .../SQLiteV2PersistentStorageTests.cs | 8 +++++++- .../VisualStudioDesignerAttributeService.cs | 3 +++ .../AbstractTableDataSourceFindUsagesContext.cs | 1 + .../FindReferences/StreamingFindUsagesPresenter.cs | 13 ++++++++++--- .../VisualStudioProjectTelemetryService.cs | 5 ++++- .../TodoComments/VisualStudioTodoCommentsService.cs | 6 +++++- .../Portable/Shared/TestHooks/FeatureAttribute.cs | 2 ++ 9 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs b/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs index 6eba6f1291013..fd2ad0bd30745 100644 --- a/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs +++ b/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.PersistentStorage; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SQLite.v2; using Microsoft.CodeAnalysis.Storage; using Microsoft.CodeAnalysis.Test.Utilities; @@ -68,7 +69,9 @@ public void GlobalSetup() .WithChangedOption(StorageOptions.DatabaseMustSucceed, true))); var connectionPoolService = _workspace.ExportProvider.GetExportedValue(); - _storageService = new SQLitePersistentStorageService(_workspace.Options, connectionPoolService, new LocationService()); + _storageService = new SQLitePersistentStorageService( + _workspace.Options, connectionPoolService, new LocationService(), + _workspace.ExportProvider.GetExportedValue().GetListener(FeatureAttribute.Workspace)); var solution = _workspace.CurrentSolution; _storage = _storageService.GetStorageWorkerAsync(_workspace, SolutionKey.ToSolutionKey(solution), solution, CancellationToken.None).AsTask().GetAwaiter().GetResult(); diff --git a/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs b/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs index 4b6d4af210ab8..d760162eb2f23 100644 --- a/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs +++ b/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.PersistentStorage; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Storage; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.VisualStudio.LanguageServices.UnitTests; diff --git a/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs b/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs index 06976afc94576..b21e6c48620f9 100644 --- a/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs +++ b/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SQLite.v2; using Microsoft.CodeAnalysis.Storage; using Xunit; @@ -23,7 +24,12 @@ namespace Microsoft.CodeAnalysis.UnitTests.WorkspaceServices public class SQLiteV2PersistentStorageTests : AbstractPersistentStorageTests { internal override AbstractPersistentStorageService GetStorageService(OptionSet options, IMefHostExportProvider exportProvider, IPersistentStorageLocationService locationService, IPersistentStorageFaultInjector? faultInjector, string relativePathBase) - => new SQLitePersistentStorageService(options, exportProvider.GetExports().Single().Value, locationService, faultInjector); + => new SQLitePersistentStorageService( + options, + exportProvider.GetExports().Single().Value, + locationService, + exportProvider.GetExports().Single().Value.GetListener(FeatureAttribute.Workspace), + faultInjector); [Fact] public async Task TestCrashInNewConnection() diff --git a/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs b/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs index 05015c4ebea93..777bc632ee5b3 100644 --- a/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs +++ b/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs @@ -18,6 +18,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SolutionCrawler; using Microsoft.VisualStudio.Designer.Interfaces; using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem; @@ -66,6 +67,7 @@ internal class VisualStudioDesignerAttributeService public VisualStudioDesignerAttributeService( VisualStudioWorkspaceImpl workspace, IThreadingContext threadingContext, + IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider, Shell.SVsServiceProvider serviceProvider) : base(threadingContext) { @@ -75,6 +77,7 @@ public VisualStudioDesignerAttributeService( _workQueue = new AsyncBatchingWorkQueue( TimeSpan.FromSeconds(1), this.NotifyProjectSystemAsync, + asynchronousOperationListenerProvider.GetListener(FeatureAttribute.DesignerAttributes), ThreadingContext.DisposalToken); } diff --git a/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs b/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs index 1ec25628cadec..a7109e8e6d1da 100644 --- a/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs +++ b/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs @@ -143,6 +143,7 @@ protected AbstractTableDataSourceFindUsagesContext( _progressQueue = new AsyncBatchingWorkQueue<(int current, int maximum)>( TimeSpan.FromMilliseconds(250), this.UpdateTableProgressAsync, + presenter._asyncListener, CancellationTokenSource.Token); } diff --git a/src/VisualStudio/Core/Def/Implementation/FindReferences/StreamingFindUsagesPresenter.cs b/src/VisualStudio/Core/Def/Implementation/FindReferences/StreamingFindUsagesPresenter.cs index 63db39ae31c06..bab65db029281 100644 --- a/src/VisualStudio/Core/Def/Implementation/FindReferences/StreamingFindUsagesPresenter.cs +++ b/src/VisualStudio/Core/Def/Implementation/FindReferences/StreamingFindUsagesPresenter.cs @@ -16,6 +16,7 @@ using Microsoft.CodeAnalysis.FindUsages; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.VisualStudio.LanguageServices.Implementation.FindReferences; using Microsoft.VisualStudio.Shell.FindAllReferences; using Microsoft.VisualStudio.Shell.TableControl; @@ -38,6 +39,7 @@ internal partial class StreamingFindUsagesPresenter : public readonly ClassificationTypeMap TypeMap; public readonly IEditorFormatMapService FormatMapService; + private readonly IAsynchronousOperationListener _asyncListener; public readonly IClassificationFormatMap ClassificationFormatMap; private readonly Workspace _workspace; @@ -55,6 +57,7 @@ public StreamingFindUsagesPresenter( ClassificationTypeMap typeMap, IEditorFormatMapService formatMapService, IClassificationFormatMapService classificationFormatMapService, + IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider, [ImportMany] IEnumerable> columns) : this(workspace, threadingContext, @@ -62,7 +65,8 @@ public StreamingFindUsagesPresenter( typeMap, formatMapService, classificationFormatMapService, - GetCustomColumns(columns)) + GetCustomColumns(columns), + asynchronousOperationListenerProvider) { } @@ -77,7 +81,8 @@ public StreamingFindUsagesPresenter( exportProvider.GetExportedValue(), exportProvider.GetExportedValue(), exportProvider.GetExportedValue(), - exportProvider.GetExportedValues()) + exportProvider.GetExportedValues(), + exportProvider.GetExportedValue()) { } @@ -89,13 +94,15 @@ private StreamingFindUsagesPresenter( ClassificationTypeMap typeMap, IEditorFormatMapService formatMapService, IClassificationFormatMapService classificationFormatMapService, - IEnumerable columns) + IEnumerable columns, + IAsynchronousOperationListenerProvider asyncListenerProvider) : base(threadingContext, assertIsForeground: false) { _workspace = workspace; _serviceProvider = serviceProvider; TypeMap = typeMap; FormatMapService = formatMapService; + _asyncListener = asyncListenerProvider.GetListener(FeatureAttribute.FindReferences); ClassificationFormatMap = classificationFormatMapService.GetClassificationFormatMap("tooltip"); _customColumns = columns.ToImmutableArray(); diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs b/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs index 61b714b413b33..ca9c89316521a 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs @@ -16,6 +16,7 @@ using Microsoft.CodeAnalysis.ProjectTelemetry; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.Internal.VisualStudio.Shell; using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem; using Roslyn.Utilities; @@ -58,13 +59,15 @@ internal class VisualStudioProjectTelemetryService [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] public VisualStudioProjectTelemetryService( VisualStudioWorkspaceImpl workspace, - IThreadingContext threadingContext) : base(threadingContext) + IThreadingContext threadingContext, + IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider) : base(threadingContext) { _workspace = workspace; _workQueue = new AsyncBatchingWorkQueue( TimeSpan.FromSeconds(1), NotifyTelemetryServiceAsync, + asynchronousOperationListenerProvider.GetListener(FeatureAttribute.Telemetry), threadingContext.DisposalToken); } diff --git a/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs b/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs index 29c0c6b63ced8..58b71b901be83 100644 --- a/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs +++ b/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs @@ -18,6 +18,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SolutionCrawler; using Microsoft.CodeAnalysis.TodoComments; using Microsoft.VisualStudio.LanguageServices.ExternalAccess.VSTypeScript.Api; @@ -38,7 +39,7 @@ internal class VisualStudioTodoCommentsService { private readonly VisualStudioWorkspaceImpl _workspace; private readonly EventListenerTracker _eventListenerTracker; - + private readonly IAsynchronousOperationListener _asyncListener; private readonly ConcurrentDictionary> _documentToInfos = new(); @@ -61,11 +62,13 @@ private readonly TaskCompletionSource> eventListeners) : base(threadingContext) { _workspace = workspace; _eventListenerTracker = new EventListenerTracker(eventListeners, WellKnownEventListeners.TodoListProvider); + _asyncListener = asynchronousOperationListenerProvider.GetListener(FeatureAttribute.TodoCommentList); } public void Dispose() @@ -106,6 +109,7 @@ private async Task StartWorkerAsync() new AsyncBatchingWorkQueue( TimeSpan.FromSeconds(1), ProcessTodoCommentInfosAsync, + _asyncListener, cancellationToken)); var client = await RemoteHostClient.TryGetClientAsync(_workspace, cancellationToken).ConfigureAwait(false); diff --git a/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs b/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs index 759e1e45b6a54..9d65a42def7fd 100644 --- a/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs +++ b/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs @@ -13,6 +13,7 @@ internal static class FeatureAttribute public const string Classification = nameof(Classification); public const string CodeModel = nameof(CodeModel); public const string CompletionSet = nameof(CompletionSet); + public const string DesignerAttributes = nameof(DesignerAttributes); public const string DiagnosticService = nameof(DiagnosticService); public const string EncapsulateField = nameof(EncapsulateField); public const string ErrorList = nameof(ErrorList); @@ -42,6 +43,7 @@ internal static class FeatureAttribute public const string SignatureHelp = nameof(SignatureHelp); public const string Snippets = nameof(Snippets); public const string SolutionCrawler = nameof(SolutionCrawler); + public const string Telemetry = nameof(Telemetry); public const string TodoCommentList = nameof(TodoCommentList); public const string LanguageServer = nameof(LanguageServer); public const string Workspace = nameof(Workspace); From 83aa8a554b41367efbb19cfcf085a243390b3411 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Mon, 28 Jun 2021 13:42:20 -0700 Subject: [PATCH 10/19] Update src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs --- .../Implementation/NavigationBar/NavigationBarController.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs index cd342eaa7d92e..12371290f6580 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs @@ -113,9 +113,7 @@ public NavigationBarController( presenter.ItemSelected += OnItemSelected; // Initialize the tasks to be an empty model so we never have to deal with a null case. - _latestModelAndSelectedInfo_OnlyAccessOnUIThread.model = new( - ImmutableArray.Empty, - itemService: null!); + _latestModelAndSelectedInfo_OnlyAccessOnUIThread.model = new(ImmutableArray.Empty, itemService: null!); _latestModelAndSelectedInfo_OnlyAccessOnUIThread.selectedInfo = new(typeItem: null, memberItem: null); _model_OnlyAccessOnUIThread = _latestModelAndSelectedInfo_OnlyAccessOnUIThread.model; From e60204a37b27a359225f72cc3ade8a81cced9657 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 14:00:48 -0700 Subject: [PATCH 11/19] Move --- .../NavigationBar/NavigationBarController.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs index cd342eaa7d92e..398ace6bad33a 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs @@ -136,6 +136,11 @@ public NavigationBarController( _eventSource.Connect(); } + private void OnEventSourceChanged(object? sender, TaggerEventArgs e) + { + StartModelUpdateAndSelectedItemUpdateTasks(); + } + public void Disconnect() { AssertIsForeground(); @@ -157,11 +162,6 @@ public void Disconnect() _cancellationTokenSource.Cancel(); } - private void OnEventSourceChanged(object? sender, TaggerEventArgs e) - { - StartModelUpdateAndSelectedItemUpdateTasks(); - } - public void SetWorkspace(Workspace? newWorkspace) { StartModelUpdateAndSelectedItemUpdateTasks(); From 4ee919ea3ff3fe8e94ba59c3e1ab9c651711b892 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 14:39:57 -0700 Subject: [PATCH 12/19] Fix --- .../ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs index 390154eb27916..4e031801aadc8 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs @@ -97,7 +97,7 @@ public async Task TestNoDocumentDiagnosticsForOpenFilesIfDefaultAndFeatureFlagOf var testExperimentationService = (TestExperimentationService)testLspServer.TestWorkspace.Services.GetRequiredService(); testExperimentationService.SetExperimentOption(WellKnownExperimentNames.LspPullDiagnosticsFeatureFlag, false); - var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document); + var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document.GetURI()); Assert.Empty(results.Single().Diagnostics); } @@ -116,7 +116,7 @@ public async Task TestDocumentDiagnosticsForOpenFilesIfDefaultAndFeatureFlagOn() var testExperimentationService = (TestExperimentationService)testLspServer.TestWorkspace.Services.GetRequiredService(); testExperimentationService.SetExperimentOption(WellKnownExperimentNames.LspPullDiagnosticsFeatureFlag, true); - var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document); + var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document.GetURI()); Assert.Equal("CS1513", results.Single().Diagnostics.Single().Code); } @@ -605,7 +605,7 @@ private TestLspServer CreateTestWorkspaceWithDiagnostics(string markup, Backgrou private TestLspServer CreateTestWorkspaceFromXml(string xmlMarkup, BackgroundAnalysisScope scope, bool pullDiagnostics = true) { var testLspServer = CreateXmlTestLspServer(xmlMarkup, out _); - InitializeDiagnostics(scope, testLspServer.TestWorkspace, pullDiagnostics); + InitializeDiagnostics(scope, testLspServer.TestWorkspace, pullDiagnostics ? DiagnosticMode.Pull : DiagnosticMode.Push); return testLspServer; } From bc3e244bff34fc11a3745c2f4a4fdeeb620b10a9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 14:54:50 -0700 Subject: [PATCH 13/19] No need to hear about the workspace --- .../NavigationBar/INavigationBarController.cs | 13 ------------- .../INavigationBarControllerFactoryService.cs | 3 ++- .../NavigationBar/NavigationBarController.cs | 12 +++++------- .../NavigationBarControllerFactoryService.cs | 2 +- .../NavigationBar/NavigationBarPresenterTests.vb | 5 ----- .../Diagnostics/PullDiagnosticTests.cs | 6 +++--- ...AbstractLanguageService`2.VsCodeWindowManager.cs | 10 ++++------ 7 files changed, 15 insertions(+), 36 deletions(-) delete mode 100644 src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarController.cs diff --git a/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarController.cs b/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarController.cs deleted file mode 100644 index 99ef6d783a404..0000000000000 --- a/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarController.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -namespace Microsoft.CodeAnalysis.Editor -{ - internal interface INavigationBarController - { - void Disconnect(); - - void SetWorkspace(Workspace? newWorkspace); - } -} diff --git a/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarControllerFactoryService.cs b/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarControllerFactoryService.cs index f5516183c958b..0ff6f947d5f33 100644 --- a/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarControllerFactoryService.cs +++ b/src/EditorFeatures/Core/Extensibility/NavigationBar/INavigationBarControllerFactoryService.cs @@ -4,12 +4,13 @@ #nullable disable +using System; using Microsoft.VisualStudio.Text; namespace Microsoft.CodeAnalysis.Editor { internal interface INavigationBarControllerFactoryService { - INavigationBarController CreateController(INavigationBarPresenter presenter, ITextBuffer textBuffer); + IDisposable CreateController(INavigationBarPresenter presenter, ITextBuffer textBuffer); } } diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs index c7eb023e78dcd..96b5cd6f3189b 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs @@ -31,7 +31,7 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.NavigationBar /// The threading model for this class is simple: all non-static members are affinitized to the /// UI thread. /// - internal partial class NavigationBarController : ForegroundThreadAffinitizedObject, INavigationBarController + internal partial class NavigationBarController : ForegroundThreadAffinitizedObject, IDisposable { private readonly INavigationBarPresenter _presenter; private readonly ITextBuffer _subjectBuffer; @@ -132,6 +132,9 @@ public NavigationBarController( TaggerEventSources.OnWorkspaceRegistrationChanged(subjectBuffer)); _eventSource.Changed += OnEventSourceChanged; _eventSource.Connect(); + + // Kick off initial work to populate the navbars + StartModelUpdateAndSelectedItemUpdateTasks(); } private void OnEventSourceChanged(object? sender, TaggerEventArgs e) @@ -139,7 +142,7 @@ private void OnEventSourceChanged(object? sender, TaggerEventArgs e) StartModelUpdateAndSelectedItemUpdateTasks(); } - public void Disconnect() + void IDisposable.Dispose() { AssertIsForeground(); @@ -160,11 +163,6 @@ public void Disconnect() _cancellationTokenSource.Cancel(); } - public void SetWorkspace(Workspace? newWorkspace) - { - StartModelUpdateAndSelectedItemUpdateTasks(); - } - private void StartModelUpdateAndSelectedItemUpdateTasks() { // If we disconnected already, just disregard diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarControllerFactoryService.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarControllerFactoryService.cs index 0c4460514dd88..5f7166993c57f 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarControllerFactoryService.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarControllerFactoryService.cs @@ -34,7 +34,7 @@ public NavigationBarControllerFactoryService( _asyncListener = listenerProvider.GetListener(FeatureAttribute.NavigationBar); } - public INavigationBarController CreateController(INavigationBarPresenter presenter, ITextBuffer textBuffer) + public IDisposable CreateController(INavigationBarPresenter presenter, ITextBuffer textBuffer) { return new NavigationBarController( _threadingContext, diff --git a/src/EditorFeatures/Test2/NavigationBar/NavigationBarPresenterTests.vb b/src/EditorFeatures/Test2/NavigationBar/NavigationBarPresenterTests.vb index cfb2f43dd403a..d11df11f74712 100644 --- a/src/EditorFeatures/Test2/NavigationBar/NavigationBarPresenterTests.vb +++ b/src/EditorFeatures/Test2/NavigationBar/NavigationBarPresenterTests.vb @@ -117,8 +117,6 @@ class C Dim controllerFactory = workspace.GetService(Of INavigationBarControllerFactoryService)() Dim controller = controllerFactory.CreateController(mockPresenter, baseDocument.GetTextBuffer()) - controller.SetWorkspace(workspace) - Dim listenerProvider = workspace.ExportProvider.GetExport(Of IAsynchronousOperationListenerProvider).Value Dim workspaceWaiter = listenerProvider.GetWaiter(FeatureAttribute.Workspace) Dim navbarWaiter = listenerProvider.GetWaiter(FeatureAttribute.NavigationBar) @@ -186,8 +184,6 @@ End Class Dim controllerFactory = workspace.GetService(Of INavigationBarControllerFactoryService)() Dim controller = controllerFactory.CreateController(mockPresenter, baseDocument.GetTextBuffer()) - controller.SetWorkspace(workspace) - Dim listenerProvider = workspace.ExportProvider.GetExport(Of IAsynchronousOperationListenerProvider).Value Dim workspaceWaiter = listenerProvider.GetWaiter(FeatureAttribute.Workspace) Dim navbarWaiter = listenerProvider.GetWaiter(FeatureAttribute.NavigationBar) @@ -324,7 +320,6 @@ End Class Dim controllerFactory = workspace.GetService(Of INavigationBarControllerFactoryService)() Dim controller = controllerFactory.CreateController(mockPresenter, document.GetTextBuffer()) - controller.SetWorkspace(workspace) mockPresenter.RaiseDropDownFocused() Assert.Equal("VBProj", projectName) diff --git a/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs index 390154eb27916..4e031801aadc8 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs @@ -97,7 +97,7 @@ public async Task TestNoDocumentDiagnosticsForOpenFilesIfDefaultAndFeatureFlagOf var testExperimentationService = (TestExperimentationService)testLspServer.TestWorkspace.Services.GetRequiredService(); testExperimentationService.SetExperimentOption(WellKnownExperimentNames.LspPullDiagnosticsFeatureFlag, false); - var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document); + var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document.GetURI()); Assert.Empty(results.Single().Diagnostics); } @@ -116,7 +116,7 @@ public async Task TestDocumentDiagnosticsForOpenFilesIfDefaultAndFeatureFlagOn() var testExperimentationService = (TestExperimentationService)testLspServer.TestWorkspace.Services.GetRequiredService(); testExperimentationService.SetExperimentOption(WellKnownExperimentNames.LspPullDiagnosticsFeatureFlag, true); - var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document); + var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document.GetURI()); Assert.Equal("CS1513", results.Single().Diagnostics.Single().Code); } @@ -605,7 +605,7 @@ private TestLspServer CreateTestWorkspaceWithDiagnostics(string markup, Backgrou private TestLspServer CreateTestWorkspaceFromXml(string xmlMarkup, BackgroundAnalysisScope scope, bool pullDiagnostics = true) { var testLspServer = CreateXmlTestLspServer(xmlMarkup, out _); - InitializeDiagnostics(scope, testLspServer.TestWorkspace, pullDiagnostics); + InitializeDiagnostics(scope, testLspServer.TestWorkspace, pullDiagnostics ? DiagnosticMode.Pull : DiagnosticMode.Push); return testLspServer; } diff --git a/src/VisualStudio/Core/Def/Implementation/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs b/src/VisualStudio/Core/Def/Implementation/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs index 8254d8556ad95..accf5a857fbf3 100644 --- a/src/VisualStudio/Core/Def/Implementation/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs +++ b/src/VisualStudio/Core/Def/Implementation/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Diagnostics; using System.Linq; using System.Threading.Tasks; @@ -31,7 +32,7 @@ internal class VsCodeWindowManager : IVsCodeWindowManager, IVsCodeWindowEvents private readonly IThreadingContext _threadingContext; private readonly IAsynchronousOperationListener _asynchronousOperationListener; - private INavigationBarController? _navigationBarController; + private IDisposable? _navigationBarController; private IVsDropdownBarClient? _dropdownBarClient; private IOptionService? _optionService; private WorkspaceRegistration? _workspaceRegistration; @@ -73,8 +74,6 @@ private async Task UpdateWorkspaceAsync() // There's a new workspace, so make sure we unsubscribe from the old workspace option changes and subscribe to new. UpdateOptionChangedSource(_workspaceRegistration.Workspace); - _navigationBarController?.SetWorkspace(_workspaceRegistration.Workspace); - // Trigger a check to see if the dropdown should be added / removed now that the buffer is in a different workspace. AddOrRemoveDropdown(); } @@ -209,12 +208,11 @@ private void AdddropdownBar(IVsDropdownBarManager dropdownManager) var textBuffer = _languageService.EditorAdaptersFactoryService.GetDataBuffer(buffer); var controllerFactoryService = _languageService.Package.ComponentModel.GetService(); var newController = controllerFactoryService.CreateController(navigationBarClient, textBuffer); - newController.SetWorkspace(_workspaceRegistration?.Workspace); var hr = dropdownManager.AddDropdownBar(cCombos: 3, pClient: navigationBarClient); if (ErrorHandler.Failed(hr)) { - newController.Disconnect(); + newController.Dispose(); ErrorHandler.ThrowOnFailure(hr); } @@ -229,7 +227,7 @@ private void RemoveDropdownBar(IVsDropdownBarManager dropdownManager) { if (_navigationBarController != null) { - _navigationBarController.Disconnect(); + _navigationBarController.Dispose(); _navigationBarController = null; } From 4f4d5bf62a69d790c4b353f0561989d83333bd6b Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Mon, 28 Jun 2021 15:23:14 -0700 Subject: [PATCH 14/19] Update src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs --- .../TodoComments/VisualStudioTodoCommentsService.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs b/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs index 58b71b901be83..b2cf6682cd1de 100644 --- a/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs +++ b/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs @@ -40,8 +40,7 @@ internal class VisualStudioTodoCommentsService private readonly VisualStudioWorkspaceImpl _workspace; private readonly EventListenerTracker _eventListenerTracker; private readonly IAsynchronousOperationListener _asyncListener; - private readonly ConcurrentDictionary> _documentToInfos - = new(); + private readonly ConcurrentDictionary> _documentToInfos = new(); /// /// Remote service connection. Created on demand when we startup and then From 85e754968ca2540eccc55fab3a235acd37cc1adf Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 15:27:20 -0700 Subject: [PATCH 15/19] Wait for previous batch to complete --- .../Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs index 2878b59820cee..5de3d57c0f404 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs @@ -114,16 +114,18 @@ public void AddWork(IEnumerable items) // No in-flight task. Kick one off to process these messages a second from now. // We always attach the task to the previous one so that notifications to the ui // follow the same order as the notification the OOP server sent to us. - _updateTask = ContinueAfterDelay(); + _updateTask = ContinueAfterDelay(_updateTask); _taskInFlight = true; } } return; - async Task ContinueAfterDelay() + async Task ContinueAfterDelay(Task lastTask) { using var _ = _asyncListener.BeginAsyncOperation(nameof(AddWork)); + await lastTask.ConfigureAwait(false); + // Ensure that we always yield the current thread this is necessary for correctness as we are called // inside a lock that _taskInFlight to true. We must ensure that the work to process the next batch // must be on another thread that runs afterwards, can only grab the thread once we release it and will From b3341bec6a026b369a62f2a9ad31ae9f10771362 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 17:32:33 -0700 Subject: [PATCH 16/19] Use ValueTask --- .../SyntacticClassificationTaggerProvider.TagComputer.cs | 2 +- .../TaggerEventSources.WorkspaceChangedEventSource.cs | 2 +- ...tractAsynchronousTaggerProvider.TagSource_TagsChanged.cs | 6 +++--- .../Protocol/CustomProtocol/FindUsagesLSPContext.cs | 4 ++-- .../VisualStudioDesignerAttributeService.cs | 2 +- .../Contexts/AbstractTableDataSourceFindUsagesContext.cs | 4 ++-- .../LanguageClient/VisualStudioInProcLanguageServer.cs | 2 +- .../ProjectTelemetry/VisualStudioProjectTelemetryService.cs | 2 +- .../TodoComments/VisualStudioTodoCommentsService.cs | 4 ++-- .../Implementation/Workspace/SourceGeneratedFileManager.cs | 5 +++-- .../Core/Def/Packaging/PackageInstallerServiceFactory.cs | 6 +++--- .../Core/Impl/CodeModel/ProjectCodeModelFactory.cs | 2 +- .../Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs | 4 ++-- .../Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs | 6 +++--- .../Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs | 6 +++--- .../SQLite/v2/SQLitePersistentStorage_FlushWrites.cs | 4 ++-- 16 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/Classification/SyntacticClassificationTaggerProvider.TagComputer.cs b/src/EditorFeatures/Core/Implementation/Classification/SyntacticClassificationTaggerProvider.TagComputer.cs index d4d083cd5eaca..ef3376af8d8d1 100644 --- a/src/EditorFeatures/Core/Implementation/Classification/SyntacticClassificationTaggerProvider.TagComputer.cs +++ b/src/EditorFeatures/Core/Implementation/Classification/SyntacticClassificationTaggerProvider.TagComputer.cs @@ -265,7 +265,7 @@ private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs args) /// the editor. Calls to are serialized by /// so we don't need to worry about multiple calls to this happening concurrently. /// - private async Task ProcessChangesAsync(ImmutableArray snapshots, CancellationToken cancellationToken) + private async ValueTask ProcessChangesAsync(ImmutableArray snapshots, CancellationToken cancellationToken) { // We have potentially heard about several changes to the subject buffer. However // we only need to process the latest once. diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs index e265ecb2f3d4d..e00a949d931f0 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs @@ -29,7 +29,7 @@ public WorkspaceChangedEventSource( processBatchAsync: cancellationToken => { RaiseChanged(); - return Task.CompletedTask; + return ValueTaskFactory.CompletedTask; }, asyncListener, CancellationToken.None); diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs index 3437033b7809f..a7cb189fe08ba 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs @@ -42,12 +42,12 @@ private void OnTagsChangedForBuffer( } } - private Task ProcessTagsChangedAsync( + private ValueTask ProcessTagsChangedAsync( ImmutableArray snapshotSpans, CancellationToken cancellationToken) { var tagsChanged = this.TagsChanged; if (tagsChanged == null) - return Task.CompletedTask; + return ValueTaskFactory.CompletedTask; foreach (var collection in snapshotSpans) { @@ -65,7 +65,7 @@ private Task ProcessTagsChangedAsync( tagsChanged(this, new SnapshotSpanEventArgs(span)); } - return Task.CompletedTask; + return ValueTaskFactory.CompletedTask; } } } diff --git a/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs b/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs index a089ccc382b6d..364b2d7e942a5 100644 --- a/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs +++ b/src/Features/LanguageServer/Protocol/CustomProtocol/FindUsagesLSPContext.cs @@ -338,11 +338,11 @@ static ClassifiedTextRun[] GetClassifiedTextRuns( } } - private Task ReportReferencesAsync(ImmutableArray referencesToReport, CancellationToken cancellationToken) + private ValueTask ReportReferencesAsync(ImmutableArray referencesToReport, CancellationToken cancellationToken) { // We can report outside of the lock here since _progress is thread-safe. _progress.Report(referencesToReport.ToArray()); - return Task.CompletedTask; + return ValueTaskFactory.CompletedTask; } } } diff --git a/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs b/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs index 777bc632ee5b3..52df9e1854c5e 100644 --- a/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs +++ b/src/VisualStudio/Core/Def/Implementation/DesignerAttribute/VisualStudioDesignerAttributeService.cs @@ -146,7 +146,7 @@ public void StartScanningForDesignerAttributesInCurrentProcess(CancellationToken workspaceKinds: WorkspaceKind.Host)); } - private async Task NotifyProjectSystemAsync( + private async ValueTask NotifyProjectSystemAsync( ImmutableArray data, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs b/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs index a7109e8e6d1da..c2f51fd3e1b48 100644 --- a/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs +++ b/src/VisualStudio/Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs @@ -407,7 +407,7 @@ protected sealed override ValueTask ReportProgressAsync(int current, int maximum return default; } - private Task UpdateTableProgressAsync(ImmutableArray<(int current, int maximum)> nextBatch, CancellationToken _) + private ValueTask UpdateTableProgressAsync(ImmutableArray<(int current, int maximum)> nextBatch, CancellationToken _) { if (!nextBatch.IsEmpty) { @@ -427,7 +427,7 @@ private Task UpdateTableProgressAsync(ImmutableArray<(int current, int maximum)> _findReferencesWindow.SetProgress(current, maximum); } - return Task.CompletedTask; + return ValueTaskFactory.CompletedTask; } #endregion diff --git a/src/VisualStudio/Core/Def/Implementation/LanguageClient/VisualStudioInProcLanguageServer.cs b/src/VisualStudio/Core/Def/Implementation/LanguageClient/VisualStudioInProcLanguageServer.cs index ea964c8b3a819..522f8835ee73b 100644 --- a/src/VisualStudio/Core/Def/Implementation/LanguageClient/VisualStudioInProcLanguageServer.cs +++ b/src/VisualStudio/Core/Def/Implementation/LanguageClient/VisualStudioInProcLanguageServer.cs @@ -204,7 +204,7 @@ private void DiagnosticService_DiagnosticsUpdated(Solution? solution, DocumentId => Uri.Compare(uri1, uri2, UriComponents.AbsoluteUri, UriFormat.SafeUnescaped, StringComparison.OrdinalIgnoreCase)); // internal for testing purposes - internal async Task ProcessDiagnosticUpdatedBatchAsync( + internal async ValueTask ProcessDiagnosticUpdatedBatchAsync( IDiagnosticService? diagnosticService, ImmutableArray documentIds, CancellationToken cancellationToken) { if (diagnosticService == null) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs b/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs index ca9c89316521a..f29c8a9d05d17 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectTelemetry/VisualStudioProjectTelemetryService.cs @@ -120,7 +120,7 @@ private async Task StartWorkerAsync() cancellationToken).ConfigureAwait(false); } - private async Task NotifyTelemetryServiceAsync( + private async ValueTask NotifyTelemetryServiceAsync( ImmutableArray infos, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs b/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs index b2cf6682cd1de..4463117ab20ba 100644 --- a/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs +++ b/src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs @@ -145,7 +145,7 @@ private void ComputeTodoCommentsInCurrentProcess(CancellationToken cancellationT workspaceKinds: WorkspaceKind.Host)); } - private Task ProcessTodoCommentInfosAsync( + private ValueTask ProcessTodoCommentInfosAsync( ImmutableArray docAndCommentsArray, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); @@ -184,7 +184,7 @@ private Task ProcessTodoCommentInfosAsync( } } - return Task.CompletedTask; + return ValueTaskFactory.CompletedTask; } private void AddFilteredInfos( diff --git a/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs b/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs index 708ce56de7ccc..25309ad0dfa83 100644 --- a/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs +++ b/src/VisualStudio/Core/Def/Implementation/Workspace/SourceGeneratedFileManager.cs @@ -9,6 +9,7 @@ using System.IO; using System.Linq; using System.Threading; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Host.Mef; @@ -191,7 +192,7 @@ void IRunningDocumentTableEventListener.OnOpenDocument(string moniker, ITextBuff openFile = new OpenSourceGeneratedFile(this, textBuffer, _visualStudioWorkspace, documentIdentity, _threadingContext); _openFiles.Add(moniker, openFile); - _threadingContext.JoinableTaskFactory.Run(() => openFile.RefreshFileAsync(CancellationToken.None)); + _threadingContext.JoinableTaskFactory.Run(() => openFile.RefreshFileAsync(CancellationToken.None).AsTask()); // Update the RDT flags to ensure the file can't be saved or appears in any MRUs as it's a temporary generated file name. var cookie = ((IVsRunningDocumentTable4)_runningDocumentTable).GetDocumentCookie(moniker); @@ -321,7 +322,7 @@ public void Dispose() private string GeneratorDisplayName => _documentIdentity.GeneratorTypeName; - public async Task RefreshFileAsync(CancellationToken cancellationToken) + public async ValueTask RefreshFileAsync(CancellationToken cancellationToken) { SourceText? generatedSource = null; var project = _workspace.CurrentSolution.GetProject(_documentIdentity.DocumentId.ProjectId); diff --git a/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs b/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs index bfa8dd1049081..0a8892990afed 100644 --- a/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs +++ b/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs @@ -440,7 +440,7 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) _workQueue.AddWork((solutionChanged, changedProject)); } - private Task ProcessWorkQueueAsync( + private ValueTask ProcessWorkQueueAsync( ImmutableArray<(bool solutionChanged, ProjectId? changedProject)> workQueue, CancellationToken cancellationToken) { ThisCanBeCalledOnAnyThread(); @@ -449,13 +449,13 @@ private Task ProcessWorkQueueAsync( // If we've been disconnected, then there's no point proceeding. if (_workspace == null || !IsEnabled) - return Task.CompletedTask; + return ValueTaskFactory.CompletedTask; return ProcessWorkQueueWorkerAsync(workQueue, cancellationToken); } [MethodImpl(MethodImplOptions.NoInlining)] - private async Task ProcessWorkQueueWorkerAsync( + private async ValueTask ProcessWorkQueueWorkerAsync( ImmutableArray<(bool solutionChanged, ProjectId? changedProject)> workQueue, CancellationToken cancellationToken) { ThisCanBeCalledOnAnyThread(); diff --git a/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs b/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs index 06a6f7fc6a383..1a545d92168cc 100644 --- a/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs +++ b/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs @@ -69,7 +69,7 @@ public ProjectCodeModelFactory( internal IAsynchronousOperationListener Listener { get; } - private async Task ProcessNextDocumentBatchAsync( + private async ValueTask ProcessNextDocumentBatchAsync( ImmutableArray documentIds, CancellationToken cancellationToken) { // This logic preserves the previous behavior we had with IForegroundNotificationService. diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs index 7eb26a198630d..a5ecc1df789f5 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`0.cs @@ -16,14 +16,14 @@ internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue { public AsyncBatchingWorkQueue( TimeSpan delay, - Func processBatchAsync, + Func processBatchAsync, IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) : base(delay, Convert(processBatchAsync), EqualityComparer.Default, asyncListener, cancellationToken) { } - private static Func, CancellationToken, Task> Convert(Func processBatchAsync) + private static Func, CancellationToken, ValueTask> Convert(Func processBatchAsync) => (items, ct) => processBatchAsync(ct); public void AddWork() diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs index ec22008f93e9a..5e17e3090a256 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`1.cs @@ -16,7 +16,7 @@ internal class AsyncBatchingWorkQueue : AsyncBatchingWorkQueue, CancellationToken, Task> processBatchAsync, + Func, CancellationToken, ValueTask> processBatchAsync, IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) : this(delay, @@ -29,7 +29,7 @@ public AsyncBatchingWorkQueue( public AsyncBatchingWorkQueue( TimeSpan delay, - Func, CancellationToken, Task> processBatchAsync, + Func, CancellationToken, ValueTask> processBatchAsync, IEqualityComparer? equalityComparer, IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) @@ -37,7 +37,7 @@ public AsyncBatchingWorkQueue( { } - private static Func, CancellationToken, Task> Convert(Func, CancellationToken, Task> processBatchAsync) + private static Func, CancellationToken, ValueTask> Convert(Func, CancellationToken, ValueTask> processBatchAsync) => async (items, ct) => { await processBatchAsync(items, ct).ConfigureAwait(false); diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs index 5de3d57c0f404..debca352c369c 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue`2.cs @@ -34,7 +34,7 @@ internal class AsyncBatchingWorkQueue /// /// Callback to actually perform the processing of the next batch of work. /// - private readonly Func, CancellationToken, Task> _processBatchAsync; + private readonly Func, CancellationToken, ValueTask> _processBatchAsync; private readonly IAsynchronousOperationListener _asyncListener; private readonly CancellationToken _cancellationToken; @@ -76,7 +76,7 @@ internal class AsyncBatchingWorkQueue public AsyncBatchingWorkQueue( TimeSpan delay, - Func, CancellationToken, Task> processBatchAsync, + Func, CancellationToken, ValueTask> processBatchAsync, IEqualityComparer? equalityComparer, IAsynchronousOperationListener asyncListener, CancellationToken cancellationToken) @@ -159,7 +159,7 @@ private void AddItemsToBatch(IEnumerable items) } } - private Task ProcessNextBatchAsync(CancellationToken cancellationToken) + private ValueTask ProcessNextBatchAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); return _processBatchAsync(GetNextBatchAndResetQueue(), _cancellationToken); diff --git a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs index 4772d0f77b330..4389e046bda35 100644 --- a/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs +++ b/src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage_FlushWrites.cs @@ -23,12 +23,12 @@ private void EnqueueFlushTask() _flushQueue.AddWork(); } - private Task FlushInMemoryDataToDiskIfNotShutdownAsync(CancellationToken cancellationToken) + private async ValueTask FlushInMemoryDataToDiskIfNotShutdownAsync(CancellationToken cancellationToken) { // When we are asked to flush, go actually acquire the write-scheduler and perform the actual writes from // it. Note: this is only called max every FlushAllDelayMS. So we don't bother trying to avoid the delegate // allocation here. - return PerformWriteAsync(FlushInMemoryDataToDisk, cancellationToken); + await PerformWriteAsync(FlushInMemoryDataToDisk, cancellationToken).ConfigureAwait(false); } private Task FlushWritesOnCloseAsync() From 37c06e0d69e0dc03b7e7f2bfdcbae12b32ee9357 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Mon, 28 Jun 2021 17:33:27 -0700 Subject: [PATCH 17/19] Update src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs Co-authored-by: Sam Harwell --- .../Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs b/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs index b21e6c48620f9..5680a104422e0 100644 --- a/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs +++ b/src/VisualStudio/CSharp/Test/PersistentStorage/SQLiteV2PersistentStorageTests.cs @@ -28,7 +28,7 @@ internal override AbstractPersistentStorageService GetStorageService(OptionSet o options, exportProvider.GetExports().Single().Value, locationService, - exportProvider.GetExports().Single().Value.GetListener(FeatureAttribute.Workspace), + exportProvider.GetExports().Single().Value.GetListener(FeatureAttribute.PersistentStorage), faultInjector); [Fact] From 0d526efd60af10d15664494a7fabe6c233fbdb5b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 17:34:58 -0700 Subject: [PATCH 18/19] Add feature attribute --- src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs | 2 +- .../Core/Portable/Shared/TestHooks/FeatureAttribute.cs | 1 + .../Portable/Storage/DesktopPersistenceStorageServiceFactory.cs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs b/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs index fd2ad0bd30745..9dc2949a346a5 100644 --- a/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs +++ b/src/Tools/IdeBenchmarks/SQLitePersistentStorageBenchmark.cs @@ -71,7 +71,7 @@ public void GlobalSetup() var connectionPoolService = _workspace.ExportProvider.GetExportedValue(); _storageService = new SQLitePersistentStorageService( _workspace.Options, connectionPoolService, new LocationService(), - _workspace.ExportProvider.GetExportedValue().GetListener(FeatureAttribute.Workspace)); + _workspace.ExportProvider.GetExportedValue().GetListener(FeatureAttribute.PersistentStorage)); var solution = _workspace.CurrentSolution; _storage = _storageService.GetStorageWorkerAsync(_workspace, SolutionKey.ToSolutionKey(solution), solution, CancellationToken.None).AsTask().GetAwaiter().GetResult(); diff --git a/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs b/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs index 9d65a42def7fd..c1d410a65b6b0 100644 --- a/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs +++ b/src/Workspaces/Core/Portable/Shared/TestHooks/FeatureAttribute.cs @@ -34,6 +34,7 @@ internal static class FeatureAttribute public const string NavigationBar = nameof(NavigationBar); public const string Outlining = nameof(Outlining); public const string PackageInstaller = nameof(PackageInstaller); + public const string PersistentStorage = nameof(PersistentStorage); public const string ReferenceHighlighting = nameof(ReferenceHighlighting); public const string Rename = nameof(Rename); public const string RenameTracking = nameof(RenameTracking); diff --git a/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs b/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs index dd6743b406e63..b912912731405 100644 --- a/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs +++ b/src/Workspaces/Core/Portable/Storage/DesktopPersistenceStorageServiceFactory.cs @@ -46,7 +46,7 @@ public DesktopPersistenceStorageServiceFactory( IAsynchronousOperationListenerProvider asyncOperationListenerProvider) { _connectionPoolService = connectionPoolService; - _asyncListener = asyncOperationListenerProvider.GetListener(FeatureAttribute.Workspace); + _asyncListener = asyncOperationListenerProvider.GetListener(FeatureAttribute.PersistentStorage); } public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) From 6c5151bf845d22b42d957faf2f0384f98920d47d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 28 Jun 2021 17:40:57 -0700 Subject: [PATCH 19/19] Simplify --- .../NavigationBar/NavigationBarController.cs | 18 +++++------------- ...NavigationBarController_ModelComputation.cs | 13 +++++-------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs index 96b5cd6f3189b..e43ae4c6a9b80 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController.cs @@ -40,11 +40,6 @@ internal partial class NavigationBarController : ForegroundThreadAffinitizedObje private bool _disconnected = false; - /// - /// The last fully computed model. - /// - private NavigationBarModel _model_OnlyAccessOnUIThread; - /// /// Latest model and selected items produced once completes and presents the /// single item to the view. These can then be read in when the dropdown is expanded and we want to show all @@ -70,14 +65,13 @@ internal partial class NavigationBarController : ForegroundThreadAffinitizedObje /// compute the model once for every batch. The bool type parameter isn't used, but is provided as this /// type is generic. /// - private readonly AsyncBatchingWorkQueue _computeModelQueue; + private readonly AsyncBatchingWorkQueue _computeModelQueue; /// /// Queue to batch up work to do to determine the selected item. Used so we can batch up a lot of events and - /// only compute the selected item once for every batch. The bool type parameter isn't used, but is - /// provided as this type is generic. + /// only compute the selected item once for every batch. /// - private readonly AsyncBatchingWorkQueue _selectItemQueue; + private readonly AsyncBatchingWorkQueue _selectItemQueue; public NavigationBarController( IThreadingContext threadingContext, @@ -92,17 +86,16 @@ public NavigationBarController( _uiThreadOperationExecutor = uiThreadOperationExecutor; _asyncListener = asyncListener; - _computeModelQueue = new AsyncBatchingWorkQueue( + _computeModelQueue = new AsyncBatchingWorkQueue( TimeSpan.FromMilliseconds(TaggerConstants.ShortDelay), ComputeModelAndSelectItemAsync, EqualityComparer.Default, asyncListener, _cancellationTokenSource.Token); - _selectItemQueue = new AsyncBatchingWorkQueue( + _selectItemQueue = new AsyncBatchingWorkQueue( TimeSpan.FromMilliseconds(TaggerConstants.NearImmediateDelay), SelectItemAsync, - EqualityComparer.Default, asyncListener, _cancellationTokenSource.Token); @@ -115,7 +108,6 @@ public NavigationBarController( // Initialize the tasks to be an empty model so we never have to deal with a null case. _latestModelAndSelectedInfo_OnlyAccessOnUIThread.model = new(ImmutableArray.Empty, itemService: null!); _latestModelAndSelectedInfo_OnlyAccessOnUIThread.selectedInfo = new(typeItem: null, memberItem: null); - _model_OnlyAccessOnUIThread = _latestModelAndSelectedInfo_OnlyAccessOnUIThread.model; // Use 'compilation available' as that may produce different results from the initial 'frozen partial' // snapshot we use. diff --git a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs index daf53af1493a6..2e3f26f6f2fcd 100644 --- a/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs +++ b/src/EditorFeatures/Core/Implementation/NavigationBar/NavigationBarController_ModelComputation.cs @@ -25,7 +25,7 @@ internal partial class NavigationBarController /// /// Starts a new task to compute the model based on the current text. /// - private async Task ComputeModelAndSelectItemAsync(ImmutableArray unused, CancellationToken cancellationToken) + private async ValueTask ComputeModelAndSelectItemAsync(ImmutableArray unused, CancellationToken cancellationToken) { // Jump back to the UI thread to determine what snapshot the user is processing. await this.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); @@ -39,12 +39,10 @@ private async Task ComputeModelAndSelectItemAsync(ImmutableArray unused, C await this.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); - _model_OnlyAccessOnUIThread = model; - // Now, enqueue work to select the right item in this new model. StartSelectedItemUpdateTask(); - return; + return model; static async Task ComputeModelAsync(ITextSnapshot textSnapshot, CancellationToken cancellationToken) { @@ -78,10 +76,10 @@ static async Task ComputeModelAsync(ITextSnapshot textSnapsh private void StartSelectedItemUpdateTask() { // 'true' value is unused. this just signals to the queue that we have work to do. - _selectItemQueue.AddWork(true); + _selectItemQueue.AddWork(); } - private async Task SelectItemAsync(ImmutableArray unused, CancellationToken cancellationToken) + private async ValueTask SelectItemAsync(CancellationToken cancellationToken) { // Switch to the UI so we can determine where the user is. await this.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); @@ -92,8 +90,7 @@ private async Task SelectItemAsync(ImmutableArray unused, CancellationToke return; // Ensure the latest model is computed and grab it while on the UI thread. - await _computeModelQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(true); - var model = _model_OnlyAccessOnUIThread; + var model = await _computeModelQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(true); // Jump back to the BG to do any expensive work walking the entire model await TaskScheduler.Default;