Skip to content

Commit 8624a25

Browse files
madskristensenmadskristensen
committed
Optimize error handling and span invalidation efficiency
--- - 🚀 Switched snapshots to Dictionary for O(1) lookups in SinkManager - 🛠️ Improved AddErrors to filter and group errors efficiently in TableDataSource - ✨ Minimized TagsChanged invalidation region in SameWordHighlighterBase for better UI performance - ♻️ Added GC.SuppressFinalize in SinkManager.Dispose for improved resource management [release]
1 parent 87d95e9 commit 8624a25

3 files changed

Lines changed: 125 additions & 21 deletions

File tree

‎src/toolkit/Community.VisualStudio.Toolkit.Shared/ErrorList/SinkManager.cs‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
using System;
1+
using System;
22
using System.Collections.Generic;
3-
using System.Linq;
43
using Microsoft.VisualStudio.Shell.TableManager;
54

65
namespace Community.VisualStudio.Toolkit
@@ -23,9 +22,9 @@ internal class SinkManager : IDisposable
2322
private readonly Action<SinkManager> _onDispose;
2423

2524
/// <summary>
26-
/// Snapshot collection
25+
/// Snapshot collection indexed by FilePath for O(1) lookups
2726
/// </summary>
28-
private readonly List<TableEntriesSnapshot> _snapshots = new();
27+
private readonly Dictionary<string, TableEntriesSnapshot> _snapshots = new();
2928

3029
/// <summary>
3130
/// Constructor
@@ -46,19 +45,17 @@ public void UpdateSink(IEnumerable<TableEntriesSnapshot> snapshots)
4645
{
4746
foreach (TableEntriesSnapshot snapshot in snapshots)
4847
{
49-
TableEntriesSnapshot existing = _snapshots.FirstOrDefault(s => s.FilePath == snapshot.FilePath);
50-
51-
if (existing != null)
48+
string filePath = snapshot.FilePath;
49+
if (_snapshots.TryGetValue(filePath, out TableEntriesSnapshot? existing))
5250
{
53-
_snapshots.Remove(existing);
5451
_sink.ReplaceSnapshot(existing, snapshot);
52+
_snapshots[filePath] = snapshot;
5553
}
5654
else
5755
{
5856
_sink.AddSnapshot(snapshot);
57+
_snapshots[filePath] = snapshot;
5958
}
60-
61-
_snapshots.Add(snapshot);
6259
}
6360
}
6461

@@ -87,6 +84,7 @@ protected virtual void Dispose(bool disposing)
8784
public void Dispose()
8885
{
8986
Dispose(true);
87+
GC.SuppressFinalize(this);
9088
}
9189
}
9290
}

