From a625300da1c9c247b38a98f518cb455ac1c1d333 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 12:23:40 -0700 Subject: [PATCH 01/20] In progress --- ...actAsynchronousTaggerProvider.TagSource.cs | 38 ++---------- ...ousTaggerProvider.TagSource_ProduceTags.cs | 61 ++----------------- .../Core/Tagging/TaggerContext.cs | 16 +---- 3 files changed, 14 insertions(+), 101 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs index 8c93a15e8303..164054e6663b 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs @@ -88,6 +88,12 @@ private sealed partial class TagSource #endregion + #region Mutable state. Only accessed from _eventChangeQueue + + private object? _state_accessOnlyFromEventChangeQueueCallback; + + #endregion + #region Fields that can only be accessed from the foreground thread /// @@ -124,9 +130,7 @@ private sealed partial class TagSource /// /// accumulated text changes since last tag calculation /// - private TextChangeRange? _accumulatedTextChanges_doNotAccessDirectly; private ImmutableDictionary> _cachedTagTrees_doNotAccessDirectly = ImmutableDictionary.Create>(); - private object? _state_doNotAccessDirecty; /// /// Keep track of if we are processing the first request. If our provider returns @@ -336,21 +340,6 @@ private ITaggerEventSource CreateEventSource() return TaggerEventSources.Compose(optionChangedEventSources); } - private TextChangeRange? AccumulatedTextChanges - { - get - { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - return _accumulatedTextChanges_doNotAccessDirectly; - } - - set - { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - _accumulatedTextChanges_doNotAccessDirectly = value; - } - } - private ImmutableDictionary> CachedTagTrees { get @@ -366,21 +355,6 @@ private ImmutableDictionary> CachedTagTre } } - private object? State - { - get - { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - return _state_doNotAccessDirecty; - } - - set - { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - _state_doNotAccessDirecty = value; - } - } - private void RaiseTagsChanged(ITextBuffer buffer, DiffResult difference) { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index c40775baecbf..e00f8dfcb918 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -68,42 +68,6 @@ private void OnSubjectBufferChanged(object? _, TextContentChangedEventArgs e) { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); UpdateTagsForTextChange(e); - AccumulateTextChanges(e); - } - - private void AccumulateTextChanges(TextContentChangedEventArgs contentChanged) - { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - var contentChanges = contentChanged.Changes; - var count = contentChanges.Count; - - switch (count) - { - case 0: - return; - - case 1: - // PERF: Optimize for the simple case of typing on a line. - { - var c = contentChanges[0]; - var textChangeRange = new TextChangeRange(new TextSpan(c.OldSpan.Start, c.OldSpan.Length), c.NewLength); - this.AccumulatedTextChanges = this.AccumulatedTextChanges == null - ? textChangeRange - : this.AccumulatedTextChanges.Accumulate([textChangeRange]); - } - - break; - - default: - { - using var _ = ArrayBuilder.GetInstance(count, out var textChangeRanges); - foreach (var c in contentChanges) - textChangeRanges.Add(new TextChangeRange(new TextSpan(c.OldSpan.Start, c.OldSpan.Length), c.NewLength)); - - this.AccumulatedTextChanges = this.AccumulatedTextChanges.Accumulate(textChangeRanges); - break; - } - } } private void UpdateTagsForTextChange(TextContentChangedEventArgs e) @@ -279,9 +243,8 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( var spansToTag = GetSpansAndDocumentsToTag(); var caretPosition = _dataSource.GetCaretPoint(_textView, _subjectBuffer); var oldTagTrees = this.CachedTagTrees; - var oldState = this.State; + var oldState = _state_accessOnlyFromEventChangeQueueCallback; - var textChangeRange = this.AccumulatedTextChanges; var subjectBufferVersion = _subjectBuffer.CurrentSnapshot.Version.VersionNumber; await TaskScheduler.Default; @@ -298,7 +261,7 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( // Create a context to store pass the information along and collect the results. var context = new TaggerContext( - oldState, frozenPartialSemantics, spansToTag, caretPosition, textChangeRange, oldTagTrees); + oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees); await ProduceTagsAsync(context, cancellationToken).ConfigureAwait(false); if (cancellationToken.IsCancellationRequested) @@ -308,27 +271,15 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( var newTagTrees = ComputeNewTagTrees(oldTagTrees, context); var bufferToChanges = ProcessNewTagTrees(spansToTag, oldTagTrees, newTagTrees, cancellationToken); - // 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; - // 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. cancellationToken = CancellationToken.None; this.CachedTagTrees = newTagTrees; - this.State = context.State; - if (this._subjectBuffer.CurrentSnapshot.Version.VersionNumber == subjectBufferVersion) - { - // Only clear the accumulated text changes if the subject buffer didn't change during the - // tagging operation. Otherwise, it is impossible to know which changes occurred prior to the - // request to tag, and which ones occurred during the tagging itself. Since - // AccumulatedTextChanges is a conservative representation of the work that needs to be done, in - // the event this value is not cleared the only potential impact will be slightly more work - // being done during the next classification pass. - this.AccumulatedTextChanges = null; - } + + // Note: assigning to 'State' is completely safe. It is only ever read from the _eventChangeQueue + // serial callbacks on the threadpool. + _state_accessOnlyFromEventChangeQueueCallback = context.State; OnTagsChangedForBuffer(bufferToChanges, highPriority); diff --git a/src/EditorFeatures/Core/Tagging/TaggerContext.cs b/src/EditorFeatures/Core/Tagging/TaggerContext.cs index fab148c0c801..ba972866c541 100644 --- a/src/EditorFeatures/Core/Tagging/TaggerContext.cs +++ b/src/EditorFeatures/Core/Tagging/TaggerContext.cs @@ -8,14 +8,13 @@ using System.Threading; using Microsoft.CodeAnalysis.Collections; using Microsoft.CodeAnalysis.Editor.Shared.Tagging; -using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Tagging; namespace Microsoft.CodeAnalysis.Editor.Tagging; -internal class TaggerContext where TTag : ITag +internal sealed class TaggerContext where TTag : ITag { private readonly ImmutableDictionary> _existingTags; @@ -33,13 +32,6 @@ internal class TaggerContext where TTag : ITag public ImmutableArray SpansToTag { get; } public SnapshotPoint? CaretPosition { get; } - /// - /// The text that has changed between the last successful tagging and this new request to - /// produce tags. In order to be passed this value, - /// must be specified in . - /// - public TextChangeRange? TextChangeRange { get; } - /// /// The state of the tagger. Taggers can use this to keep track of information across calls /// to . Note: state will @@ -55,14 +47,12 @@ internal TaggerContext( Document document, ITextSnapshot snapshot, bool frozenPartialSemantics, - SnapshotPoint? caretPosition = null, - TextChangeRange? textChangeRange = null) + SnapshotPoint? caretPosition = null) : this( state: null, frozenPartialSemantics, [new DocumentSnapshotSpan(document, snapshot.GetFullSpan())], caretPosition, - textChangeRange, existingTags: null) { } @@ -72,14 +62,12 @@ internal TaggerContext( bool frozenPartialSemantics, ImmutableArray spansToTag, SnapshotPoint? caretPosition, - TextChangeRange? textChangeRange, ImmutableDictionary> existingTags) { this.State = state; this.FrozenPartialSemantics = frozenPartialSemantics; this.SpansToTag = spansToTag; this.CaretPosition = caretPosition; - this.TextChangeRange = textChangeRange; _spansTagged = spansToTag.SelectAsArray(ds => ds.SnapshotSpan); _existingTags = existingTags; From 45198ea33af7010d7841db60e0f978f43c6005a5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 12:28:10 -0700 Subject: [PATCH 02/20] More ui work --- .../AbstractAsynchronousTaggerProvider.TagSource.cs | 10 +++++++--- ...AsynchronousTaggerProvider.TagSource_ProduceTags.cs | 4 ---- ...AsynchronousTaggerProvider.TagSource_TagsChanged.cs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs index 164054e6663b..59e64e32969f 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs @@ -206,9 +206,13 @@ public TagSource( // Create the tagger-specific events that will cause the tagger to refresh. _eventSource = CreateEventSource(); - // any time visibility changes, resume tagging on all taggers. Any non-visible taggers will pause - // themselves immediately afterwards. - _onVisibilityChanged = () => ResumeIfVisible(); + // Any time visibility changes try to pause us if we're not visible, or resume us if we are. + _onVisibilityChanged = () => + { + _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); + PauseIfNotVisible(); + ResumeIfVisible(); + }; // Now hook up this tagger to all interesting events. Connect(); diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index e00f8dfcb918..31afc84c13f7 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -283,10 +283,6 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( OnTagsChangedForBuffer(bufferToChanges, highPriority); - // 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(); - // If we were computing with frozen partial semantics here, 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. diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs index 957730c5b3e0..a0862bffeebb 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_TagsChanged.cs @@ -8,7 +8,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Collections; -using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Text.Shared.Extensions; using Microsoft.VisualStudio.Text; using Roslyn.Utilities; @@ -24,7 +23,8 @@ private partial class TagSource private void OnTagsChangedForBuffer( ICollection> changes, bool highPriority) { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); + // Can be called from any thread. Just filters out changes that aren't for our buffer and adds to the right + // queue to actually notify interested parties. foreach (var change in changes) { From da8522cf296d7716f302acbf1d54d69c86084c8f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 12:47:34 -0700 Subject: [PATCH 03/20] Comments --- ...mbeddedClassificationViewTaggerProvider.cs | 68 ++++++++++--------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs index 96ea478c3b9c..24ca5dcbbf11 100644 --- a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs +++ b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs @@ -137,8 +137,9 @@ public async Task ProduceTagsAsync( if (document == null) return; + var currentSemanticVersion = await document.Project.GetDependentSemanticVersionAsync(cancellationToken).ConfigureAwait(false); var classified = await TryClassifyContainingMemberSpanAsync( - context, document, spanToTag.SnapshotSpan, classificationService, options, cancellationToken).ConfigureAwait(false); + context, document, spanToTag.SnapshotSpan, classificationService, options, currentSemanticVersion, cancellationToken).ConfigureAwait(false); if (classified) { return; @@ -147,7 +148,7 @@ public async Task ProduceTagsAsync( // We weren't able to use our specialized codepaths for semantic classifying. // Fall back to classifying the full span that was asked for. await ClassifySpansAsync( - context, document, spanToTag.SnapshotSpan, classificationService, options, cancellationToken).ConfigureAwait(false); + context, document, spanToTag.SnapshotSpan, classificationService, options, currentSemanticVersion, cancellationToken).ConfigureAwait(false); } private async Task TryClassifyContainingMemberSpanAsync( @@ -156,39 +157,39 @@ private async Task TryClassifyContainingMemberSpanAsync( SnapshotSpan snapshotSpan, IClassificationService classificationService, ClassificationOptions options, + VersionStamp currentSemanticVersion, CancellationToken cancellationToken) { - var range = context.TextChangeRange; - if (range == null) - { - // There was no text change range, we can't just reclassify a member body. - return false; - } - // there was top level edit, check whether that edit updated top level element if (!document.SupportsSyntaxTree) return false; - var lastSemanticVersion = (VersionStamp?)context.State; - if (lastSemanticVersion != null) - { - var currentSemanticVersion = await document.Project.GetDependentSemanticVersionAsync(cancellationToken).ConfigureAwait(false); - if (lastSemanticVersion.Value != currentSemanticVersion) - { - // A top level change was made. We can't perform this optimization. - return false; - } - } + // No cached state, so we can't check if the edits were just inside a member. + if (context.State is null) + return false; + + var (lastSemanticVersion, lastTextSnapshot) = ((VersionStamp, ITextSnapshot))context.State; + + // if a top level change was made. We can't perform this optimization. + if (lastSemanticVersion != currentSemanticVersion) + return false; var service = document.GetRequiredLanguageService(); - // perf optimization. Check whether all edits since the last update has happened within - // a member. If it did, it will find the member that contains the changes and only refresh - // that member. If possible, try to get a speculative binder to make things even cheaper. + // perf optimization. Check whether all edits since the last update has happened within a member. If it did, it + // will find the member that contains the changes and only refresh that member. If possible, try to get a + // speculative binder to make things even cheaper. + + var lastSourceText = lastTextSnapshot.AsText(); + var currentSourceText = snapshotSpan.Snapshot.AsText(); + + var textChangeRanges = lastSourceText.GetChangeRanges(currentSourceText); + var collapsedRange = TextChangeRange.Collapse(textChangeRanges); + + var changedSpan = new TextSpan(collapsedRange.Span.Start, collapsedRange.NewLength); var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var changedSpan = new TextSpan(range.Value.Span.Start, range.Value.NewLength); var member = service.GetContainingMemberDeclaration(root, changedSpan.Start); if (member == null || !member.FullSpan.Contains(changedSpan)) { @@ -221,7 +222,7 @@ private async Task TryClassifyContainingMemberSpanAsync( // re-classify only the member we're inside. await ClassifySpansAsync( - context, document, subSpanToTag, classificationService, options, cancellationToken).ConfigureAwait(false); + context, document, subSpanToTag, classificationService, options, currentSemanticVersion, cancellationToken).ConfigureAwait(false); return true; } @@ -231,6 +232,7 @@ private async Task ClassifySpansAsync( SnapshotSpan snapshotSpan, IClassificationService classificationService, ClassificationOptions options, + VersionStamp currentSemanticVersion, CancellationToken cancellationToken) { try @@ -243,29 +245,33 @@ private async Task ClassifySpansAsync( // that we preserve that same behavior in OOP if we end up computing the tags there. options = options with { FrozenPartialSemantics = context.FrozenPartialSemantics }; + var span = snapshotSpan.Span; + var snapshot = snapshotSpan.Snapshot; + if (_type == ClassificationType.Semantic) { await classificationService.AddSemanticClassificationsAsync( - document, snapshotSpan.Span.ToTextSpan(), options, classifiedSpans, cancellationToken).ConfigureAwait(false); + document, span.ToTextSpan(), options, classifiedSpans, cancellationToken).ConfigureAwait(false); } else if (_type == ClassificationType.EmbeddedLanguage) { await classificationService.AddEmbeddedLanguageClassificationsAsync( - document, snapshotSpan.Span.ToTextSpan(), options, classifiedSpans, cancellationToken).ConfigureAwait(false); + document, span.ToTextSpan(), options, classifiedSpans, cancellationToken).ConfigureAwait(false); } else { throw ExceptionUtilities.UnexpectedValue(_type); } - foreach (var span in classifiedSpans) - context.AddTag(ClassificationUtilities.Convert(_typeMap, snapshotSpan.Snapshot, span)); - - var version = await document.Project.GetDependentSemanticVersionAsync(cancellationToken).ConfigureAwait(false); + foreach (var classifiedSpan in classifiedSpans) + context.AddTag(ClassificationUtilities.Convert(_typeMap, snapshot, classifiedSpan)); // Let the context know that this was the span we actually tried to tag. context.SetSpansTagged([snapshotSpan]); - context.State = version; + + // Store teh semantic version and snapshot we used to produce these tags. We can use this in the future + // to try to limit what we classify, if all edits were made within a single member. + context.State = (currentSemanticVersion, snapshot); } } catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken)) From 594cf9555ebf45ddeaae0b6b776e870d84c313e8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 12:58:10 -0700 Subject: [PATCH 04/20] in progres --- ...ousTaggerProvider.TagSource_ProduceTags.cs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 31afc84c13f7..97b8d57bc09d 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -205,7 +205,7 @@ private async ValueTask ProcessEventChangeAsync( /// /// If this tagging request should be processed as quickly as possible with no extra delays added for it. /// - private async Task RecomputeTagsAsync( + private async Task>> RecomputeTagsAsync( bool highPriority, bool frozenPartialSemantics, CancellationToken cancellationToken) @@ -288,9 +288,9 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( // token for this expensive work so that it can be canceled by future lightweight work. if (frozenPartialSemantics) this.EnqueueWork(highPriority, frozenPartialSemantics: false, _nonFrozenComputationCancellationSeries.CreateNext(default)); - } - return default; + return newTagTrees; + } } private ImmutableArray GetSpansAndDocumentsToTag() @@ -564,23 +564,24 @@ private DiffResult ComputeDifference( if (_disposalTokenSource.Token.IsCancellationRequested) return null; - // If this is the first time we're being asked for tags, and we're a tagger that requires the initial - // tags be available synchronously on this call, and the computation of tags hasn't completed yet, then - // force the tags to be computed now on this thread. The singular use case for this is Outlining which - // needs those tags synchronously computed for things like Metadata-as-Source collapsing. + var tagTrees = this.CachedTagTrees; + + // If this is the first time we're being asked for tags, and we're a tagger that requires the initial tags + // be available synchronously on this call, and the computation of tags hasn't completed yet, then force the + // tags to be computed now on this thread. The singular use case for this is Outlining which needs those + // tags synchronously computed for things like Metadata-as-Source collapsing. if (_firstTagsRequest && _dataSource.ComputeInitialTagsSynchronously(buffer) && - !this.CachedTagTrees.TryGetValue(buffer, out _)) + !tagTrees.TryGetValue(buffer, out _)) { // Compute this as a high priority work item to have the lease amount of blocking as possible. - _dataSource.ThreadingContext.JoinableTaskFactory.Run(() => + tagTrees = _dataSource.ThreadingContext.JoinableTaskFactory.Run(() => this.RecomputeTagsAsync(highPriority: true, _dataSource.SupportsFrozenPartialSemantics, _disposalTokenSource.Token)); } _firstTagsRequest = false; - // We're on the UI thread, so it's safe to access these variables. - this.CachedTagTrees.TryGetValue(buffer, out var tags); + tagTrees.TryGetValue(buffer, out var tags); return tags; } From 96813b935c7df4b8d2bf68301875d5aa8bd272a4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 13:27:02 -0700 Subject: [PATCH 05/20] IN progress --- ...actAsynchronousTaggerProvider.TagSource.cs | 37 +++------ ...ousTaggerProvider.TagSource_ProduceTags.cs | 83 ++++++++++++------- 2 files changed, 66 insertions(+), 54 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs index 59e64e32969f..6e5e78348007 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs @@ -27,12 +27,12 @@ internal partial class AbstractAsynchronousTaggerProvider /// tagging infrastructure. It is the coordinator between s, /// s, and s. /// - /// The is the type that actually owns the - /// list of cached tags. When an says tags need to be recomputed, - /// the tag source starts the computation and calls to build - /// the new list of tags. When that's done, the tags are stored in . The - /// tagger, when asked for tags from the editor, then returns the tags that are stored in - /// + /// The is the type that actually owns the list of cached tags. When an says tags need to be recomputed, the tag source starts the computation and calls + /// to build the new list of tags. When + /// that's done, the tags are stored in . The tagger, when asked + /// for tags from the editor, then returns the tags that are stored in /// /// There is a one-to-many relationship between s /// and s. Special cases, like reference highlighting (which processes multiple @@ -86,6 +86,11 @@ private sealed partial class TagSource /// private readonly CancellationSeries _nonFrozenComputationCancellationSeries; + /// + /// The last tag trees that we computed per buffer. + /// + private ImmutableDictionary> _cachedTagTrees_mayChangeFromAnyThread = ImmutableDictionary>.Empty; + #endregion #region Mutable state. Only accessed from _eventChangeQueue @@ -127,11 +132,6 @@ private sealed partial class TagSource #region Mutable state. Can only be accessed from the foreground thread - /// - /// accumulated text changes since last tag calculation - /// - private ImmutableDictionary> _cachedTagTrees_doNotAccessDirectly = ImmutableDictionary.Create>(); - /// /// Keep track of if we are processing the first request. If our provider returns /// for , @@ -344,21 +344,6 @@ private ITaggerEventSource CreateEventSource() return TaggerEventSources.Compose(optionChangedEventSources); } - private ImmutableDictionary> CachedTagTrees - { - get - { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - return _cachedTagTrees_doNotAccessDirectly; - } - - set - { - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - _cachedTagTrees_doNotAccessDirectly = value; - } - } - private void RaiseTagsChanged(ITextBuffer buffer, DiffResult difference) { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 97b8d57bc09d..cb3228c9b15b 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -54,8 +54,8 @@ private void RemoveAllTags() { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - var oldTagTrees = this.CachedTagTrees; - this.CachedTagTrees = ImmutableDictionary>.Empty; + var oldTagTrees = Interlocked.Exchange( + ref _cachedTagTrees_mayChangeFromAnyThread, ImmutableDictionary>.Empty); var snapshot = _subjectBuffer.CurrentSnapshot; var oldTagTree = GetTagTree(snapshot, oldTagTrees); @@ -176,7 +176,8 @@ private async ValueTask ProcessEventChangeAsync( using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(lastNonFrozenComputationToken, cancellationToken); try { - return await RecomputeTagsAsync(highPriority, frozenPartialSemantics, linkedTokenSource.Token).ConfigureAwait(false); + await RecomputeTagsAsync(highPriority, frozenPartialSemantics, linkedTokenSource.Token).ConfigureAwait(false); + return default; } catch (OperationCanceledException ex) when (ExceptionUtilities.IsCurrentOperationBeingCancelled(ex, linkedTokenSource.Token)) { @@ -187,8 +188,31 @@ private async ValueTask ProcessEventChangeAsync( { // Normal request to either compute frozen partial tags, or compute normal tags in a tagger that does // *not* support frozen partial tagging. - return await RecomputeTagsAsync(highPriority, frozenPartialSemantics, cancellationToken).ConfigureAwait(false); + await RecomputeTagsAsync(highPriority, frozenPartialSemantics, cancellationToken).ConfigureAwait(false); + return default; + } + } + + private async ValueTask<(ImmutableDictionary> oldTagTrees, ImmutableDictionary> newTagTrees)> + CompareAndSwapTagTreesAsync( + Func>, ValueTask>>> callback, + CancellationToken cancellationToken) + { + while (!cancellationToken.IsCancellationRequested) + { + var oldTagTrees = Volatile.Read(ref _cachedTagTrees_mayChangeFromAnyThread); + + // Compute the new tag trees, based on what the current tag trees are. Intentionally CA(true) here so + // we stay on the UI thread if we're in a JTF blocking call. + var newTagTrees = await callback(oldTagTrees).ConfigureAwait(true); + + // Now, try to update the cached tag trees to what we computed. If we win, we're done. Otherwise, some + // other thread was able to do this, and we need to try again. + if (oldTagTrees == Interlocked.CompareExchange(ref _cachedTagTrees_mayChangeFromAnyThread, newTagTrees, oldTagTrees)) + return (oldTagTrees, newTagTrees); } + + return default; } /// @@ -208,11 +232,13 @@ private async ValueTask ProcessEventChangeAsync( private async Task>> RecomputeTagsAsync( bool highPriority, bool frozenPartialSemantics, + bool calledFromJtfRun, CancellationToken cancellationToken) { + // Jump to the main thread, as we have to grab the spans to tag and the caret point. await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable(); if (cancellationToken.IsCancellationRequested) - return default; + return ImmutableDictionary>.Empty; // 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 @@ -226,14 +252,11 @@ private async Task>> // bails gracefully by checking below. await _visibilityTracker.DelayWhileNonVisibleAsync( _dataSource.ThreadingContext, _dataSource.AsyncListener, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(captureContext: true); - - if (cancellationToken.IsCancellationRequested) - return default; } _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); if (cancellationToken.IsCancellationRequested) - return default; + return ImmutableDictionary>.Empty; using (Logger.LogBlock(FunctionId.Tagger_TagSource_RecomputeTags, cancellationToken)) { @@ -242,15 +265,14 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( // again on the foreground. var spansToTag = GetSpansAndDocumentsToTag(); var caretPosition = _dataSource.GetCaretPoint(_textView, _subjectBuffer); - var oldTagTrees = this.CachedTagTrees; - var oldState = _state_accessOnlyFromEventChangeQueueCallback; - - var subjectBufferVersion = _subjectBuffer.CurrentSnapshot.Version.VersionNumber; - await TaskScheduler.Default; + // If we're being called from within a blocking JTF.Run call, we don't want to switch to the background + // if we can avoid it. + if (!calledFromJtfRun) + await TaskScheduler.Default; if (cancellationToken.IsCancellationRequested) - return default; + return ImmutableDictionary>.Empty; if (frozenPartialSemantics) { @@ -259,23 +281,28 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( ds.SnapshotSpan)); } - // Create a context to store pass the information along and collect the results. - var context = new TaggerContext( - oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees); - await ProduceTagsAsync(context, cancellationToken).ConfigureAwait(false); + // Now spin, trying to compute the updated tags. We only need to do this as the tag state is also + // allowed to change on the UI thread (for example, taggers can say they want tags to be immediately + // removed when an edit happens. So, we need to keep recomputing the tags until we win and become the + // latest tags. + var oldState = _state_accessOnlyFromEventChangeQueueCallback; - if (cancellationToken.IsCancellationRequested) - return default; + var (oldTagTrees, newTagTrees) = await CompareAndSwapTagTreesAsync( + async oldTagTrees => + { + // Create a context to store pass the information along and collect the results. + var context = new TaggerContext( + oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees); + await ProduceTagsAsync(context, cancellationToken).ConfigureAwait(false); - // Process the result to determine what changed. - var newTagTrees = ComputeNewTagTrees(oldTagTrees, context); - var bufferToChanges = ProcessNewTagTrees(spansToTag, oldTagTrees, newTagTrees, cancellationToken); + return ComputeNewTagTrees(oldTagTrees, context); + }, cancellationToken).ConfigureAwait(continueOnCapturedContext: calledFromJtfRun); - // 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. - cancellationToken = CancellationToken.None; + // We may get back null if we were canceled. Immediately bail out in that case. + if (newTagTrees is null) + return ImmutableDictionary>.Empty; - this.CachedTagTrees = newTagTrees; + var bufferToChanges = ProcessNewTagTrees(spansToTag, oldTagTrees, newTagTrees, cancellationToken); // Note: assigning to 'State' is completely safe. It is only ever read from the _eventChangeQueue // serial callbacks on the threadpool. From 1dfb46eac93328fa789bc6e69fc1e11fdb85486a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 13:28:52 -0700 Subject: [PATCH 06/20] IN progress --- ...ousTaggerProvider.TagSource_ProduceTags.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index cb3228c9b15b..90ebb63d9ae7 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -226,9 +226,11 @@ private async ValueTask ProcessEventChangeAsync( /// In the event of a cancellation request, this method may either return at the next availability /// or throw a cancellation exception. /// - /// - /// If this tagging request should be processed as quickly as possible with no extra delays added for it. + /// If this tagging request should be processed as quickly as possible with no extra + /// delays added for it. /// + /// If this method is being called from within a JTF.Run call. This is used to + /// ensure we don't do unnecessary switches to the threadpool while JTF is waiting on us. private async Task>> RecomputeTagsAsync( bool highPriority, bool frozenPartialSemantics, @@ -254,12 +256,12 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( _dataSource.ThreadingContext, _dataSource.AsyncListener, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(captureContext: true); } - _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - if (cancellationToken.IsCancellationRequested) - return ImmutableDictionary>.Empty; - using (Logger.LogBlock(FunctionId.Tagger_TagSource_RecomputeTags, cancellationToken)) { + _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); + if (cancellationToken.IsCancellationRequested) + return ImmutableDictionary>.Empty; + // Make a copy of all the data we need while we're on the foreground. Then switch to a threadpool // thread to do the computation. Finally, once new tags have been computed, then we update our state // again on the foreground. @@ -591,19 +593,18 @@ private DiffResult ComputeDifference( if (_disposalTokenSource.Token.IsCancellationRequested) return null; - var tagTrees = this.CachedTagTrees; - // If this is the first time we're being asked for tags, and we're a tagger that requires the initial tags // be available synchronously on this call, and the computation of tags hasn't completed yet, then force the // tags to be computed now on this thread. The singular use case for this is Outlining which needs those // tags synchronously computed for things like Metadata-as-Source collapsing. + var tagTrees = _cachedTagTrees_mayChangeFromAnyThread; if (_firstTagsRequest && _dataSource.ComputeInitialTagsSynchronously(buffer) && !tagTrees.TryGetValue(buffer, out _)) { // Compute this as a high priority work item to have the lease amount of blocking as possible. tagTrees = _dataSource.ThreadingContext.JoinableTaskFactory.Run(() => - this.RecomputeTagsAsync(highPriority: true, _dataSource.SupportsFrozenPartialSemantics, _disposalTokenSource.Token)); + this.RecomputeTagsAsync(highPriority: true, _dataSource.SupportsFrozenPartialSemantics, calledFromJtfRun: true, _disposalTokenSource.Token)); } _firstTagsRequest = false; From 6b3608c51ac4fe062e24a651f41b89edf5076a09 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 13:38:37 -0700 Subject: [PATCH 07/20] Spin --- ...ousTaggerProvider.TagSource_ProduceTags.cs | 52 ++++++++++++------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 90ebb63d9ae7..b25ea510c328 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -93,32 +93,48 @@ private void RemoveTagsThatIntersectEdit(TextContentChangedEventArgs e) if (e.Changes.Count == 0) return; - var buffer = e.After.TextBuffer; - if (!this.CachedTagTrees.TryGetValue(buffer, out var treeForBuffer)) - return; - var snapshot = e.After; + var buffer = snapshot.TextBuffer; - var tagsToRemove = e.Changes.SelectMany(c => treeForBuffer.GetIntersectingSpans(new SnapshotSpan(snapshot, c.NewSpan))); - if (!tagsToRemove.Any()) - return; + // Everything we're passing in here is synchronous. So we can assert that this must complete synchronously + // as well. + var (oldTagTrees, newTagTrees) = CompareAndSwapTagTreesAsync( + oldTagTrees => + { + if (oldTagTrees.TryGetValue(buffer, out var treeForBuffer)) + { + var tagsToRemove = e.Changes.SelectMany(c => treeForBuffer.GetIntersectingSpans(new SnapshotSpan(snapshot, c.NewSpan))); + if (tagsToRemove.Any()) + { + var allTags = treeForBuffer.GetSpans(e.After).ToList(); + var newTagTree = new TagSpanIntervalTree( + buffer, + treeForBuffer.SpanTrackingMode, + allTags.Except(tagsToRemove, comparer: this)); + return new(oldTagTrees.SetItem(buffer, newTagTree)); + } + } + + // return oldTagTrees to indicate nothing changed. + return new(oldTagTrees); + }, _dataSource.ThreadingContext.DisposalToken).VerifyCompleted(); - var allTags = treeForBuffer.GetSpans(e.After).ToList(); - var newTagTree = new TagSpanIntervalTree( - buffer, - treeForBuffer.SpanTrackingMode, - allTags.Except(tagsToRemove, comparer: this)); + // Can happen if we were canceled. Just bail out immediate. + if (newTagTrees is null) + return; - this.CachedTagTrees = this.CachedTagTrees.SetItem(snapshot.TextBuffer, newTagTree); + // Nothing changed. Bail out. + if (oldTagTrees == newTagTrees) + return; // Not sure why we are diffing when we already have tagsToRemove. is it due to _tagSpanComparer might return // different result than GetIntersectingSpans? // // treeForBuffer basically points to oldTagTrees. case where oldTagTrees not exist is already taken cared by // CachedTagTrees.TryGetValue. - var difference = ComputeDifference(snapshot, newTagTree, treeForBuffer); + var difference = ComputeDifference(snapshot, newTagTrees[buffer], oldTagTrees[buffer]); - RaiseTagsChanged(snapshot.TextBuffer, difference); + RaiseTagsChanged(buffer, difference); } private TagSpanIntervalTree GetTagTree(ITextSnapshot snapshot, ImmutableDictionary> tagTrees) @@ -176,7 +192,7 @@ private async ValueTask ProcessEventChangeAsync( using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(lastNonFrozenComputationToken, cancellationToken); try { - await RecomputeTagsAsync(highPriority, frozenPartialSemantics, linkedTokenSource.Token).ConfigureAwait(false); + await RecomputeTagsAsync(highPriority, frozenPartialSemantics, calledFromJtfRun: false, linkedTokenSource.Token).ConfigureAwait(false); return default; } catch (OperationCanceledException ex) when (ExceptionUtilities.IsCurrentOperationBeingCancelled(ex, linkedTokenSource.Token)) @@ -188,12 +204,12 @@ private async ValueTask ProcessEventChangeAsync( { // Normal request to either compute frozen partial tags, or compute normal tags in a tagger that does // *not* support frozen partial tagging. - await RecomputeTagsAsync(highPriority, frozenPartialSemantics, cancellationToken).ConfigureAwait(false); + await RecomputeTagsAsync(highPriority, frozenPartialSemantics, calledFromJtfRun: false, cancellationToken).ConfigureAwait(false); return default; } } - private async ValueTask<(ImmutableDictionary> oldTagTrees, ImmutableDictionary> newTagTrees)> + private async Task<(ImmutableDictionary> oldTagTrees, ImmutableDictionary> newTagTrees)> CompareAndSwapTagTreesAsync( Func>, ValueTask>>> callback, CancellationToken cancellationToken) From 3ad1639d090a708cf63d5323962bdfd824c635ad Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 13:39:16 -0700 Subject: [PATCH 08/20] Done --- ...AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index b25ea510c328..e79eccc25bfb 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -305,11 +305,12 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( // latest tags. var oldState = _state_accessOnlyFromEventChangeQueueCallback; + TaggerContext context = null!; var (oldTagTrees, newTagTrees) = await CompareAndSwapTagTreesAsync( async oldTagTrees => { // Create a context to store pass the information along and collect the results. - var context = new TaggerContext( + context = new TaggerContext( oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees); await ProduceTagsAsync(context, cancellationToken).ConfigureAwait(false); From 1d544fbe8c4dff67c2b6dd32b861c67e5d6c1630 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 14:03:10 -0700 Subject: [PATCH 09/20] Cleanup --- ...mbeddedClassificationViewTaggerProvider.cs | 3 - ...nchronousTaggerProvider.BufferToTagTree.cs | 57 +++++++++++++++++++ ...actAsynchronousTaggerProvider.TagSource.cs | 14 ++++- ...ousTaggerProvider.TagSource_ProduceTags.cs | 53 ++++++++--------- .../AbstractAsynchronousTaggerProvider.cs | 11 +--- .../Core/Tagging/TaggerTextChangeBehavior.cs | 11 +--- 6 files changed, 100 insertions(+), 49 deletions(-) create mode 100644 src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs diff --git a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs index 24ca5dcbbf11..05f2bdf1af80 100644 --- a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs +++ b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs @@ -39,9 +39,6 @@ internal abstract class AbstractSemanticOrEmbeddedClassificationViewTaggerProvid private readonly IGlobalOptionService _globalOptions; private readonly ClassificationType _type; - // We want to track text changes so that we can try to only reclassify a method body if - // all edits were contained within one. - protected sealed override TaggerTextChangeBehavior TextChangeBehavior => TaggerTextChangeBehavior.TrackTextChanges; protected sealed override ImmutableArray Options { get; } = [SemanticColorizerOptionsStorage.SemanticColorizer]; protected AbstractSemanticOrEmbeddedClassificationViewTaggerProvider( diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs new file mode 100644 index 000000000000..f9f57c682f9e --- /dev/null +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs @@ -0,0 +1,57 @@ +// 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.Immutable; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Threading; +using Microsoft.CodeAnalysis.Editor.Shared.Tagging; +using Microsoft.VisualStudio.Text; + +namespace Microsoft.CodeAnalysis.Editor.Tagging; + +internal partial class AbstractAsynchronousTaggerProvider +{ + private readonly struct BufferToTagTree(ImmutableDictionary> map) + { + public static readonly BufferToTagTree Empty = new(ImmutableDictionary>.Empty); + + public readonly ImmutableDictionary> Map = map; + + public bool IsDefault => Map is null; + + public TagSpanIntervalTree this[ITextBuffer buffer] => Map[buffer]; + + internal static BufferToTagTree InterlockedExchange(ref BufferToTagTree location, BufferToTagTree value) + => new(Interlocked.Exchange(ref Unsafe.AsRef(in location.Map), value.Map)); + + internal static BufferToTagTree InterlockedCompareExchange(ref BufferToTagTree location, BufferToTagTree value, BufferToTagTree comparand) + => new(Interlocked.CompareExchange(ref Unsafe.AsRef(in location.Map), value.Map, comparand.Map)); + + public bool TryGetValue(ITextBuffer textBuffer, [NotNullWhen(true)] out TagSpanIntervalTree? tagTree) + => Map.TryGetValue(textBuffer, out tagTree); + + public BufferToTagTree Add(ITextBuffer buffer, TagSpanIntervalTree newTagTree) + => new(Map.Add(buffer, newTagTree)); + + public BufferToTagTree SetItem(ITextBuffer buffer, TagSpanIntervalTree newTagTree) + => new(Map.SetItem(buffer, newTagTree)); + + public static bool operator ==(BufferToTagTree left, BufferToTagTree right) + => left.Map == right.Map; + + public static bool operator !=(BufferToTagTree left, BufferToTagTree right) + => !(left == right); + + public override bool Equals([NotNullWhen(true)] object? obj) + => throw new NotSupportedException(); + + public override int GetHashCode() + => throw new NotSupportedException(); + + public bool ContainsKey(ITextBuffer oldBuffer) + => Map.ContainsKey(oldBuffer); + } +} diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs index 6e5e78348007..d088ffc73d82 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs @@ -5,7 +5,9 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Editor.Shared.Tagging; @@ -89,7 +91,7 @@ private sealed partial class TagSource /// /// The last tag trees that we computed per buffer. /// - private ImmutableDictionary> _cachedTagTrees_mayChangeFromAnyThread = ImmutableDictionary>.Empty; + private BufferToTagTree _cachedTagTrees_mayChangeFromAnyThread = BufferToTagTree.Empty; #endregion @@ -233,8 +235,11 @@ void Connect() _eventSource.Changed += OnEventSourceChanged; - if (_dataSource.TextChangeBehavior.HasFlag(TaggerTextChangeBehavior.TrackTextChanges)) + if (_dataSource.TextChangeBehavior.HasFlag(TaggerTextChangeBehavior.RemoveAllTags) || + _dataSource.TextChangeBehavior.HasFlag(TaggerTextChangeBehavior.RemoveTagsThatIntersectEdits)) + { _subjectBuffer.Changed += OnSubjectBufferChanged; + } if (_dataSource.CaretChangeBehavior.HasFlag(TaggerCaretChangeBehavior.RemoveAllTagsOnCaretMoveOutsideOfTag)) { @@ -278,8 +283,11 @@ void Disconnect() _textView.Caret.PositionChanged -= OnCaretPositionChanged; } - if (_dataSource.TextChangeBehavior.HasFlag(TaggerTextChangeBehavior.TrackTextChanges)) + if (_dataSource.TextChangeBehavior.HasFlag(TaggerTextChangeBehavior.RemoveAllTags) || + _dataSource.TextChangeBehavior.HasFlag(TaggerTextChangeBehavior.RemoveTagsThatIntersectEdits)) + { _subjectBuffer.Changed -= OnSubjectBufferChanged; + } _eventSource.Changed -= OnEventSourceChanged; diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index e79eccc25bfb..d72ed050deeb 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -54,8 +54,8 @@ private void RemoveAllTags() { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - var oldTagTrees = Interlocked.Exchange( - ref _cachedTagTrees_mayChangeFromAnyThread, ImmutableDictionary>.Empty); + var oldTagTrees = BufferToTagTree.InterlockedExchange( + ref _cachedTagTrees_mayChangeFromAnyThread, BufferToTagTree.Empty); var snapshot = _subjectBuffer.CurrentSnapshot; var oldTagTree = GetTagTree(snapshot, oldTagTrees); @@ -120,7 +120,7 @@ private void RemoveTagsThatIntersectEdit(TextContentChangedEventArgs e) }, _dataSource.ThreadingContext.DisposalToken).VerifyCompleted(); // Can happen if we were canceled. Just bail out immediate. - if (newTagTrees is null) + if (newTagTrees.IsDefault) return; // Nothing changed. Bail out. @@ -137,7 +137,7 @@ private void RemoveTagsThatIntersectEdit(TextContentChangedEventArgs e) RaiseTagsChanged(buffer, difference); } - private TagSpanIntervalTree GetTagTree(ITextSnapshot snapshot, ImmutableDictionary> tagTrees) + private TagSpanIntervalTree GetTagTree(ITextSnapshot snapshot, BufferToTagTree tagTrees) { return tagTrees.TryGetValue(snapshot.TextBuffer, out var tagTree) ? tagTree @@ -209,14 +209,13 @@ private async ValueTask ProcessEventChangeAsync( } } - private async Task<(ImmutableDictionary> oldTagTrees, ImmutableDictionary> newTagTrees)> - CompareAndSwapTagTreesAsync( - Func>, ValueTask>>> callback, - CancellationToken cancellationToken) + private async Task<(BufferToTagTree oldTagTrees, BufferToTagTree newTagTrees)> CompareAndSwapTagTreesAsync( + Func> callback, + CancellationToken cancellationToken) { while (!cancellationToken.IsCancellationRequested) { - var oldTagTrees = Volatile.Read(ref _cachedTagTrees_mayChangeFromAnyThread); + var oldTagTrees = _cachedTagTrees_mayChangeFromAnyThread; // Compute the new tag trees, based on what the current tag trees are. Intentionally CA(true) here so // we stay on the UI thread if we're in a JTF blocking call. @@ -224,7 +223,7 @@ private async ValueTask ProcessEventChangeAsync( // Now, try to update the cached tag trees to what we computed. If we win, we're done. Otherwise, some // other thread was able to do this, and we need to try again. - if (oldTagTrees == Interlocked.CompareExchange(ref _cachedTagTrees_mayChangeFromAnyThread, newTagTrees, oldTagTrees)) + if (oldTagTrees == BufferToTagTree.InterlockedCompareExchange(ref _cachedTagTrees_mayChangeFromAnyThread, newTagTrees, oldTagTrees)) return (oldTagTrees, newTagTrees); } @@ -247,7 +246,7 @@ private async ValueTask ProcessEventChangeAsync( /// /// If this method is being called from within a JTF.Run call. This is used to /// ensure we don't do unnecessary switches to the threadpool while JTF is waiting on us. - private async Task>> RecomputeTagsAsync( + private async Task RecomputeTagsAsync( bool highPriority, bool frozenPartialSemantics, bool calledFromJtfRun, @@ -256,7 +255,7 @@ private async Task>> // Jump to the main thread, as we have to grab the spans to tag and the caret point. await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable(); if (cancellationToken.IsCancellationRequested) - return ImmutableDictionary>.Empty; + return default; // 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 @@ -276,7 +275,7 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); if (cancellationToken.IsCancellationRequested) - return ImmutableDictionary>.Empty; + return default; // Make a copy of all the data we need while we're on the foreground. Then switch to a threadpool // thread to do the computation. Finally, once new tags have been computed, then we update our state @@ -290,7 +289,7 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( await TaskScheduler.Default; if (cancellationToken.IsCancellationRequested) - return ImmutableDictionary>.Empty; + return default; if (frozenPartialSemantics) { @@ -311,15 +310,15 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( { // Create a context to store pass the information along and collect the results. context = new TaggerContext( - oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees); + oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees.Map); await ProduceTagsAsync(context, cancellationToken).ConfigureAwait(false); return ComputeNewTagTrees(oldTagTrees, context); }, cancellationToken).ConfigureAwait(continueOnCapturedContext: calledFromJtfRun); // We may get back null if we were canceled. Immediately bail out in that case. - if (newTagTrees is null) - return ImmutableDictionary>.Empty; + if (newTagTrees.IsDefault) + return default; var bufferToChanges = ProcessNewTagTrees(spansToTag, oldTagTrees, newTagTrees, cancellationToken); @@ -376,9 +375,7 @@ private static void CheckSnapshot(ITextSnapshot snapshot) } } - private ImmutableDictionary> ComputeNewTagTrees( - ImmutableDictionary> oldTagTrees, - TaggerContext context) + private BufferToTagTree ComputeNewTagTrees(BufferToTagTree oldTagTrees, TaggerContext context) { // Ignore any tag spans reported for any buffers we weren't interested in. @@ -397,7 +394,7 @@ private ImmutableDictionary> ComputeNewTa // for that buffer. In general this will work by keeping around old tags that // weren't in the range that was re-tagged, and merging them with the new tags // produced for the range that was re-tagged. - var newTagTrees = ImmutableDictionary>.Empty; + var newTagTrees = BufferToTagTree.Empty; foreach (var buffer in buffersToTag) { var newTagTree = ComputeNewTagTree(oldTagTrees, buffer, newTagsByBuffer[buffer], spansToInvalidateByBuffer[buffer]); @@ -409,7 +406,7 @@ private ImmutableDictionary> ComputeNewTa } private TagSpanIntervalTree? ComputeNewTagTree( - ImmutableDictionary> oldTagTrees, + BufferToTagTree oldTagTrees, ITextBuffer textBuffer, IEnumerable> newTags, IEnumerable spansToInvalidate) @@ -484,15 +481,15 @@ private Task ProduceTagsAsync(TaggerContext context, CancellationToken can private Dictionary ProcessNewTagTrees( ImmutableArray spansToTag, - ImmutableDictionary> oldTagTrees, - ImmutableDictionary> newTagTrees, + BufferToTagTree oldTagTrees, + BufferToTagTree newTagTrees, CancellationToken cancellationToken) { using (Logger.LogBlock(FunctionId.Tagger_TagSource_ProcessNewTags, cancellationToken)) { var bufferToChanges = new Dictionary(); - foreach (var (latestBuffer, latestSpans) in newTagTrees) + foreach (var (latestBuffer, latestSpans) in newTagTrees.Map) { var snapshot = spansToTag.First(s => s.SnapshotSpan.Snapshot.TextBuffer == latestBuffer).SnapshotSpan.Snapshot; @@ -508,7 +505,7 @@ private Dictionary ProcessNewTagTrees( } } - foreach (var (oldBuffer, previousSpans) in oldTagTrees) + foreach (var (oldBuffer, previousSpans) in oldTagTrees.Map) { if (!newTagTrees.ContainsKey(oldBuffer)) { @@ -626,6 +623,10 @@ private DiffResult ComputeDifference( _firstTagsRequest = false; + // We can get a defualt instance back if we were canceled. + if (tagTrees.IsDefault) + return null; + tagTrees.TryGetValue(buffer, out var tags); return tags; } diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs index 08b0c89e4d93..b5ebabac14f2 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.cs @@ -40,14 +40,9 @@ internal abstract partial class AbstractAsynchronousTaggerProvider where T private readonly ITextBufferVisibilityTracker? _visibilityTracker; /// - /// The behavior the tagger engine will have when text changes happen to the subject buffer - /// it is attached to. Most taggers can simply use . - /// However, advanced taggers that want to perform specialized behavior depending on what has - /// actually changed in the file can specify . - /// - /// If this is specified the tagger engine will track text changes and pass them along as - /// when calling - /// . + /// The behavior the tagger engine will have when text changes happen to the subject buffer it is attached to. Most + /// taggers can simply use . However, advanced taggers that want to + /// perform specialized behavior depending on what has actually changed in the file can specify that here. /// protected virtual TaggerTextChangeBehavior TextChangeBehavior => TaggerTextChangeBehavior.None; diff --git a/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs b/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs index b91bfe6d6058..8e36389be065 100644 --- a/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs +++ b/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs @@ -19,13 +19,6 @@ internal enum TaggerTextChangeBehavior /// None = 0, - /// - /// The async tagger infrastructure will track text changes to the subject buffer it is - /// attached to. The text changes will be provided to the - /// that is passed to . - /// - TrackTextChanges = 1 << 0, - /// /// The async tagger infrastructure will track text changes to the subject buffer it is /// attached to. The text changes will be provided to the @@ -33,7 +26,7 @@ internal enum TaggerTextChangeBehavior /// /// On any edit, tags that intersect the text change range will immediately removed. /// - RemoveTagsThatIntersectEdits = TrackTextChanges | (1 << 1), + RemoveTagsThatIntersectEdits = 1 << 1, /// /// The async tagger infrastructure will track text changes to the subject buffer it is @@ -41,5 +34,5 @@ internal enum TaggerTextChangeBehavior /// /// On any edit all tags will we be removed. /// - RemoveAllTags = TrackTextChanges | (1 << 2), + RemoveAllTags = 1 << 2, } From 4d1998a23fa3b0bf71da5dc8568b766933cb0861 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 14:03:36 -0700 Subject: [PATCH 10/20] Cleanup --- .../Core/EditAndContinue/ActiveStatementTaggerProvider.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTaggerProvider.cs b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTaggerProvider.cs index f76bdbbe9b2b..911b8fa3f548 100644 --- a/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTaggerProvider.cs +++ b/src/EditorFeatures/Core/EditAndContinue/ActiveStatementTaggerProvider.cs @@ -40,10 +40,6 @@ internal partial class ActiveStatementTaggerProvider( [Import(AllowDefault = true)] ITextBufferVisibilityTracker? visibilityTracker, IAsynchronousOperationListenerProvider listenerProvider) : AsynchronousTaggerProvider(threadingContext, globalOptions, visibilityTracker, listenerProvider.GetListener(FeatureAttribute.Classification)) { - // We want to track text changes so that we can try to only reclassify a method body if - // all edits were contained within one. - protected override TaggerTextChangeBehavior TextChangeBehavior => TaggerTextChangeBehavior.TrackTextChanges; - protected override TaggerDelay EventChangeDelay => TaggerDelay.NearImmediate; protected override ITaggerEventSource CreateEventSource(ITextView? textView, ITextBuffer subjectBuffer) From ba6cbe9d28eb776e194be02a32a82c19194e16af Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 14:06:51 -0700 Subject: [PATCH 11/20] Add docs --- .../Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs index d088ffc73d82..01f0e5f32171 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs @@ -89,7 +89,10 @@ private sealed partial class TagSource private readonly CancellationSeries _nonFrozenComputationCancellationSeries; /// - /// The last tag trees that we computed per buffer. + /// The last tag trees that we computed per buffer. Note: this can be read/written from any thread. Because of + /// that, we have to use safe operations to actually read or write it. This includes using looping "compare and + /// swap" algorithms to make sure that it is consistently moved forward no matter which thread is trying to + /// mutate it. /// private BufferToTagTree _cachedTagTrees_mayChangeFromAnyThread = BufferToTagTree.Empty; From 4e01406c9e976b5b1fc14a01f1cc851cb878d630 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 14:23:33 -0700 Subject: [PATCH 12/20] Reverse --- ...bstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs index 05f2bdf1af80..73cc10529dcc 100644 --- a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs +++ b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs @@ -180,7 +180,7 @@ private async Task TryClassifyContainingMemberSpanAsync( var lastSourceText = lastTextSnapshot.AsText(); var currentSourceText = snapshotSpan.Snapshot.AsText(); - var textChangeRanges = lastSourceText.GetChangeRanges(currentSourceText); + var textChangeRanges = currentSourceText.GetChangeRanges(lastSourceText); var collapsedRange = TextChangeRange.Collapse(textChangeRanges); var changedSpan = new TextSpan(collapsedRange.Span.Start, collapsedRange.NewLength); From 13b94930ce24e48235a3894334902bd7e6d38066 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 15:43:53 -0700 Subject: [PATCH 13/20] Remove wrapper --- ...nchronousTaggerProvider.BufferToTagTree.cs | 57 ------------------- ...actAsynchronousTaggerProvider.TagSource.cs | 2 +- ...ousTaggerProvider.TagSource_ProduceTags.cs | 47 +++++++-------- 3 files changed, 25 insertions(+), 81 deletions(-) delete mode 100644 src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs deleted file mode 100644 index f9f57c682f9e..000000000000 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.BufferToTagTree.cs +++ /dev/null @@ -1,57 +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.Immutable; -using System.Diagnostics.CodeAnalysis; -using System.Runtime.CompilerServices; -using System.Threading; -using Microsoft.CodeAnalysis.Editor.Shared.Tagging; -using Microsoft.VisualStudio.Text; - -namespace Microsoft.CodeAnalysis.Editor.Tagging; - -internal partial class AbstractAsynchronousTaggerProvider -{ - private readonly struct BufferToTagTree(ImmutableDictionary> map) - { - public static readonly BufferToTagTree Empty = new(ImmutableDictionary>.Empty); - - public readonly ImmutableDictionary> Map = map; - - public bool IsDefault => Map is null; - - public TagSpanIntervalTree this[ITextBuffer buffer] => Map[buffer]; - - internal static BufferToTagTree InterlockedExchange(ref BufferToTagTree location, BufferToTagTree value) - => new(Interlocked.Exchange(ref Unsafe.AsRef(in location.Map), value.Map)); - - internal static BufferToTagTree InterlockedCompareExchange(ref BufferToTagTree location, BufferToTagTree value, BufferToTagTree comparand) - => new(Interlocked.CompareExchange(ref Unsafe.AsRef(in location.Map), value.Map, comparand.Map)); - - public bool TryGetValue(ITextBuffer textBuffer, [NotNullWhen(true)] out TagSpanIntervalTree? tagTree) - => Map.TryGetValue(textBuffer, out tagTree); - - public BufferToTagTree Add(ITextBuffer buffer, TagSpanIntervalTree newTagTree) - => new(Map.Add(buffer, newTagTree)); - - public BufferToTagTree SetItem(ITextBuffer buffer, TagSpanIntervalTree newTagTree) - => new(Map.SetItem(buffer, newTagTree)); - - public static bool operator ==(BufferToTagTree left, BufferToTagTree right) - => left.Map == right.Map; - - public static bool operator !=(BufferToTagTree left, BufferToTagTree right) - => !(left == right); - - public override bool Equals([NotNullWhen(true)] object? obj) - => throw new NotSupportedException(); - - public override int GetHashCode() - => throw new NotSupportedException(); - - public bool ContainsKey(ITextBuffer oldBuffer) - => Map.ContainsKey(oldBuffer); - } -} diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs index 01f0e5f32171..2524f8601823 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs @@ -94,7 +94,7 @@ private sealed partial class TagSource /// swap" algorithms to make sure that it is consistently moved forward no matter which thread is trying to /// mutate it. /// - private BufferToTagTree _cachedTagTrees_mayChangeFromAnyThread = BufferToTagTree.Empty; + private ImmutableDictionary> _cachedTagTrees_mayChangeFromAnyThread = ImmutableDictionary>.Empty; #endregion diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index d72ed050deeb..6b02a1563a9b 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -54,8 +54,8 @@ private void RemoveAllTags() { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); - var oldTagTrees = BufferToTagTree.InterlockedExchange( - ref _cachedTagTrees_mayChangeFromAnyThread, BufferToTagTree.Empty); + var oldTagTrees = Interlocked.Exchange( + ref _cachedTagTrees_mayChangeFromAnyThread, ImmutableDictionary>.Empty); var snapshot = _subjectBuffer.CurrentSnapshot; var oldTagTree = GetTagTree(snapshot, oldTagTrees); @@ -120,7 +120,7 @@ private void RemoveTagsThatIntersectEdit(TextContentChangedEventArgs e) }, _dataSource.ThreadingContext.DisposalToken).VerifyCompleted(); // Can happen if we were canceled. Just bail out immediate. - if (newTagTrees.IsDefault) + if (newTagTrees is null) return; // Nothing changed. Bail out. @@ -137,7 +137,7 @@ private void RemoveTagsThatIntersectEdit(TextContentChangedEventArgs e) RaiseTagsChanged(buffer, difference); } - private TagSpanIntervalTree GetTagTree(ITextSnapshot snapshot, BufferToTagTree tagTrees) + private TagSpanIntervalTree GetTagTree(ITextSnapshot snapshot, ImmutableDictionary> tagTrees) { return tagTrees.TryGetValue(snapshot.TextBuffer, out var tagTree) ? tagTree @@ -209,8 +209,9 @@ private async ValueTask ProcessEventChangeAsync( } } - private async Task<(BufferToTagTree oldTagTrees, BufferToTagTree newTagTrees)> CompareAndSwapTagTreesAsync( - Func> callback, + private async Task<(ImmutableDictionary> oldTagTrees, ImmutableDictionary> newTagTrees)> + CompareAndSwapTagTreesAsync( + Func>, ValueTask>>> callback, CancellationToken cancellationToken) { while (!cancellationToken.IsCancellationRequested) @@ -223,7 +224,7 @@ private async ValueTask ProcessEventChangeAsync( // Now, try to update the cached tag trees to what we computed. If we win, we're done. Otherwise, some // other thread was able to do this, and we need to try again. - if (oldTagTrees == BufferToTagTree.InterlockedCompareExchange(ref _cachedTagTrees_mayChangeFromAnyThread, newTagTrees, oldTagTrees)) + if (oldTagTrees == Interlocked.CompareExchange(ref _cachedTagTrees_mayChangeFromAnyThread, newTagTrees, oldTagTrees)) return (oldTagTrees, newTagTrees); } @@ -246,7 +247,7 @@ private async ValueTask ProcessEventChangeAsync( /// /// If this method is being called from within a JTF.Run call. This is used to /// ensure we don't do unnecessary switches to the threadpool while JTF is waiting on us. - private async Task RecomputeTagsAsync( + private async Task>?> RecomputeTagsAsync( bool highPriority, bool frozenPartialSemantics, bool calledFromJtfRun, @@ -255,7 +256,7 @@ private async Task RecomputeTagsAsync( // Jump to the main thread, as we have to grab the spans to tag and the caret point. await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable(); if (cancellationToken.IsCancellationRequested) - return default; + return null; // 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 @@ -275,7 +276,7 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( { _dataSource.ThreadingContext.ThrowIfNotOnUIThread(); if (cancellationToken.IsCancellationRequested) - return default; + return null; // Make a copy of all the data we need while we're on the foreground. Then switch to a threadpool // thread to do the computation. Finally, once new tags have been computed, then we update our state @@ -289,7 +290,7 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( await TaskScheduler.Default; if (cancellationToken.IsCancellationRequested) - return default; + return null; if (frozenPartialSemantics) { @@ -310,15 +311,15 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( { // Create a context to store pass the information along and collect the results. context = new TaggerContext( - oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees.Map); + oldState, frozenPartialSemantics, spansToTag, caretPosition, oldTagTrees); await ProduceTagsAsync(context, cancellationToken).ConfigureAwait(false); return ComputeNewTagTrees(oldTagTrees, context); }, cancellationToken).ConfigureAwait(continueOnCapturedContext: calledFromJtfRun); // We may get back null if we were canceled. Immediately bail out in that case. - if (newTagTrees.IsDefault) - return default; + if (newTagTrees is null) + return null; var bufferToChanges = ProcessNewTagTrees(spansToTag, oldTagTrees, newTagTrees, cancellationToken); @@ -375,7 +376,7 @@ private static void CheckSnapshot(ITextSnapshot snapshot) } } - private BufferToTagTree ComputeNewTagTrees(BufferToTagTree oldTagTrees, TaggerContext context) + private ImmutableDictionary> ComputeNewTagTrees(ImmutableDictionary> oldTagTrees, TaggerContext context) { // Ignore any tag spans reported for any buffers we weren't interested in. @@ -394,7 +395,7 @@ private BufferToTagTree ComputeNewTagTrees(BufferToTagTree oldTagTrees, TaggerCo // for that buffer. In general this will work by keeping around old tags that // weren't in the range that was re-tagged, and merging them with the new tags // produced for the range that was re-tagged. - var newTagTrees = BufferToTagTree.Empty; + var newTagTrees = ImmutableDictionary>.Empty; foreach (var buffer in buffersToTag) { var newTagTree = ComputeNewTagTree(oldTagTrees, buffer, newTagsByBuffer[buffer], spansToInvalidateByBuffer[buffer]); @@ -406,7 +407,7 @@ private BufferToTagTree ComputeNewTagTrees(BufferToTagTree oldTagTrees, TaggerCo } private TagSpanIntervalTree? ComputeNewTagTree( - BufferToTagTree oldTagTrees, + ImmutableDictionary> oldTagTrees, ITextBuffer textBuffer, IEnumerable> newTags, IEnumerable spansToInvalidate) @@ -481,15 +482,15 @@ private Task ProduceTagsAsync(TaggerContext context, CancellationToken can private Dictionary ProcessNewTagTrees( ImmutableArray spansToTag, - BufferToTagTree oldTagTrees, - BufferToTagTree newTagTrees, + ImmutableDictionary> oldTagTrees, + ImmutableDictionary> newTagTrees, CancellationToken cancellationToken) { using (Logger.LogBlock(FunctionId.Tagger_TagSource_ProcessNewTags, cancellationToken)) { var bufferToChanges = new Dictionary(); - foreach (var (latestBuffer, latestSpans) in newTagTrees.Map) + foreach (var (latestBuffer, latestSpans) in newTagTrees) { var snapshot = spansToTag.First(s => s.SnapshotSpan.Snapshot.TextBuffer == latestBuffer).SnapshotSpan.Snapshot; @@ -505,7 +506,7 @@ private Dictionary ProcessNewTagTrees( } } - foreach (var (oldBuffer, previousSpans) in oldTagTrees.Map) + foreach (var (oldBuffer, previousSpans) in oldTagTrees) { if (!newTagTrees.ContainsKey(oldBuffer)) { @@ -623,8 +624,8 @@ private DiffResult ComputeDifference( _firstTagsRequest = false; - // We can get a defualt instance back if we were canceled. - if (tagTrees.IsDefault) + // We can get null back if we were canceled. + if (tagTrees is null) return null; tagTrees.TryGetValue(buffer, out var tags); From 5723ae14bba2537bb3a86f23148ec9df9322be4a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 15:45:36 -0700 Subject: [PATCH 14/20] Update src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs Co-authored-by: Jason Malinowski --- ...bstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs index 73cc10529dcc..fb1b01a339e9 100644 --- a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs +++ b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs @@ -266,7 +266,7 @@ await classificationService.AddEmbeddedLanguageClassificationsAsync( // Let the context know that this was the span we actually tried to tag. context.SetSpansTagged([snapshotSpan]); - // Store teh semantic version and snapshot we used to produce these tags. We can use this in the future + // Store the semantic version and snapshot we used to produce these tags. We can use this in the future // to try to limit what we classify, if all edits were made within a single member. context.State = (currentSemanticVersion, snapshot); } From 5ef1b2a5ef605e42f3482a0436a5bfbeb84bf283 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 15:46:33 -0700 Subject: [PATCH 15/20] fix comments --- .../Core/Tagging/TaggerTextChangeBehavior.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs b/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs index 8e36389be065..008425992693 100644 --- a/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs +++ b/src/EditorFeatures/Core/Tagging/TaggerTextChangeBehavior.cs @@ -20,19 +20,14 @@ internal enum TaggerTextChangeBehavior None = 0, /// - /// The async tagger infrastructure will track text changes to the subject buffer it is - /// attached to. The text changes will be provided to the - /// that is passed to . - /// - /// On any edit, tags that intersect the text change range will immediately removed. + /// The async tagger infrastructure will track text changes to the subject buffer it is attached to. On any edit, + /// tags that intersect the text change range will immediately removed. /// RemoveTagsThatIntersectEdits = 1 << 1, /// - /// The async tagger infrastructure will track text changes to the subject buffer it is - /// attached to. - /// - /// On any edit all tags will we be removed. + /// The async tagger infrastructure will track text changes to the subject buffer it is attached to. On any edit all + /// tags will we be removed. /// RemoveAllTags = 1 << 2, } From 4eed86727441c312d8adccb2481960af8d651940 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 15:51:57 -0700 Subject: [PATCH 16/20] use correct token --- .../AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 6b02a1563a9b..c120d199bc81 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -117,7 +117,7 @@ private void RemoveTagsThatIntersectEdit(TextContentChangedEventArgs e) // return oldTagTrees to indicate nothing changed. return new(oldTagTrees); - }, _dataSource.ThreadingContext.DisposalToken).VerifyCompleted(); + }, _disposalTokenSource.Token).VerifyCompleted(); // Can happen if we were canceled. Just bail out immediate. if (newTagTrees is null) From 2b83bd9ba71556f9c57a6b66a2eeed09fa7c011b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 15:54:45 -0700 Subject: [PATCH 17/20] Add docs --- ...tractAsynchronousTaggerProvider.TagSource_ProduceTags.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index c120d199bc81..17eb80f2cece 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -209,6 +209,12 @@ private async ValueTask ProcessEventChangeAsync( } } + /// + /// Spins, repeatedly calling into with the current state of the tag trees. When + /// the result of the callback can be saved without any intervening writes to happening on another thread, then this helper returns. This + /// helper may also returns in the case of cancellation. + /// private async Task<(ImmutableDictionary> oldTagTrees, ImmutableDictionary> newTagTrees)> CompareAndSwapTagTreesAsync( Func>, ValueTask>>> callback, From 0e6342d5a4804e88b2bdb1d1cc15d66aee1fb6b4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 16:03:01 -0700 Subject: [PATCH 18/20] Text image helper --- ...mbeddedClassificationViewTaggerProvider.cs | 9 +- .../Text/Extensions.SnapshotSourceText.cs | 75 +-------------- src/EditorFeatures/Text/ITextImageHelpers.cs | 93 +++++++++++++++++++ 3 files changed, 99 insertions(+), 78 deletions(-) create mode 100644 src/EditorFeatures/Text/ITextImageHelpers.cs diff --git a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs index fb1b01a339e9..64e82b739496 100644 --- a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs +++ b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs @@ -165,7 +165,7 @@ private async Task TryClassifyContainingMemberSpanAsync( if (context.State is null) return false; - var (lastSemanticVersion, lastTextSnapshot) = ((VersionStamp, ITextSnapshot))context.State; + var (lastSemanticVersion, lastTextImage) = ((VersionStamp, ITextImage))context.State; // if a top level change was made. We can't perform this optimization. if (lastSemanticVersion != currentSemanticVersion) @@ -177,10 +177,11 @@ private async Task TryClassifyContainingMemberSpanAsync( // will find the member that contains the changes and only refresh that member. If possible, try to get a // speculative binder to make things even cheaper. - var lastSourceText = lastTextSnapshot.AsText(); - var currentSourceText = snapshotSpan.Snapshot.AsText(); + var currentTextImage = ((ITextSnapshot2)snapshotSpan.Snapshot).TextImage; - var textChangeRanges = currentSourceText.GetChangeRanges(lastSourceText); + // We explicitly want the range in reverse (going from current, to last image). That way the change span is in + // coordinates corresponding to the node we have *now*. + var textChangeRanges = ITextImageHelpers.GetChangeRanges(currentTextImage, lastTextImage); var collapsedRange = TextChangeRange.Collapse(textChangeRanges); var changedSpan = new TextSpan(collapsedRange.Span.Start, collapsedRange.NewLength); diff --git a/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs b/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs index 9adb30508369..a85c7fa6f784 100644 --- a/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs +++ b/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs @@ -24,7 +24,6 @@ public static partial class Extensions /// private class SnapshotSourceText : SourceText { - private static readonly Func s_textLog = (v1, v2) => string.Format("FullRange : from {0} to {1}", v1, v2); /// /// The backing the SourceText instance @@ -361,7 +360,7 @@ private IReadOnlyList GetChangeRanges(ITextImage? oldImage, int } else { - return GetChangeRanges(oldImage, newImage, forward: oldImage.Version.VersionNumber <= newImage.Version.VersionNumber); + return ITextImageHelpers.GetChangeRanges(oldImage, newImage); } } @@ -373,78 +372,6 @@ private static bool AreSameReiteratedVersion(ITextImage oldImage, ITextImage new return oldSnapshot != null && newSnapshot != null && oldSnapshot.Version.ReiteratedVersionNumber == newSnapshot.Version.ReiteratedVersionNumber; } - private static readonly Func s_forwardTextChangeRange = c => CreateTextChangeRange(c, forward: true); - private static readonly Func s_backwardTextChangeRange = c => CreateTextChangeRange(c, forward: false); - - private static IReadOnlyList GetChangeRanges(ITextImage snapshot1, ITextImage snapshot2, bool forward) - { - var oldSnapshot = forward ? snapshot1 : snapshot2; - var newSnapshot = forward ? snapshot2 : snapshot1; - - INormalizedTextChangeCollection? changes = null; - for (var oldVersion = oldSnapshot.Version; - oldVersion != newSnapshot.Version; - oldVersion = oldVersion.Next) - { - if (oldVersion.Changes.Count != 0) - { - if (changes != null) - { - // Oops - more than one "textual" change between these snapshots, bail and try to find smallest changes span - Logger.Log(FunctionId.Workspace_SourceText_GetChangeRanges, s_textLog, snapshot1.Version.VersionNumber, snapshot2.Version.VersionNumber); - - return [GetChangeRanges(oldSnapshot.Version, newSnapshot.Version, forward)]; - } - else - { - changes = oldVersion.Changes; - } - } - } - - if (changes == null) - { - return ImmutableArray.Create(); - } - else - { - return ImmutableArray.CreateRange(changes.Select(forward ? s_forwardTextChangeRange : s_backwardTextChangeRange)); - } - } - - private static TextChangeRange GetChangeRanges(ITextImageVersion oldVersion, ITextImageVersion newVersion, bool forward) - { - TextChangeRange? range = null; - var iterator = GetMultipleVersionTextChanges(oldVersion, newVersion, forward); - foreach (var changes in forward ? iterator : iterator.Reverse()) - { - range = range.Accumulate(changes); - } - - RoslynDebug.Assert(range.HasValue); - return range.Value; - } - - private static IEnumerable> GetMultipleVersionTextChanges( - ITextImageVersion oldVersion, ITextImageVersion newVersion, bool forward) - { - for (var version = oldVersion; version != newVersion; version = version.Next) - { - yield return version.Changes.Select(forward ? s_forwardTextChangeRange : s_backwardTextChangeRange); - } - } - - private static TextChangeRange CreateTextChangeRange(ITextChange change, bool forward) - { - if (forward) - { - return new TextChangeRange(new TextSpan(change.OldSpan.Start, change.OldSpan.Length), change.NewLength); - } - else - { - return new TextChangeRange(new TextSpan(change.NewSpan.Start, change.NewSpan.Length), change.OldLength); - } - } #endregion } } diff --git a/src/EditorFeatures/Text/ITextImageHelpers.cs b/src/EditorFeatures/Text/ITextImageHelpers.cs new file mode 100644 index 000000000000..92b1ee5464db --- /dev/null +++ b/src/EditorFeatures/Text/ITextImageHelpers.cs @@ -0,0 +1,93 @@ +// 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.Linq; +using Microsoft.CodeAnalysis.Internal.Log; +using Microsoft.VisualStudio.Text; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.Text; + +internal static class ITextImageHelpers +{ + private static readonly Func s_textLog = (v1, v2) => string.Format("FullRange : from {0} to {1}", v1, v2); + + private static readonly Func s_forwardTextChangeRange = c => CreateTextChangeRange(c, forward: true); + private static readonly Func s_backwardTextChangeRange = c => CreateTextChangeRange(c, forward: false); + + public static IReadOnlyList GetChangeRanges(ITextImage oldImage, ITextImage newImage) + { + var forward = oldImage.Version.VersionNumber <= newImage.Version.VersionNumber; + + var oldSnapshot = forward ? oldImage : newImage; + var newSnapshot = forward ? oldImage : newImage; + + INormalizedTextChangeCollection? changes = null; + for (var oldVersion = oldSnapshot.Version; + oldVersion != newSnapshot.Version; + oldVersion = oldVersion.Next) + { + if (oldVersion.Changes.Count != 0) + { + if (changes != null) + { + // Oops - more than one "textual" change between these snapshots, bail and try to find smallest changes span + Logger.Log(FunctionId.Workspace_SourceText_GetChangeRanges, s_textLog, oldImage.Version.VersionNumber, newImage.Version.VersionNumber); + + return [GetChangeRanges(oldSnapshot.Version, newSnapshot.Version, forward)]; + } + else + { + changes = oldVersion.Changes; + } + } + } + + if (changes == null) + { + return []; + } + else + { + return ImmutableArray.CreateRange(changes.Select(forward ? s_forwardTextChangeRange : s_backwardTextChangeRange)); + } + } + + private static TextChangeRange GetChangeRanges(ITextImageVersion oldVersion, ITextImageVersion newVersion, bool forward) + { + TextChangeRange? range = null; + var iterator = GetMultipleVersionTextChanges(oldVersion, newVersion, forward); + foreach (var changes in forward ? iterator : iterator.Reverse()) + { + range = range.Accumulate(changes); + } + + RoslynDebug.Assert(range.HasValue); + return range.Value; + } + + private static IEnumerable> GetMultipleVersionTextChanges( + ITextImageVersion oldVersion, ITextImageVersion newVersion, bool forward) + { + for (var version = oldVersion; version != newVersion; version = version.Next) + { + yield return version.Changes.Select(forward ? s_forwardTextChangeRange : s_backwardTextChangeRange); + } + } + + private static TextChangeRange CreateTextChangeRange(ITextChange change, bool forward) + { + if (forward) + { + return new TextChangeRange(new TextSpan(change.OldSpan.Start, change.OldSpan.Length), change.NewLength); + } + else + { + return new TextChangeRange(new TextSpan(change.NewSpan.Start, change.NewSpan.Length), change.OldLength); + } + } +} From e08f41f9e7a894d7f5c5a335e83b60a3d56b3a9c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 16:16:18 -0700 Subject: [PATCH 19/20] use text images --- ...EmbeddedClassificationViewTaggerProvider.cs | 18 ++++++++++-------- src/EditorFeatures/Text/ITextImageHelpers.cs | 17 ++++++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs index 64e82b739496..f98ee744936f 100644 --- a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs +++ b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs @@ -165,7 +165,8 @@ private async Task TryClassifyContainingMemberSpanAsync( if (context.State is null) return false; - var (lastSemanticVersion, lastTextImage) = ((VersionStamp, ITextImage))context.State; + // Retrieve the information about the last time we classified this document. + var (lastSemanticVersion, lastTextImageVersion) = ((VersionStamp, ITextImageVersion))context.State; // if a top level change was made. We can't perform this optimization. if (lastSemanticVersion != currentSemanticVersion) @@ -177,11 +178,9 @@ private async Task TryClassifyContainingMemberSpanAsync( // will find the member that contains the changes and only refresh that member. If possible, try to get a // speculative binder to make things even cheaper. - var currentTextImage = ((ITextSnapshot2)snapshotSpan.Snapshot).TextImage; + var currentTextImageVersion = GetTextImageVersion(snapshotSpan); - // We explicitly want the range in reverse (going from current, to last image). That way the change span is in - // coordinates corresponding to the node we have *now*. - var textChangeRanges = ITextImageHelpers.GetChangeRanges(currentTextImage, lastTextImage); + var textChangeRanges = ITextImageHelpers.GetChangeRanges(lastTextImageVersion, currentTextImageVersion); var collapsedRange = TextChangeRange.Collapse(textChangeRanges); var changedSpan = new TextSpan(collapsedRange.Span.Start, collapsedRange.NewLength); @@ -224,6 +223,9 @@ await ClassifySpansAsync( return true; } + private static ITextImageVersion GetTextImageVersion(SnapshotSpan snapshotSpan) + => ((ITextSnapshot2)snapshotSpan.Snapshot).TextImage.Version; + private async Task ClassifySpansAsync( TaggerContext context, Document document, @@ -267,9 +269,9 @@ await classificationService.AddEmbeddedLanguageClassificationsAsync( // Let the context know that this was the span we actually tried to tag. context.SetSpansTagged([snapshotSpan]); - // Store the semantic version and snapshot we used to produce these tags. We can use this in the future - // to try to limit what we classify, if all edits were made within a single member. - context.State = (currentSemanticVersion, snapshot); + // Store the semantic version and text-image-version we used to produce these tags. We can use this in + // the future to try to limit what we classify, if all edits were made within a single member. + context.State = (currentSemanticVersion, GetTextImageVersion(snapshotSpan)); } } catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken)) diff --git a/src/EditorFeatures/Text/ITextImageHelpers.cs b/src/EditorFeatures/Text/ITextImageHelpers.cs index 92b1ee5464db..69a640f2e45c 100644 --- a/src/EditorFeatures/Text/ITextImageHelpers.cs +++ b/src/EditorFeatures/Text/ITextImageHelpers.cs @@ -20,15 +20,18 @@ internal static class ITextImageHelpers private static readonly Func s_backwardTextChangeRange = c => CreateTextChangeRange(c, forward: false); public static IReadOnlyList GetChangeRanges(ITextImage oldImage, ITextImage newImage) + => GetChangeRanges(oldImage.Version, newImage.Version); + + public static IReadOnlyList GetChangeRanges(ITextImageVersion oldImageVersion, ITextImageVersion newImageVersion) { - var forward = oldImage.Version.VersionNumber <= newImage.Version.VersionNumber; + var forward = oldImageVersion.VersionNumber <= newImageVersion.VersionNumber; - var oldSnapshot = forward ? oldImage : newImage; - var newSnapshot = forward ? oldImage : newImage; + var oldSnapshotVersion = forward ? oldImageVersion : newImageVersion; + var newSnapshotVersion = forward ? newImageVersion : oldImageVersion; INormalizedTextChangeCollection? changes = null; - for (var oldVersion = oldSnapshot.Version; - oldVersion != newSnapshot.Version; + for (var oldVersion = oldSnapshotVersion; + oldVersion != newSnapshotVersion; oldVersion = oldVersion.Next) { if (oldVersion.Changes.Count != 0) @@ -36,9 +39,9 @@ public static IReadOnlyList GetChangeRanges(ITextImage oldImage if (changes != null) { // Oops - more than one "textual" change between these snapshots, bail and try to find smallest changes span - Logger.Log(FunctionId.Workspace_SourceText_GetChangeRanges, s_textLog, oldImage.Version.VersionNumber, newImage.Version.VersionNumber); + Logger.Log(FunctionId.Workspace_SourceText_GetChangeRanges, s_textLog, oldImageVersion.VersionNumber, newImageVersion.VersionNumber); - return [GetChangeRanges(oldSnapshot.Version, newSnapshot.Version, forward)]; + return [GetChangeRanges(oldSnapshotVersion, newSnapshotVersion, forward)]; } else { From 900ed3d2ebc29c5ee74d5e4f7a8d923f56d929cf Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 May 2024 16:25:13 -0700 Subject: [PATCH 20/20] Bring back bt --- ...ynchronousTaggerProvider.TagSource_ProduceTags.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 17eb80f2cece..8a2fca0a6b0b 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -327,7 +327,12 @@ await _visibilityTracker.DelayWhileNonVisibleAsync( if (newTagTrees is null) return null; - var bufferToChanges = ProcessNewTagTrees(spansToTag, oldTagTrees, newTagTrees, cancellationToken); + // 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. Null out our token so nothing + // accidentally attempts to use it. + cancellationToken = CancellationToken.None; + + var bufferToChanges = ProcessNewTagTrees(spansToTag, oldTagTrees, newTagTrees); // Note: assigning to 'State' is completely safe. It is only ever read from the _eventChangeQueue // serial callbacks on the threadpool. @@ -489,10 +494,9 @@ private Task ProduceTagsAsync(TaggerContext context, CancellationToken can private Dictionary ProcessNewTagTrees( ImmutableArray spansToTag, ImmutableDictionary> oldTagTrees, - ImmutableDictionary> newTagTrees, - CancellationToken cancellationToken) + ImmutableDictionary> newTagTrees) { - using (Logger.LogBlock(FunctionId.Tagger_TagSource_ProcessNewTags, cancellationToken)) + using (Logger.LogBlock(FunctionId.Tagger_TagSource_ProcessNewTags, CancellationToken.None)) { var bufferToChanges = new Dictionary();