Skip to content

Commit

Permalink
Merge pull request #73786 from CyrusNajmabadi/frozenNavBar
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored May 31, 2024
2 parents 7185315 + f94a119 commit f1642d7
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ protected AbstractEditorNavigationBarItemService(IThreadingContext threadingCont
public async Task<ImmutableArray<NavigationBarItem>> GetItemsAsync(
Document document,
bool workspaceSupportsDocumentChanges,
bool forceFrozenPartialSemanticsForCrossProcessOperations,
bool frozenPartialSemantics,
ITextVersion textVersion,
CancellationToken cancellationToken)
{
var service = document.GetRequiredLanguageService<CodeAnalysis.NavigationBar.INavigationBarItemService>();
var items = await service.GetItemsAsync(document, workspaceSupportsDocumentChanges, forceFrozenPartialSemanticsForCrossProcessOperations, cancellationToken).ConfigureAwait(false);
var items = await service.GetItemsAsync(document, workspaceSupportsDocumentChanges, frozenPartialSemantics, cancellationToken).ConfigureAwait(false);
return items.SelectAsArray(v => (NavigationBarItem)new WrappedNavigationBarItem(textVersion, v));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal interface INavigationBarItemService : ILanguageService
Task<ImmutableArray<NavigationBarItem>> GetItemsAsync(
Document document,
bool workspaceSupportsDocumentChanges,
bool forceFrozenPartialSemanticsForCrossProcessOperations,
bool frozenPartialSemantics,
ITextVersion textVersion,
CancellationToken cancellationToken);
bool ShowItemGrayedIfNear(NavigationBarItem item);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public Task<ImmutableArray<NavigationBarItem>> GetItemsAsync(
Document document, ITextVersion textVersion, CancellationToken cancellationToken)
{
return ((INavigationBarItemService)this).GetItemsAsync(
document, workspaceSupportsDocumentChanges: true, forceFrozenPartialSemanticsForCrossProcessOperations: false, textVersion, cancellationToken);
document, workspaceSupportsDocumentChanges: true, frozenPartialSemantics: false, textVersion, cancellationToken);
}

async Task<ImmutableArray<NavigationBarItem>> INavigationBarItemService.GetItemsAsync(
Expand Down
26 changes: 21 additions & 5 deletions src/EditorFeatures/Core/NavigationBar/NavigationBarController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Data.Common;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -64,7 +65,15 @@ internal partial class NavigationBarController : IDisposable
/// 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.
/// </summary>
private readonly AsyncBatchingWorkQueue<VoidResult, NavigationBarModel?> _computeModelQueue;
private readonly AsyncBatchingWorkQueue<NavigationBarQueueItem, NavigationBarModel?> _computeModelQueue;

/// <summary>
/// This cancellation series controls the non-frozen nav-bar computation pass. We want this to be separately
/// cancellable so that if new events come in that we cancel the expensive non-frozen nav-bar pass (which might be
/// computing skeletons, SG docs, etc.), do the next cheap frozen-nav-bar-pass, and then push the
/// expensive-nonfrozen-nav-bar-pass to the end again.
/// </summary>
private readonly CancellationSeries _nonFrozenComputationCancellationSeries;

/// <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
Expand Down Expand Up @@ -92,11 +101,12 @@ public NavigationBarController(
_visibilityTracker = visibilityTracker;
_uiThreadOperationExecutor = uiThreadOperationExecutor;
_asyncListener = asyncListener;
_nonFrozenComputationCancellationSeries = new(_cancellationTokenSource.Token);

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

Expand Down Expand Up @@ -196,7 +206,13 @@ private void StartModelUpdateAndSelectedItemUpdateTasks()
if (_disconnected)
return;

_computeModelQueue.AddWork(default(VoidResult));
// Cancel any expensive, in-flight, nav-bar work as there's now a request to perform lightweight tagging. Note:
// intentionally ignoring the return value here. We're enqueuing normal work here, so it has no associated
// token with it.
_ = _nonFrozenComputationCancellationSeries.CreateNext();
_computeModelQueue.AddWork(
new NavigationBarQueueItem(FrozenPartialSemantics: true, NonFrozenComputationToken: null),
cancelExistingWork: true);
}

private void OnCaretMovedOrActiveViewChanged(object? sender, EventArgs e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Internal.Log;
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 @@ -24,7 +22,58 @@ 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<VoidResult> _, CancellationToken cancellationToken)
private async ValueTask<NavigationBarModel?> ComputeModelAndSelectItemAsync(
ImmutableSegmentedList<NavigationBarQueueItem> queueItems, CancellationToken cancellationToken)
{
// If any of the requests are for frozen partial, then we do compute with frozen partial semantics. We
// always want these "fast but inaccurate" passes to happen first. That pass will then enqueue the work
// to do the slow-but-accurate pass.
var frozenPartialSemantics = queueItems.Any(t => t.FrozenPartialSemantics);

if (!frozenPartialSemantics)
{
// We're asking for the expensive nav-bar-pass, Kick off the work to do that, but attach ourselves to the
// requested cancellation token so this expensive work can be canceled if new requests for frozen partial
// work come in.

// Since we're not frozen-partial, all requests must have an associated cancellation token. And all but
// the last *must* be already canceled (since each is canceled as new work is added).
Contract.ThrowIfFalse(queueItems.All(t => !t.FrozenPartialSemantics));
Contract.ThrowIfFalse(queueItems.All(t => t.NonFrozenComputationToken != null));
Contract.ThrowIfFalse(queueItems.Take(queueItems.Count - 1).All(t => t.NonFrozenComputationToken!.Value.IsCancellationRequested));

var lastNonFrozenComputationToken = queueItems[^1].NonFrozenComputationToken!.Value;

// Need a dedicated try/catch here since we're operating on a different token than the queue's token.
using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(lastNonFrozenComputationToken, cancellationToken);
try
{
return await ComputeModelAndSelectItemAsync(frozenPartialSemantics: false, linkedTokenSource.Token).ConfigureAwait(false);
}
catch (OperationCanceledException ex) when (ExceptionUtilities.IsCurrentOperationBeingCancelled(ex, linkedTokenSource.Token))
{
return null;
}
}
else
{
// Normal request to either compute nav-bar items using frozen partial semantics.
var model = await ComputeModelAndSelectItemAsync(frozenPartialSemantics: true, cancellationToken).ConfigureAwait(false);

// After that completes, enqueue work to compute *without* frozen partial snapshots so we move to accurate
// results shortly. Create and pass along a new cancellation token for this expensive work so that it can be
// canceled by future lightweight work.
_computeModelQueue.AddWork(new NavigationBarQueueItem(FrozenPartialSemantics: false, _nonFrozenComputationCancellationSeries.CreateNext(default)));

return model;
}
}

/// <summary>
/// Starts a new task to compute the model based on the current text.
/// </summary>
private async ValueTask<NavigationBarModel?> ComputeModelAndSelectItemAsync(
bool frozenPartialSemantics, 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 @@ -53,19 +102,13 @@ internal partial class NavigationBarController

async Task<NavigationBarModel?> ComputeModelAsync()
{
// 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 forceFrozenPartialSemanticsForCrossProcessOperations = true;

var workspace = textSnapshot.TextBuffer.GetWorkspace();
if (workspace is null)
return null;

var document = textSnapshot.AsText().GetDocumentWithFrozenPartialSemantics(cancellationToken);
var document = frozenPartialSemantics
? textSnapshot.AsText().GetDocumentWithFrozenPartialSemantics(cancellationToken)
: textSnapshot.AsText().GetOpenDocumentInCurrentContextWithChanges();
if (document == null)
return null;

Expand All @@ -90,7 +133,7 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
var items = await itemService.GetItemsAsync(
document,
workspace.CanApplyChange(ApplyChangesKind.ChangeDocument),
forceFrozenPartialSemanticsForCrossProcessOperations,
frozenPartialSemantics,
textSnapshot.Version,
cancellationToken).ConfigureAwait(false);
return new NavigationBarModel(itemService, items);
Expand Down
16 changes: 16 additions & 0 deletions src/EditorFeatures/Core/NavigationBar/NavigationBarQueueItem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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.Threading;

namespace Microsoft.CodeAnalysis.Editor.Implementation.NavigationBar;

/// <param name="FrozenPartialSemantics">Indicates if we should compute with frozen partial semantics or
/// not.</param>
/// <param name="NonFrozenComputationToken">If <paramref name="FrozenPartialSemantics"/> is false, then this is a
/// cancellation token that can cancel the expensive work being done if new frozen-partial work is
/// requested.</param>
internal readonly record struct NavigationBarQueueItem(
bool FrozenPartialSemantics,
CancellationToken? NonFrozenComputationToken);
8 changes: 4 additions & 4 deletions src/EditorFeatures/Test2/NavigationBar/TestHelpers.vb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.NavigationBar

Dim service = document.GetLanguageService(Of INavigationBarItemService)()
Dim actualItems = Await service.GetItemsAsync(
document, workspaceSupportsDocumentChanges:=True, forceFrozenPartialSemanticsForCrossProcessOperations:=False, snapshot.Version, Nothing)
document, workspaceSupportsDocumentChanges:=True, frozenPartialSemantics:=False, snapshot.Version, Nothing)

AssertEqual(expectedItems, actualItems, document.GetLanguageService(Of ISyntaxFactsService)().IsCaseSensitive)
End Using
Expand All @@ -58,7 +58,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.NavigationBar

Dim service = document.GetLanguageService(Of INavigationBarItemService)()
Dim items = Await service.GetItemsAsync(
document, workspaceSupportsDocumentChanges:=True, forceFrozenPartialSemanticsForCrossProcessOperations:=False, snapshot.Version, Nothing)
document, workspaceSupportsDocumentChanges:=True, frozenPartialSemantics:=False, snapshot.Version, Nothing)

Dim hostDocument = workspace.Documents.Single(Function(d) d.CursorPosition.HasValue)
Dim model As New NavigationBarModel(service, items)
Expand Down Expand Up @@ -92,7 +92,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.NavigationBar
Dim service = document.GetLanguageService(Of INavigationBarItemService)()

Dim items = Await service.GetItemsAsync(
document, workspaceSupportsDocumentChanges:=True, forceFrozenPartialSemanticsForCrossProcessOperations:=False, snapshot.Version, Nothing)
document, workspaceSupportsDocumentChanges:=True, frozenPartialSemantics:=False, snapshot.Version, Nothing)

Dim leftItem = items.Single(Function(i) i.Text = leftItemToSelectText)
Dim rightItem = selectRightItem(leftItem.ChildItems)
Expand Down Expand Up @@ -121,7 +121,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.NavigationBar

Dim service = DirectCast(sourceDocument.GetLanguageService(Of INavigationBarItemService)(), AbstractEditorNavigationBarItemService)
Dim items = Await service.GetItemsAsync(
sourceDocument, workspaceSupportsDocumentChanges:=True, forceFrozenPartialSemanticsForCrossProcessOperations:=False, snapshot.Version, Nothing)
sourceDocument, workspaceSupportsDocumentChanges:=True, frozenPartialSemantics:=False, snapshot.Version, Nothing)

