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

Ensure taggers that compute frozen partial data eventually move to a 'correct' final state. #72878

Merged
merged 36 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
db6b65e
Initial work to support frozen partial tagging
CyrusNajmabadi Apr 4, 2024
0dab34b
Update tests
CyrusNajmabadi Apr 4, 2024
209a95c
Add tests
CyrusNajmabadi Apr 4, 2024
1411434
Update tests
CyrusNajmabadi Apr 4, 2024
be9305d
in progress
CyrusNajmabadi Apr 4, 2024
303982a
Cancellation series
CyrusNajmabadi Apr 4, 2024
06116b2
comments
CyrusNajmabadi Apr 4, 2024
b976155
move docs
CyrusNajmabadi Apr 4, 2024
4e3d729
Simplify
CyrusNajmabadi Apr 4, 2024
d869168
ASserts
CyrusNajmabadi Apr 4, 2024
7c8bf35
Update src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProv…
CyrusNajmabadi Apr 4, 2024
3ec3530
Simplify
CyrusNajmabadi Apr 4, 2024
f554737
Merge branch 'frozenPartialTagging' of https://github.com/CyrusNajmab…
CyrusNajmabadi Apr 4, 2024
24c34e5
move helpers
CyrusNajmabadi Apr 4, 2024
3de1764
indentatin
CyrusNajmabadi Apr 4, 2024
84c7aab
REmove type
CyrusNajmabadi Apr 4, 2024
5469dee
Docs
CyrusNajmabadi Apr 4, 2024
766085b
REmove unused usings
CyrusNajmabadi Apr 4, 2024
d9c8454
Consistentcy
CyrusNajmabadi Apr 4, 2024
02bd1ee
lint
CyrusNajmabadi Apr 4, 2024
4db1689
Merge remote-tracking branch 'upstream/main' into frozenPartialTagging
CyrusNajmabadi Apr 5, 2024
54485a1
Use disposal source
CyrusNajmabadi Apr 5, 2024
bb87d24
Switch to any
CyrusNajmabadi Apr 5, 2024
f4a0212
simplify logic
CyrusNajmabadi Apr 5, 2024
ba2fb3a
move option lower, and inline method
CyrusNajmabadi Apr 5, 2024
2717585
do the same for highlighting
CyrusNajmabadi Apr 5, 2024
5ae41b5
voidresult
CyrusNajmabadi Apr 5, 2024
05d0c6f
Update src/EditorFeatures/Core/ReferenceHighlighting/ReferenceHighlig…
CyrusNajmabadi Apr 5, 2024
df8d68a
voidresult
CyrusNajmabadi Apr 5, 2024
492c3fc
Merge branch 'frozenPartialTagging' of https://github.com/CyrusNajmab…
CyrusNajmabadi Apr 5, 2024
f31e40f
Simplify
CyrusNajmabadi Apr 5, 2024
6e8da4c
REvert
CyrusNajmabadi Apr 5, 2024
b78d0a8
revert
CyrusNajmabadi Apr 5, 2024
c58a604
remove redundant check
CyrusNajmabadi Apr 5, 2024
a9b19d3
Docs
CyrusNajmabadi Apr 5, 2024
0e8d413
Update src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProv…
CyrusNajmabadi Apr 5, 2024
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 @@ -45,7 +45,9 @@ private static async Task<IEnumerable<ITagSpan<BraceHighlightTag>>> ProduceTagsA

var context = new TaggerContext<BraceHighlightTag>(
buffer.CurrentSnapshot.GetRelatedDocumentsWithChanges().FirstOrDefault(),
buffer.CurrentSnapshot, new SnapshotPoint(buffer.CurrentSnapshot, position));
buffer.CurrentSnapshot,
frozenPartialSemantics: false,
new SnapshotPoint(buffer.CurrentSnapshot, position));
await producer.GetTestAccessor().ProduceTagsAsync(context);

