Pool collections during tagging. #73708
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -171,6 +172,7 @@ private sealed partial class 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.
needs to be an instance member as it creates hashsets that use 'this' as the special IEqualityComparer to compare tag-spans.
| => 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.
this clause was added as a fast path. The rest was just broken into separate clauses.
| 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.
lots of wasteful linq/enumerator code here. it all got changes to simple code using pools.
CyrusNajmabadi
changed the title
|
@ToddGrun this is ready for review. |
| @@ -24,13 +24,14 @@ namespace Microsoft.CodeAnalysis.Editor.Shared.Tagging; | |||
| internal sealed partial class TagSpanIntervalTree<TTag>( | |||
There was a problem hiding this comment.
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).
|
@ToddGrun this isready for review. |
| @@ -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.
probably :) i can try to address in followup pr.
| 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.
ArrayBuilder.First() is fine. it's not linq :)
Followup to #73703.
Drops memory allocations from:
to just
A dropping from 98mb of allocs in the ide tagger to just 20 (an 80% decrease).