Dim leftItem = items.Single(Function(i) i.Text = leftItemToSelectText)
Dim rightItem = leftItem.ChildItems.Single(Function(i) i.Text = rightItemToSelectText)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal abstract class AbstractNavigationBarItemService : INavigationBarItemSer
{
protected abstract Task<ImmutableArray<RoslynNavigationBarItem>> GetItemsInCurrentProcessAsync(Document document, bool supportsCodeGeneration, CancellationToken cancellationToken);

public async Task<ImmutableArray<RoslynNavigationBarItem>> GetItemsAsync(Document document, bool supportsCodeGeneration, bool forceFrozenPartialSemanticsForCrossProcessOperations, CancellationToken cancellationToken)
public async Task<ImmutableArray<RoslynNavigationBarItem>> GetItemsAsync(Document document, bool supportsCodeGeneration, bool frozenPartialSemantics, CancellationToken cancellationToken)
{
var client = await RemoteHostClient.TryGetClientAsync(document.Project, cancellationToken).ConfigureAwait(false);
if (client != null)
Expand All @@ -28,7 +28,7 @@ public async Task<ImmutableArray<RoslynNavigationBarItem>> GetItemsAsync(Documen
var documentId = document.Id;
var result = await client.TryInvokeAsync<IRemoteNavigationBarItemService, ImmutableArray<SerializableNavigationBarItem>>(
document.Project,
(service, solutionInfo, cancellationToken) => service.GetItemsAsync(solutionInfo, documentId, supportsCodeGeneration, forceFrozenPartialSemanticsForCrossProcessOperations, cancellationToken),
(service, solutionInfo, cancellationToken) => service.GetItemsAsync(solutionInfo, documentId, supportsCodeGeneration, frozenPartialSemantics, cancellationToken),
cancellationToken).ConfigureAwait(false);

return result.HasValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ namespace Microsoft.CodeAnalysis.NavigationBar;

internal interface INavigationBarItemService : ILanguageService
{
Task<ImmutableArray<RoslynNavigationBarItem>> GetItemsAsync(Document document, bool supportsCodeGeneration, bool forceFrozenPartialSemanticsForCrossProcessOperations, CancellationToken cancellationToken);
Task<ImmutableArray<RoslynNavigationBarItem>> GetItemsAsync(Document document, bool supportsCodeGeneration, bool frozenPartialSemantics, CancellationToken cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public async Task<object[]> HandleRequestAsync(RoslynDocumentSymbolParams reques
var clientCapabilities = context.GetRequiredClientCapabilities();

var navBarService = document.Project.Services.GetRequiredService<INavigationBarItemService>();
var navBarItems = await navBarService.GetItemsAsync(document, supportsCodeGeneration: false, forceFrozenPartialSemanticsForCrossProcessOperations: false, cancellationToken).ConfigureAwait(false);
var navBarItems = await navBarService.GetItemsAsync(document, supportsCodeGeneration: false, frozenPartialSemantics: false, cancellationToken).ConfigureAwait(false);
if (navBarItems.IsEmpty)
return [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public FSharpNavigationBarItemService(
public Task<ImmutableArray<NavigationBarItem>> GetItemsAsync(Document document, ITextVersion textVersion, CancellationToken cancellationToken)
{
return ((INavigationBarItemService)this).GetItemsAsync(
document, workspaceSupportsDocumentChanges: true, forceFrozenPartialSemanticsForCrossProcessOperations: false, textVersion, cancellationToken);
document, workspaceSupportsDocumentChanges: true, frozenPartialSemantics: false, textVersion, cancellationToken);
}

async Task<ImmutableArray<NavigationBarItem>> INavigationBarItemService.GetItemsAsync(
Expand Down
Loading

0 comments on commit f1642d7

Please sign in to comment.