return context.TagSpans;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ protected AbstractSemanticOrEmbeddedClassificationViewTaggerProvider(

protected sealed override TaggerDelay EventChangeDelay => TaggerDelay.Short;

/// <summary>
/// We support frozen partial semantics, so we can quickly get classification items without building SG docs. We
/// will still run a tagging pass after the frozen-pass where we run again on non-frozen docs.
/// </summary>
protected sealed override bool SupportsFrozenPartialSemantics => true;

protected sealed override ITaggerEventSource CreateEventSource(ITextView textView, ITextBuffer subjectBuffer)
{
this.ThreadingContext.ThrowIfNotOnUIThread();
Expand Down Expand Up @@ -92,7 +98,24 @@ protected sealed override Task ProduceTagsAsync(
if (isLspSemanticTokensEnabled)
return Task.CompletedTask;

var classificationOptions = _globalOptions.GetClassificationOptions(document.Project.Language);
// Because of the `SupportsFrozenPartialSemantics => true` property above, we'll do classification in two
// passes. In the first pass we do not block getting classifications on building the full compilation. This
// may take a significant amount of time and can cause a very latency sensitive operation (copying) to block the
// user while we wait on this work to happen.
//
// It's also a better experience to get classifications to the user faster versus waiting a potentially large
// amount of time waiting for all the compilation information to be built. For example, we can classify types
// that we've parsed in other files, or partially loaded from metadata, even if we're still parsing/loading. For
// cross language projects, this also produces semantic classifications more quickly as we do not have to wait
// on skeletons to be built.
//
// In the second pass though, we will go and do things without frozen-partial semantics, so that we do always
// snap to a final correct state. Note: the expensive second pass will be kicked down the road as new events
// come in to classify things.
var classificationOptions = _globalOptions.GetClassificationOptions(document.Project.Language) with
{
FrozenPartialSemantics = context.FrozenPartialSemantics,
};
return ClassificationUtilities.ProduceTagsAsync(
context, spanToTag, classificationService, _typeMap, classificationOptions, _type, cancellationToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,6 @@ public static async Task ProduceTagsAsync(
if (document == null)
return;

// Don't block getting classifications on building the full compilation. This may take a significant amount
// of time and can cause a very latency sensitive operation (copying) to block the user while we wait on this
// work to happen.
//
// It's also a better experience to get classifications to the user faster versus waiting a potentially
// large amount of time waiting for all the compilation information to be built. For example, we can
// classify types that we've parsed in other files, or partially loaded from metadata, even if we're still
// parsing/loading. For cross language projects, this also produces semantic classifications more quickly
// as we do not have to wait on skeletons to be built.

document = document.WithFrozenPartialSemantics(cancellationToken);
options = options with { ForceFrozenPartialSemanticsForCrossProcessOperations = true };

var classified = await TryClassifyContainingMemberSpanAsync(
context, document, spanToTag.SnapshotSpan, classificationService, typeMap, options, type, cancellationToken).ConfigureAwait(false);
if (classified)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ internal sealed partial class ReferenceHighlightingViewTaggerProvider(

protected override TaggerDelay EventChangeDelay => TaggerDelay.Medium;

/// <summary>
/// We support frozen partial semantics, so we can quickly get reference highlights without building SG docs. We
/// will still run a tagging pass after the frozen-pass where we run again on non-frozen docs.
/// </summary>
protected override bool SupportsFrozenPartialSemantics => true;

protected override ITaggerEventSource CreateEventSource(ITextView textView, ITextBuffer subjectBuffer)
{
// Note: we don't listen for OnTextChanged. Text changes to this buffer will get
Expand Down Expand Up @@ -129,7 +135,10 @@ protected override Task ProduceTagsAsync(
}

// Otherwise, we need to go produce all tags.
var options = _globalOptions.GetHighlightingOptions(document.Project.Language);
var options = _globalOptions.GetHighlightingOptions(document.Project.Language) with
{
FrozenPartialSemantics = context.FrozenPartialSemantics,
};
return ProduceTagsAsync(context, caretPosition, document, options, cancellationToken);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Tagging;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -75,11 +73,18 @@ private sealed partial class TagSource
private readonly AsyncBatchingWorkQueue<NormalizedSnapshotSpanCollection> _normalPriTagsChangedQueue;

/// <summary>
/// Boolean specifies if this is the initial set of tags being computed or not. This queue is used to batch
/// up event change notifications and only dispatch one recomputation every <see cref="EventChangeDelay"/>
/// to actually produce the latest set of tags.
/// This queue is used to batch up event change notifications and only dispatch one recomputation every <see
/// cref="EventChangeDelay"/> to actually produce the latest set of tags.
/// </summary>
private readonly AsyncBatchingWorkQueue<bool, VoidResult> _eventChangeQueue;
private readonly AsyncBatchingWorkQueue<TagSourceQueueItem> _eventChangeQueue;

/// <summary>
/// For taggers that support tagging frozen and non-frozen snapshots, this cancellation series controls the
/// non-frozen tagging pass. We want this to be separately cancellable so that if new events come in that we
/// cancel the expensive non-frozen tagging pass (which might be computing skeletons, SG docs, etc.), do the
/// next cheap frozen-tagging-pass, and then push the expensive-nonfrozen-tagging-pass to the end again.
/// </summary>
private readonly CancellationSeries _nonFrozenComputationCancellationSeries = new();
Copy link
Member Author

Choose a reason for hiding this comment

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

@sharwell for eyes.


#endregion

Expand Down Expand Up @@ -164,10 +169,10 @@ public TagSource(
//
// PERF: Use AsyncBatchingWorkQueue<bool, VoidResult> instead of AsyncBatchingWorkQueue<bool> because
// the latter has an async state machine that rethrows a very common cancellation exception.
_eventChangeQueue = new AsyncBatchingWorkQueue<bool, VoidResult>(
_eventChangeQueue = new AsyncBatchingWorkQueue<TagSourceQueueItem>(
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
dataSource.EventChangeDelay.ComputeTimeDelay(),
ProcessEventChangeAsync,
EqualityComparer<bool>.Default,
EqualityComparer<TagSourceQueueItem>.Default,
asyncListener,
_disposalTokenSource.Token);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Tagging;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
Expand Down Expand Up @@ -168,13 +169,93 @@ private void OnEventSourceChanged(object? _1, TaggerEventArgs _2)
=> EnqueueWork(highPriority: false);

private void EnqueueWork(bool highPriority)
=> _eventChangeQueue.AddWork(highPriority, _dataSource.CancelOnNewWork);
{

EnqueueWork(highPriority, _dataSource.SupportsFrozenPartialSemantics);
}

private void EnqueueWork(bool highPriority, bool frozenPartialSemantics)
Copy link
Member Author

Choose a reason for hiding this comment

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

@sharwell for eyes on thsi threading logic.

{
// Cancellation token if this expensive work that we want to be cancellable when cheap work comes in.
CancellationToken? nonFrozenComputationToken = null;

if (_dataSource.SupportsFrozenPartialSemantics)
{
// We do support frozen partial work. Cancel any expensive work in flight as new work has come in.
var nextToken = _nonFrozenComputationCancellationSeries.CreateNext();

// If this is new work *is* the expensive work, then use this token to allow cancellation of it when
// more new work comes in.
if (!frozenPartialSemantics)
nonFrozenComputationToken = nextToken;
}

private ValueTask<VoidResult> ProcessEventChangeAsync(ImmutableSegmentedList<bool> changes, CancellationToken cancellationToken)
_eventChangeQueue.AddWork(
new TagSourceQueueItem(highPriority, frozenPartialSemantics, nonFrozenComputationToken),
_dataSource.CancelOnNewWork);
}

private async ValueTask ProcessEventChangeAsync(
ImmutableSegmentedList<TagSourceQueueItem> changes, CancellationToken cancellationToken)
{
if (changes.Count == 0)
return;

// If any of the requests was high priority, then compute at that speed.
var highPriority = changes.Contains(true);
return new ValueTask<VoidResult>(RecomputeTagsAsync(highPriority, cancellationToken));
var highPriority = changes.Contains(x => x.HighPriority);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

// 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 = changes.Contains(t => t.FrozenPartialSemantics);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

if (frozenPartialSemantics)
{
// If we were asking for frozen partial semantics, then just proceed as normal, getting those tags
// quickly, but inaccurately.
await RecomputeTagsAsync(highPriority, frozenPartialSemantics: true, cancellationToken).ConfigureAwait(false);
}
else
{
if (!_dataSource.SupportsFrozenPartialSemantics)
{
// We're asking for normal expensive full tags, and this tagger doesn't support frozen partial
// tagging anyways, so just proceed as normal, asking for expensive tags.
await RecomputeTagsAsync(highPriority, frozenPartialSemantics: false, cancellationToken).ConfigureAwait(false);
}
else
{
// We're asking for expensive tags, and this tagger supports frozen partial tags. Kick off the work
Copy link
Member Author

Choose a reason for hiding this comment

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

@sharwell for extra attention here.

// to do this expensive tagging, but attach ourselves to the requested cancellation token so this
// expensive work can be canceled if new requests for frozen partial work come in.

// We must have at least one request asking for full semantics.
Contract.ThrowIfFalse(changes.Any(c => !c.FrozenPartialSemantics));

// All those requests should have cancellation tokens provided with them.
Contract.ThrowIfFalse(changes.Where(c => !c.FrozenPartialSemantics).All(c => c.NonFrozenComputationToken != null));
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

// Get the first non-cancelled token if present.
var nonFrozenComputationToken = changes.FirstOrNull(t => t.NonFrozenComputationToken?.IsCancellationRequested is false)?.NonFrozenComputationToken;
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

// If there is no non-frozen cancellation token that has not been triggered yet, then all non-frozen work
// has been canceled, and we can immediately bail out.
if (nonFrozenComputationToken is null)
return;

// Otherwise, link this token with the main queue cancellation token and do the actual tagging.
using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, nonFrozenComputationToken.Value);

// Need a dedicated try/catch here since we're operating on a different token than the queue's token.
try
{
await RecomputeTagsAsync(highPriority, frozenPartialSemantics, linkedTokenSource.Token).ConfigureAwait(false);
}
catch (Exception ex) when (FatalError.ReportAndPropagateUnlessCanceled(ex, linkedTokenSource.Token))
{
}
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

i felt like there could be a cleaner way of writing all this. but i couldn't figure it out :)

}

/// <summary>
Expand All @@ -191,11 +272,14 @@ private ValueTask<VoidResult> ProcessEventChangeAsync(ImmutableSegmentedList<boo
/// <param name="highPriority">
/// If this tagging request should be processed as quickly as possible with no extra delays added for it.
/// </param>
private async Task<VoidResult> RecomputeTagsAsync(bool highPriority, CancellationToken cancellationToken)
private async Task RecomputeTagsAsync(
bool highPriority,
bool frozenPartialSemantics,
CancellationToken cancellationToken)
{
await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable();
if (cancellationToken.IsCancellationRequested)
return default;
return;

// if we're tagging documents that are not visible, then introduce a long delay so that we avoid
// consuming machine resources on work the user isn't likely to see. ConfigureAwait(true) so that if
Expand All @@ -211,12 +295,12 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
_dataSource.ThreadingContext, _dataSource.AsyncListener, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(captureContext: true);

if (cancellationToken.IsCancellationRequested)
return default;
return;
}

_dataSource.ThreadingContext.ThrowIfNotOnUIThread();
if (cancellationToken.IsCancellationRequested)
return default;
return;

using (Logger.LogBlock(FunctionId.Tagger_TagSource_RecomputeTags, cancellationToken))
{
Expand All @@ -234,15 +318,22 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
await TaskScheduler.Default;

if (cancellationToken.IsCancellationRequested)
return default;
return;

if (frozenPartialSemantics)
{
spansToTag = spansToTag.SelectAsArray(ds => new DocumentSnapshotSpan(
ds.Document?.WithFrozenPartialSemantics(cancellationToken),
ds.SnapshotSpan));
}

// Create a context to store pass the information along and collect the results.
var context = new TaggerContext<TTag>(
oldState, spansToTag, caretPosition, textChangeRange, oldTagTrees);
oldState, frozenPartialSemantics, spansToTag, caretPosition, textChangeRange, oldTagTrees);
await ProduceTagsAsync(context, cancellationToken).ConfigureAwait(false);

if (cancellationToken.IsCancellationRequested)
return default;
return;

// Process the result to determine what changed.
var newTagTrees = ComputeNewTagTrees(oldTagTrees, context);
Expand All @@ -251,7 +342,7 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
// Then switch back to the UI thread to update our state and kick off the work to notify the editor.
await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable();
if (cancellationToken.IsCancellationRequested)
return default;
return;

// Once we assign our state, we're uncancellable. We must report the changed information
// to the editor. The only case where it's ok not to is if the tagger itself is disposed.
Expand All @@ -275,9 +366,16 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
// Once we've computed tags, pause ourselves if we're no longer visible. That way we don't consume any
// machine resources that the user won't even notice.
PauseIfNotVisible();
}

return default;
// If we were computing with frozen partial semantics here, enqueue work to compute *without* frozen
// partial snapshots so we move to accurate results shortly. Note: when the queue goes to process this
// message, if it sees any other events asking for frozen-partial-semantics, it will process in that
// mode again, kicking the can down the road to finally end with non-frozen-partial computation
if (frozenPartialSemantics)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
this.EnqueueWork(highPriority, frozenPartialSemantics: false);
}
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}
}

private ImmutableArray<DocumentSnapshotSpan> GetSpansAndDocumentsToTag()
Expand Down Expand Up @@ -560,8 +658,8 @@ private DiffResult ComputeDifference(
!this.CachedTagTrees.TryGetValue(buffer, out _))
{
// Compute this as a high priority work item to have the lease amount of blocking as possible.
_dataSource.ThreadingContext.JoinableTaskFactory.Run(() =>
this.RecomputeTagsAsync(highPriority: true, _disposalTokenSource.Token));
_dataSource.ThreadingContext.JoinableTaskFactory.Run(async () =>
await this.RecomputeTagsAsync(highPriority: true, _dataSource.SupportsFrozenPartialSemantics, _disposalTokenSource.Token).ConfigureAwait(false));
}

_firstTagsRequest = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ internal abstract partial class AbstractAsynchronousTaggerProvider<TTag> where T
/// </summary>
protected virtual bool CancelOnNewWork { get; }

/// <summary>
/// Whether or not this tagger would like to use frozen-partial snapshots to compute tags. TODO: doc more before submitting.
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
protected virtual bool SupportsFrozenPartialSemantics { get; }
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

protected virtual void BeforeTagsChanged(ITextSnapshot snapshot)
{
}
Expand Down
Loading