Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slow down the work navbar does, and cancel updating it when receiving a high volume of user events #73680

Merged
merged 5 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ internal partial class NavigationBarController : IDisposable

/// <summary>
/// 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 <c>bool</c> type parameter isn't used, but is provided as this
/// type is generic.
/// compute the model once for every batch.
/// </summary>
private readonly AsyncBatchingWorkQueue<bool, NavigationBarModel?> _computeModelQueue;
private readonly AsyncBatchingWorkQueue<VoidResult, NavigationBarModel?> _computeModelQueue;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switched to VoidResult to make it clear this value isn't used.


/// <summary>
/// Queue to batch up work to do to determine the selected item. Used so we can batch up a lot of events and
Expand Down Expand Up @@ -93,15 +92,15 @@ public NavigationBarController(
_uiThreadOperationExecutor = uiThreadOperationExecutor;
_asyncListener = asyncListener;

_computeModelQueue = new AsyncBatchingWorkQueue<bool, NavigationBarModel?>(
_computeModelQueue = new AsyncBatchingWorkQueue<VoidResult, NavigationBarModel?>(
DelayTimeSpan.Short,
ComputeModelAndSelectItemAsync,
EqualityComparer<bool>.Default,
EqualityComparer<VoidResult>.Default,
asyncListener,
_cancellationTokenSource.Token);

_selectItemQueue = new AsyncBatchingWorkQueue(
DelayTimeSpan.NearImmediate,
DelayTimeSpan.Short,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the cadence on this, couldn't observe any problem at this speed.

SelectItemAsync,
asyncListener,
_cancellationTokenSource.Token);
Expand Down Expand Up @@ -195,8 +194,7 @@ private void StartModelUpdateAndSelectedItemUpdateTasks()
if (_disconnected)
return;

// 'true' value is unused. this just signals to the queue that we have work to do.
_computeModelQueue.AddWork(true);
_computeModelQueue.AddWork(default(VoidResult));
}

private void OnCaretMovedOrActiveViewChanged(object? sender, EventArgs e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.Workspaces;
using Microsoft.VisualStudio.Threading;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.Implementation.NavigationBar;

Expand All @@ -22,7 +23,7 @@ internal partial class NavigationBarController
/// <summary>
/// Starts a new task to compute the model based on the current text.
/// </summary>
private async ValueTask<NavigationBarModel?> ComputeModelAndSelectItemAsync(ImmutableSegmentedList<bool> unused, CancellationToken cancellationToken)
private async ValueTask<NavigationBarModel?> ComputeModelAndSelectItemAsync(ImmutableSegmentedList<VoidResult> _, CancellationToken cancellationToken)
{
// Jump back to the UI thread to determine what snapshot the user is processing.
await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable();
Expand Down Expand Up @@ -98,8 +99,8 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
/// </summary>
private void StartSelectedItemUpdateTask()
{
// 'true' value is unused. this just signals to the queue that we have work to do.
_selectItemQueue.AddWork();
// Cancel any in flight work. This way we don't update until a short lull after the last user event we received.
_selectItemQueue.AddWork(cancelExistingWork: true);
}

private async ValueTask SelectItemAsync(CancellationToken cancellationToken)
Expand Down
Loading