Drop roslyn impact on scrolling perf by 30% #73648
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -1198,7 +1198,7 @@ public void Sort(Comparison<T> comparison) | |||
|
|
|||
| if (_size > 1) | |||
| { | |||
| SegmentedArray.Sort<T>(_items, 0, _size, Comparer<T>.Create(comparison)); | |||
| SegmentedArray.Sort(_items, 0, _size, comparison); | |||
There was a problem hiding this comment.
existing codepath forced an allocation from comparison to comparer. That comparer was later converted back to a comparison by grabbing off the .Compare method from it. So this double allocated going through this path.
THe path that takes an IComparer single-allocs (tearing off the .Compare delegate).
Now, there is at least no alloc if passing in a comparison delegate. we pass it through all the way to the final place that uses it.
There was a problem hiding this comment.
Allocations caused by this contributed to nearly half of the excess costs here (as well as a ton of garbage):
| @@ -394,6 +394,22 @@ public static void Sort<T>(SegmentedArray<T> array, int index, int length, IComp | |||
| } | |||
| } | |||
|
|
|||
| public static void Sort<T>(SegmentedArray<T> array, int index, int length, Comparison<T> comparison) | |||
There was a problem hiding this comment.
copy of method above, just taking and passing through a Comparison instead of an IComparer. See https://github.com/dotnet/roslyn/pull/73648/files#r1610644205 for more details.
There was a problem hiding this comment.
To better match the reference implementation (from dotnet/runtime), let's remove this method and inline these two lines to the caller:
roslyn/src/Dependencies/Collections/SegmentedArray.cs
Lines 408 to 409 in 1eba0d9
| @@ -90,7 +90,7 @@ public IEnumerable<ITagSpan<IClassificationTag>> GetTags(NormalizedSnapshotSpanC | |||
| } | |||
|
|
|||
| private static IEnumerable<ITagSpan<IClassificationTag>> GetIntersectingTags(NormalizedSnapshotSpanCollection spans, TagSpanIntervalTree<IClassificationTag> cachedTags) | |||
| => SegmentedListPool<ITagSpan<IClassificationTag>>.ComputeList( | |||
| => SegmentedListPool<TagSpan<IClassificationTag>>.ComputeList( | |||
There was a problem hiding this comment.
switched everything to be TagSpan based. Avoids lots and lots and lots and lots of virtual dispatch. Everything is non-virtual on the impl type, allowing for major inlining.
|
@sharwell for eyes on the Segmented collections part. |
|
@ToddGrun ptal. |
CyrusNajmabadi
changed the title
| @@ -192,8 +194,7 @@ public async Task AddSyntacticClassificationsAsync(Document document, ImmutableA | |||
| if (root is null) | |||
| return; | |||
|
|
|||
| var classificationService = services.GetLanguageServices(root.Language).GetRequiredService<ISyntaxClassificationService>(); | |||
There was a problem hiding this comment.
service-retrieval continues to be surprising expensive when done in hot spots. here we can trivially lift this out such that thsi value is provided in the constructor.
Remove an overload from SegmentedArray that does not have a corresponding method in System.Array in favor of directly interacting with SegmentedArraySortHelper. Addresses feedback in dotnet#73648
Takes us from:
to