‎src/toolkit/Community.VisualStudio.Toolkit.Shared/ErrorList/TableDataSource.cs‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,25 +133,45 @@ private void UpdateAllSinks()
133133
/// </summary>
134134
public void AddErrors(IEnumerable<ErrorListItem> errors)
135135
{
136-
if (errors == null || !errors.Any())
136+
if (errors == null)
137137
{
138138
return;
139139
}
140140

141-
string? projectName = errors.FirstOrDefault(e => !string.IsNullOrEmpty(e.ProjectName))?.ProjectName ?? "";
141+
// Materialize once to avoid multiple enumeration - filter nulls and empty filenames
142+
List<ErrorListItem> errorList = new();
143+
string? projectName = null;
142144

143-
IEnumerable<ErrorListItem> cleanErrors = errors.Where(e => e != null && !string.IsNullOrEmpty(e.FileName));
145+
foreach (ErrorListItem? error in errors)
146+
{
147+
if (error != null && !string.IsNullOrEmpty(error.FileName))
148+
{
149+
errorList.Add(error);
150+
if (projectName == null && !string.IsNullOrEmpty(error.ProjectName))
151+
{
152+
projectName = error.ProjectName;
153+
}
154+
}
155+
}
156+
157+
if (errorList.Count == 0)
158+
{
159+
return;
160+
}
161+
162+
projectName ??= "";
144163

145164
lock (_syncLock)
146165
{
147-
foreach (IGrouping<string?, ErrorListItem>? fileErrorMap in cleanErrors.GroupBy(e => e.FileName))
166+
// Group already uses the materialized list
167+
foreach (IGrouping<string?, ErrorListItem>? fileErrorMap in errorList.GroupBy(e => e.FileName))
148168
{
149169
if (fileErrorMap.Key != null)
150170
{
151-
if (_snapshots.ContainsKey(fileErrorMap.Key))
171+
if (_snapshots.TryGetValue(fileErrorMap.Key, out TableEntriesSnapshot? existingSnapshot))
152172
{
153-
IEnumerable<ErrorListItem> values = cleanErrors.Where(e => e.FileName == fileErrorMap.Key);
154-
_snapshots[fileErrorMap.Key].Update(values);
173+
// Use the group directly instead of re-filtering
174+
existingSnapshot.Update(fileErrorMap);
155175
}
156176
else
157177
{

‎src/toolkit/Community.VisualStudio.Toolkit.Shared/MEF/SameWordHighlighterBase.cs‎

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,14 @@ private void ClearSpans()
143143
{
144144
lock (_syncLock)
145145
{
146+
NormalizedSnapshotSpanCollection oldSpans = _wordSpans;
147+
SnapshotSpan? oldWord = _currentWord;
148+
146149
_currentWord = null;
147150
_wordSpans = new();
148-
SnapshotSpan span = new(_buffer.CurrentSnapshot, 0, _buffer.CurrentSnapshot.Length);
149-
TagsChanged?.Invoke(this, new SnapshotSpanEventArgs(span));
151+
152+
// Compute minimal invalidation span instead of entire document
153+
RaiseTagsChangedForDifference(oldSpans, oldWord, _wordSpans, _currentWord);
150154
}
151155
}
152156
private void UpdateAtCaretPosition(CaretPosition caretPosition)
@@ -227,11 +231,93 @@ private void SynchronousUpdate(SnapshotPoint currentRequest, NormalizedSnapshotS
227231
return;
228232
}
229233

234+
NormalizedSnapshotSpanCollection oldSpans = _wordSpans;
235+
SnapshotSpan? oldWord = _currentWord;
236+
230237
_wordSpans = newSpans;
231238
_currentWord = newCurrentWord;
232239

233-
SnapshotSpan span = new(_buffer.CurrentSnapshot, 0, _buffer.CurrentSnapshot.Length);
234-
TagsChanged?.Invoke(this, new SnapshotSpanEventArgs(span));
240+
// Compute minimal invalidation span instead of entire document
241+
RaiseTagsChangedForDifference(oldSpans, oldWord, newSpans, newCurrentWord);
242+
}
243+
}
244+
245+
/// <summary>
246+
/// Raises TagsChanged for only the spans that actually changed, rather than the entire document.
247+
/// </summary>
248+
private void RaiseTagsChangedForDifference(
249+
NormalizedSnapshotSpanCollection oldSpans, SnapshotSpan? oldWord,
250+
NormalizedSnapshotSpanCollection newSpans, SnapshotSpan? newWord)
251+
{
252+
if (TagsChanged == null)
253+
{
254+
return;
255+
}
256+
257+
ITextSnapshot currentSnapshot = _buffer.CurrentSnapshot;
258+
259+
// If both are empty/null, no change needed
260+
if ((oldSpans == null || oldSpans.Count == 0) && oldWord == null &&
261+
(newSpans == null || newSpans.Count == 0) && newWord == null)
262+
{
263+
return;
264+
}
265+
266+
// Compute the bounding span that covers all changes
267+
int minStart = int.MaxValue;
268+
int maxEnd = int.MinValue;
269+
270+
// Include old spans
271+
if (oldSpans != null && oldSpans.Count > 0)
272+
{
273+
foreach (SnapshotSpan span in oldSpans)
274+
{
275+
SnapshotSpan translated = span.Snapshot == currentSnapshot
276+
? span
277+
: span.TranslateTo(currentSnapshot, SpanTrackingMode.EdgeInclusive);
278+
minStart = Math.Min(minStart, translated.Start.Position);
279+
maxEnd = Math.Max(maxEnd, translated.End.Position);
280+
}
281+
}
282+
283+
// Include old word
284+
if (oldWord.HasValue)
285+
{
286+
SnapshotSpan translated = oldWord.Value.Snapshot == currentSnapshot
287+
? oldWord.Value
288+
: oldWord.Value.TranslateTo(currentSnapshot, SpanTrackingMode.EdgeInclusive);
289+
minStart = Math.Min(minStart, translated.Start.Position);
290+
maxEnd = Math.Max(maxEnd, translated.End.Position);
291+
}
292+
293+
// Include new spans
294+
if (newSpans != null && newSpans.Count > 0)
295+
{
296+
foreach (SnapshotSpan span in newSpans)
297+
{
298+
SnapshotSpan translated = span.Snapshot == currentSnapshot
299+
? span
300+
: span.TranslateTo(currentSnapshot, SpanTrackingMode.EdgeInclusive);
301+
minStart = Math.Min(minStart, translated.Start.Position);
302+
maxEnd = Math.Max(maxEnd, translated.End.Position);
303+
}
304+
}
305+
306+
// Include new word
307+
if (newWord.HasValue)
308+
{
309+
SnapshotSpan translated = newWord.Value.Snapshot == currentSnapshot
310+
? newWord.Value
311+
: newWord.Value.TranslateTo(currentSnapshot, SpanTrackingMode.EdgeInclusive);
312+
minStart = Math.Min(minStart, translated.Start.Position);
313+
maxEnd = Math.Max(maxEnd, translated.End.Position);
314+
}
315+
316+
// If we found any spans, raise the event for the bounding region
317+
if (minStart <= maxEnd && minStart != int.MaxValue)
318+
{
319+
SnapshotSpan changedSpan = new(currentSnapshot, minStart, maxEnd - minStart);
320+
TagsChanged.Invoke(this, new SnapshotSpanEventArgs(changedSpan));
235321
}
236322
}
237323

0 commit comments

Comments
 (0)