Skip to content

Commit

Permalink
Merge pull request #73681 from CyrusNajmabadi/lessUIHops
Browse files Browse the repository at this point in the history 
Avoid having to go back to the UI thread in the navbar code
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi committed May 24, 2024
2 parents ec93266 + d37015e commit 2f4fdee
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 92 deletions.
103 changes: 60 additions & 43 deletions src/EditorFeatures/Core/NavigationBar/NavigationBarController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ internal partial class NavigationBarController : IDisposable
private bool _disconnected = false;

/// <summary>
/// The last full information we have presented. If we end up wanting to present the same thing again, we can
/// just skip doing that as the UI will already know about this.
/// The last full information we have presented. If we end up wanting to present the same thing again, we can just
/// skip doing that as the UI will already know about this. This is only ever read or written from <see
/// cref="_selectItemQueue"/>. So we don't need to worry about any synchronization over it.
/// </summary>
private (ImmutableArray<NavigationBarProjectItem> projectItems, NavigationBarProjectItem? selectedProjectItem, NavigationBarModel? model, NavigationBarSelectedTypeAndMember selectedInfo) _lastPresentedInfo;

Expand All @@ -66,10 +67,10 @@ internal partial class NavigationBarController : IDisposable
private readonly AsyncBatchingWorkQueue<VoidResult, NavigationBarModel?> _computeModelQueue;

/// <summary>
/// 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.
/// 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 value passed in is the last recorded caret position.
/// </summary>
private readonly AsyncBatchingWorkQueue _selectItemQueue;
private readonly AsyncBatchingWorkQueue<int> _selectItemQueue;

/// <summary>
/// Whether or not the navbar is paused. We pause updates when documents become non-visible. See <see
Expand Down Expand Up @@ -99,7 +100,7 @@ public NavigationBarController(
asyncListener,
_cancellationTokenSource.Token);

_selectItemQueue = new AsyncBatchingWorkQueue(
_selectItemQueue = new AsyncBatchingWorkQueue<int>(
DelayTimeSpan.Short,
SelectItemAsync,
asyncListener,
Expand All @@ -126,9 +127,10 @@ public NavigationBarController(
{
threadingContext.ThrowIfNotOnUIThread();
// any time visibility changes, resume tagging on all taggers. Any non-visible taggers will pause
// themselves immediately afterwards.
Resume();
if (_visibilityTracker?.IsVisible(_subjectBuffer) is false)
Pause();
else
Resume();
};

// Register to hear about visibility changes so we can pause/resume this tagger.
Expand All @@ -138,6 +140,26 @@ public NavigationBarController(

// Kick off initial work to populate the navbars
StartModelUpdateAndSelectedItemUpdateTasks();

return;

void Pause()
{
_paused = true;
_eventSource.Pause();
}

void Resume()
{
// if we're not actually paused, no need to do anything.
if (_paused)
{
// Set us back to running, and kick off work to compute tags now that we're visible again.
_paused = false;
_eventSource.Resume();
StartModelUpdateAndSelectedItemUpdateTasks();
}
}
}

void IDisposable.Dispose()
Expand All @@ -161,26 +183,6 @@ void IDisposable.Dispose()
_cancellationTokenSource.Cancel();
}

private void Pause()
{
_threadingContext.ThrowIfNotOnUIThread();
_paused = true;
_eventSource.Pause();
}

private void Resume()
{
_threadingContext.ThrowIfNotOnUIThread();
// if we're not actually paused, no need to do anything.
if (_paused)
{
// Set us back to running, and kick off work to compute tags now that we're visible again.
_paused = false;
_eventSource.Resume();
StartModelUpdateAndSelectedItemUpdateTasks();
}
}

public TestAccessor GetTestAccessor() => new TestAccessor(this);

private void OnEventSourceChanged(object? sender, TaggerEventArgs e)
Expand All @@ -200,31 +202,46 @@ private void StartModelUpdateAndSelectedItemUpdateTasks()
private void OnCaretMovedOrActiveViewChanged(object? sender, EventArgs e)
{
_threadingContext.ThrowIfNotOnUIThread();
StartSelectedItemUpdateTask();

var caretPoint = GetCaretPoint();
if (caretPoint == null)
return;

// Cancel any in flight work. We're on the UI thread, so we know this is the latest position of the user, and that
// this should supersede any other selection work items.
_selectItemQueue.AddWork(caretPoint.Value, cancelExistingWork: true);
}

private void GetProjectItems(out ImmutableArray<NavigationBarProjectItem> projectItems, out NavigationBarProjectItem? selectedProjectItem)
private int? GetCaretPoint()
{
var documents = _subjectBuffer.CurrentSnapshot.GetRelatedDocumentsWithChanges();
if (!documents.Any())
{
projectItems = [];
selectedProjectItem = null;
return;
}
var currentView = _presenter.TryGetCurrentView();
return currentView?.GetCaretPoint(_subjectBuffer)?.Position;
}

projectItems = [.. documents.Select(d =>
new NavigationBarProjectItem(
private (ImmutableArray<NavigationBarProjectItem> projectItems, NavigationBarProjectItem? selectedProjectItem) GetProjectItems()
{
var textContainer = _subjectBuffer.AsTextContainer();

var documents = textContainer.GetRelatedDocuments();
if (documents.IsEmpty)
return ([], null);

var projectItems = documents
.Select(d => new NavigationBarProjectItem(
d.Project.Name,
d.Project.GetGlyph(),
workspace: d.Project.Solution.Workspace,
documentId: d.Id,
language: d.Project.Language)).OrderBy(projectItem => projectItem.Text)];
language: d.Project.Language))
.OrderBy(projectItem => projectItem.Text)
.ToImmutableArray();

var document = _subjectBuffer.AsTextContainer().GetOpenDocumentInCurrentContext();
selectedProjectItem = document != null
var document = textContainer.GetOpenDocumentInCurrentContext();
var selectedProjectItem = document != null
? projectItems.FirstOrDefault(p => p.Text == document.Project.Name) ?? projectItems.First()
: projectItems.First();

return (projectItems, selectedProjectItem);
}

private void OnItemSelected(object? sender, NavigationBarItemSelectedEventArgs e)
Expand Down
68 changes: 22 additions & 46 deletions src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.Workspaces;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Threading;
using Roslyn.Utilities;

Expand All @@ -34,16 +35,19 @@ internal partial class NavigationBarController
return null;

var textSnapshot = _subjectBuffer.CurrentSnapshot;
var caretPoint = GetCaretPoint();

// Ensure we switch to the threadpool before calling GetDocumentWithFrozenPartialSemantics. It ensures
// that any IO that performs is not potentially on the UI thread.
// 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;

var model = await ComputeModelAsync().ConfigureAwait(false);

// Now, enqueue work to select the right item in this new model.
if (model != null)
StartSelectedItemUpdateTask();
// Now, enqueue work to select the right item in this new model. Note: we don't want to cancel existing items in
// the queue as it may be the case that the user moved between us capturing the initial caret point and now, and
// we'd want the selection work we enqueued for that to take precedence over us.
if (model != null && caretPoint != null)
_selectItemQueue.AddWork(caretPoint.Value, cancelExistingWork: false);

return model;

Expand Down Expand Up @@ -94,55 +98,27 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
}
}

