-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pool collections during tagging. #73708
Changes from all commits
0bd3644
42edaf7
725c6c3
201ce9f
8bf177f
d756da3
856b297
9d3d82d
3c36645
cc6177b
aa78f76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
using Microsoft.CodeAnalysis.Editor.Shared.Extensions; | ||
using Microsoft.CodeAnalysis.Editor.Shared.Tagging; | ||
using Microsoft.CodeAnalysis.Options; | ||
using Microsoft.CodeAnalysis.PooledObjects; | ||
using Microsoft.CodeAnalysis.Shared.TestHooks; | ||
using Microsoft.CodeAnalysis.Text; | ||
using Microsoft.CodeAnalysis.Workspaces; | ||
|
@@ -171,6 +172,7 @@ public TagSource( | |
_dataSource = dataSource; | ||
_asyncListener = asyncListener; | ||
_nonFrozenComputationCancellationSeries = new(_disposalTokenSource.Token); | ||
_tagSpanSetPool = new ObjectPool<HashSet<ITagSpan<TTag>>>(() => new HashSet<ITagSpan<TTag>>(this), trimOnFree: false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needs to be an instance member as it creates hashsets that use 'this' as the special IEqualityComparer to compare tag-spans. |
||
|
||
_workspaceRegistration = Workspace.GetWorkspaceRegistration(subjectBuffer.AsTextContainer()); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Collections.Generic; | ||
using Microsoft.CodeAnalysis.PooledObjects; | ||
using Microsoft.VisualStudio.Text.Tagging; | ||
|
||
namespace Microsoft.CodeAnalysis.Editor.Tagging; | ||
|
@@ -11,8 +12,18 @@ internal abstract partial class AbstractAsynchronousTaggerProvider<TTag> | |
{ | ||
private partial class TagSource : IEqualityComparer<ITagSpan<TTag>> | ||
{ | ||
private readonly ObjectPool<HashSet<ITagSpan<TTag>>> _tagSpanSetPool; | ||
|
||
public bool Equals(ITagSpan<TTag>? x, ITagSpan<TTag>? y) | ||
=> x != null && y != null && x.Span == y.Span && _dataSource.TagEquals(x.Tag, y.Tag); | ||
{ | ||
if (x == y) | ||
return true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this clause was added as a fast path. The rest was just broken into separate clauses. |
||
|
||
if (x is null || y is null) | ||
return false; | ||
|
||
return x.Span == y.Span && _dataSource.TagEquals(x.Tag, y.Tag); | ||
} | ||
|
||
/// <summary> | ||
/// For the purposes of hashing, just hash spans. This will prevent most collisions. And the rare | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -389,52 +389,56 @@ private static void CheckSnapshot(ITextSnapshot snapshot) | |
|
||
private ImmutableDictionary<ITextBuffer, TagSpanIntervalTree<TTag>> ComputeNewTagTrees(ImmutableDictionary<ITextBuffer, TagSpanIntervalTree<TTag>> oldTagTrees, TaggerContext<TTag> context) | ||
{ | ||
// Ignore any tag spans reported for any buffers we weren't interested in. | ||
|
||
var spansToTag = context.SpansToTag; | ||
var buffersToTag = spansToTag.Select(dss => dss.SnapshotSpan.Snapshot.TextBuffer).ToSet(); | ||
var newTagsByBuffer = | ||
context.TagSpans.Where(ts => buffersToTag.Contains(ts.Span.Snapshot.TextBuffer)) | ||
.ToLookup(t => t.Span.Snapshot.TextBuffer); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lots of wasteful linq/enumerator code here. it all got changes to simple code using pools. |
||
var spansTagged = context._spansTagged; | ||
|
||
var spansToInvalidateByBuffer = spansTagged.ToLookup( | ||
keySelector: span => span.Snapshot.TextBuffer, | ||
elementSelector: span => span); | ||
|
||
// Walk through each relevant buffer and decide what the interval tree should be | ||
// 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<ITextBuffer, TagSpanIntervalTree<TTag>>.Empty; | ||
using var _1 = PooledHashSet<ITextBuffer>.GetInstance(out var buffersToTag); | ||
foreach (var spanToTag in context.SpansToTag) | ||
buffersToTag.Add(spanToTag.SnapshotSpan.Snapshot.TextBuffer); | ||
|
||
using var _2 = ArrayBuilder<ITagSpan<TTag>>.GetInstance(out var newTagsInBuffer); | ||
using var _3 = ArrayBuilder<SnapshotSpan>.GetInstance(out var spansToInvalidateInBuffer); | ||
|
||
var newTagTrees = ImmutableDictionary.CreateBuilder<ITextBuffer, TagSpanIntervalTree<TTag>>(); | ||
foreach (var buffer in buffersToTag) | ||
{ | ||
var newTagTree = ComputeNewTagTree(oldTagTrees, buffer, newTagsByBuffer[buffer], spansToInvalidateByBuffer[buffer]); | ||
newTagsInBuffer.Clear(); | ||
spansToInvalidateInBuffer.Clear(); | ||
|
||
// Ignore any tag spans reported for any buffers we weren't interested in. | ||
|
||
foreach (var tagSpan in context.TagSpans) | ||
{ | ||
if (tagSpan.Span.Snapshot.TextBuffer == buffer) | ||
newTagsInBuffer.Add(tagSpan); | ||
} | ||
|
||
foreach (var span in context._spansTagged) | ||
{ | ||
if (span.Snapshot.TextBuffer == buffer) | ||
spansToInvalidateInBuffer.Add(span); | ||
} | ||
|
||
var newTagTree = ComputeNewTagTree(oldTagTrees, buffer, newTagsInBuffer, spansToInvalidateInBuffer); | ||
if (newTagTree != null) | ||
newTagTrees = newTagTrees.Add(buffer, newTagTree); | ||
newTagTrees.Add(buffer, newTagTree); | ||
} | ||
|
||
return newTagTrees; | ||
return newTagTrees.ToImmutable(); | ||
} | ||
|
||
private TagSpanIntervalTree<TTag>? ComputeNewTagTree( | ||
ImmutableDictionary<ITextBuffer, TagSpanIntervalTree<TTag>> oldTagTrees, | ||
ITextBuffer textBuffer, | ||
IEnumerable<ITagSpan<TTag>> newTags, | ||
IEnumerable<SnapshotSpan> spansToInvalidate) | ||
ArrayBuilder<ITagSpan<TTag>> newTags, | ||
ArrayBuilder<SnapshotSpan> spansToInvalidate) | ||
{ | ||
var noNewTags = newTags.IsEmpty(); | ||
var noSpansToInvalidate = spansToInvalidate.IsEmpty(); | ||
var noNewTags = newTags.IsEmpty; | ||
var noSpansToInvalidate = spansToInvalidate.IsEmpty; | ||
oldTagTrees.TryGetValue(textBuffer, out var oldTagTree); | ||
|
||
if (oldTagTree == null) | ||
{ | ||
// If we have no new tags, and no old tags either. No need to store anything for this buffer. | ||
if (noNewTags) | ||
{ | ||
// We have no new tags, and no old tags either. No need to store anything | ||
// for this buffer. | ||
return null; | ||
} | ||
|
||
// If we don't have any old tags then we just need to return the new tags. | ||
return new TagSpanIntervalTree<TTag>(textBuffer, _dataSource.SpanTrackingMode, newTags); | ||
|
@@ -443,35 +447,40 @@ private ImmutableDictionary<ITextBuffer, TagSpanIntervalTree<TTag>> ComputeNewTa | |
// If we don't have any new tags, and there was nothing to invalidate, then we can | ||
// keep whatever old tags we have without doing any additional work. | ||
if (noNewTags && noSpansToInvalidate) | ||
{ | ||
return oldTagTree; | ||
} | ||
|
||
// We either have some new tags, or we have some tags to invalidate. | ||
// First, determine which of the old tags we want to keep around. | ||
var snapshot = noNewTags ? spansToInvalidate.First().Snapshot : newTags.First().Span.Snapshot; | ||
var oldTagsToKeep = noSpansToInvalidate | ||
? oldTagTree.GetSpans(snapshot) | ||
: GetNonIntersectingTagSpans(spansToInvalidate, oldTagTree); | ||
|
||
// Then union those with the new tags to produce the final tag tree. | ||
var finalTags = oldTagsToKeep.Concat(newTags); | ||
return new TagSpanIntervalTree<TTag>(textBuffer, _dataSource.SpanTrackingMode, finalTags); | ||
if (noSpansToInvalidate) | ||
{ | ||
// If we have no spans to invalidate, then we can just keep the old tags and add the new tags. | ||
var oldTagsToKeep = oldTagTree.GetSpans(newTags.First().Span.Snapshot); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ArrayBuilder.First() is fine. it's not linq :) |
||
return new TagSpanIntervalTree<TTag>( | ||
textBuffer, _dataSource.SpanTrackingMode, oldTagsToKeep, newTags); | ||
} | ||
else | ||
{ | ||
// We do have spans to invalidate. Get the set of old tags that don't intersect with those and add the new tags. | ||
using var _1 = _tagSpanSetPool.GetPooledObject(out var nonIntersectingTagSpans); | ||
AddNonIntersectingTagSpans(spansToInvalidate, oldTagTree, nonIntersectingTagSpans); | ||
return new TagSpanIntervalTree<TTag>( | ||
textBuffer, _dataSource.SpanTrackingMode, nonIntersectingTagSpans, newTags); | ||
} | ||
} | ||
|
||
private IEnumerable<ITagSpan<TTag>> GetNonIntersectingTagSpans(IEnumerable<SnapshotSpan> spansToInvalidate, TagSpanIntervalTree<TTag> oldTagTree) | ||
private static void AddNonIntersectingTagSpans( | ||
ArrayBuilder<SnapshotSpan> spansToInvalidate, | ||
TagSpanIntervalTree<TTag> oldTagTree, | ||
HashSet<ITagSpan<TTag>> nonIntersectingTagSpans) | ||
{ | ||
var firstSpanToInvalidate = spansToInvalidate.First(); | ||
var snapshot = firstSpanToInvalidate.Snapshot; | ||
|
||
// Performance: No need to fully realize spansToInvalidate or do any of the calculations below if the | ||
// full snapshot is being invalidated. | ||
if (firstSpanToInvalidate.Length == snapshot.Length) | ||
return []; | ||
return; | ||
|
||
return oldTagTree.GetSpans(snapshot).Except( | ||
spansToInvalidate.SelectMany(oldTagTree.GetIntersectingSpans), | ||
comparer: this); | ||
oldTagTree.AddAllSpans(snapshot, nonIntersectingTagSpans); | ||
oldTagTree.RemoveIntersectingTagSpans(spansToInvalidate, nonIntersectingTagSpans); | ||
} | ||
|
||
private bool ShouldSkipTagProduction() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,7 +152,7 @@ public static void ClearAndFree<T>(this ObjectPool<HashSet<T>> pool, HashSet<T> | |
var count = set.Count; | ||
set.Clear(); | ||
|
||
if (count > Threshold) | ||
if (count > Threshold && pool.TrimOnFree) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably :) i can try to address in followup pr. |
||
{ | ||
set.TrimExcess(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: my plan fo rthis type is to get rid of it, inlining its underlying IntervalTree into the tagger directly. The helpers we have here would then be static helper methods in the tagger as they're all tagger specific helpers (most of which can be improved).