/// <summary>
/// Starts a new task to compute what item should be selected.
/// </summary>
private void StartSelectedItemUpdateTask()
{
// 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)
private async ValueTask SelectItemAsync(ImmutableSegmentedList<int> positions, CancellationToken cancellationToken)
{
// Switch to the UI so we can determine where the user is and determine the state the last time we updated
// the UI.
await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable();

// Cancellation exceptions are ignored in AsyncBatchingWorkQueue, so return without throwing if cancellation
// occurred while switching to the main thread.
if (cancellationToken.IsCancellationRequested)
return;
var lastCaretPosition = positions.Last();

await SelectItemWorkerAsync(cancellationToken).ConfigureAwait(true);
// Can grab this directly here as only this queue ever reads or writes to it.
var lastPresentedInfo = _lastPresentedInfo;

// Once we've computed and selected the latest navbar items, pause ourselves if we're no longer visible.
// That way we don't consume any machine resources that the user won't even notice.
if (_visibilityTracker?.IsVisible(_subjectBuffer) is false)
Pause();
}
// Make a task that waits indefinitely, or until the cancellation token is signaled.
var cancellationTriggeredTask = Task.Delay(-1, cancellationToken);

private async ValueTask SelectItemWorkerAsync(CancellationToken cancellationToken)
{
_threadingContext.ThrowIfNotOnUIThread();
// Get the task representing the computation of the model.
var modelTask = _computeModelQueue.WaitUntilCurrentBatchCompletesAsync();

var currentView = _presenter.TryGetCurrentView();
var caretPosition = currentView?.GetCaretPoint(_subjectBuffer);
if (!caretPosition.HasValue)
var completedTask = await Task.WhenAny(cancellationTriggeredTask, modelTask).ConfigureAwait(false);
if (completedTask == cancellationTriggeredTask)
return;

var position = caretPosition.Value.Position;
var lastPresentedInfo = _lastPresentedInfo;

// Jump back to the BG to do any expensive work walking the entire model
await TaskScheduler.Default;

// Ensure the latest model is computed.
var model = await _computeModelQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(true);

var currentSelectedItem = ComputeSelectedTypeAndMember(model, position, cancellationToken);
var model = await modelTask.ConfigureAwait(false);
var currentSelectedItem = ComputeSelectedTypeAndMember(model, lastCaretPosition, cancellationToken);

GetProjectItems(out var projectItems, out var selectedProjectItem);
var (projectItems, selectedProjectItem) = GetProjectItems();
if (Equals(model, lastPresentedInfo.model) &&
Equals(currentSelectedItem, lastPresentedInfo.selectedInfo) &&
Equals(selectedProjectItem, lastPresentedInfo.selectedProjectItem) &&
Expand Down
6 changes: 3 additions & 3 deletions src/EditorFeatures/Core/Tagging/ITaggerEventSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ internal interface ITaggerEventSource
void Disconnect();

/// <summary>
/// Pauses this event source and prevents it from firing the <see cref="Changed"/> event. Can be called many
/// times (but subsequence calls have no impact if already paused). Must be called on the UI thread.
/// Pauses this event source and prevents it from firing the <see cref="Changed"/> event. Can be called many times
/// (but subsequent calls have no impact if already paused). Must be called on the UI thread.
/// </summary>
void Pause();

/// <summary>
/// Resumes this event source and allows firing the <see cref="Changed"/> event. Can be called many times (but
/// subsequence calls have no impact if already resumed). Must be called on the UI thread.
/// subsequent calls have no impact if already resumed). Must be called on the UI thread.
/// </summary>
void Resume();

Expand Down

0 comments on commit 2f4fdee

Please sign in to comment.