Skip to content
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.

Sign up for GitHub

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

Switch to a List as the backing store for the FAR window from an ImmutableList. #73589

Merged
merged 2 commits into from
May 20, 2024
Merged

Switch to a List as the backing store for the FAR window from an ImmutableList. #73589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bb61ee
Switch to a List as the backing store for the FAR window from an Immu…
CyrusNajmabadi May 20, 2024
13eb1a9
Merge branch 'main' into farArray
CyrusNajmabadi May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions ...VisualStudio/Core/Def/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ private abstract class AbstractTableDataSourceFindUsagesContext :
/// </summary>
private bool _currentlyGroupingByDefinition;

protected ImmutableList<Entry> EntriesWhenNotGroupingByDefinition = ImmutableList<Entry>.Empty;
protected ImmutableList<Entry> EntriesWhenGroupingByDefinition = ImmutableList<Entry>.Empty;
protected readonly List<Entry> EntriesWhenNotGroupingByDefinition = [];
protected readonly List<Entry> EntriesWhenGroupingByDefinition = [];

private TableEntriesSnapshot? _lastSnapshot;
public int CurrentVersionNumber { get; protected set; }
Expand Down Expand Up @@ -182,6 +182,13 @@ protected AbstractTableDataSourceFindUsagesContext(
protected abstract ValueTask OnDefinitionFoundWorkerAsync(DefinitionItem definition, CancellationToken cancellationToken);
protected abstract ValueTask OnReferenceFoundWorkerAsync(SourceReferenceItem reference, CancellationToken cancellationToken);

protected static void AddRange<T>(List<T> list, ArrayBuilder<T> builder)
{
list.Capacity = list.Count + builder.Count;
foreach (var item in builder)
list.Add(item);
}

private static ImmutableArray<string> SelectCustomColumnsToInclude(ImmutableArray<ITableColumnDefinition> customColumns, bool includeContainingTypeAndMemberColumns, bool includeKindColumn)
{
var customColumnsToInclude = ArrayBuilder<string>.GetInstance();
Expand Down Expand Up @@ -570,10 +577,10 @@ public ITableEntriesSnapshot GetCurrentSnapshot()
// Otherwise return the appropriate list based on how we're currently
// grouping.
var entries = _cleared
? ImmutableList<Entry>.Empty
? []
: _currentlyGroupingByDefinition
? EntriesWhenGroupingByDefinition
: EntriesWhenNotGroupingByDefinition;
? EntriesWhenGroupingByDefinition.ToImmutableArray()
: EntriesWhenNotGroupingByDefinition.ToImmutableArray();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we make a snapshot to actually put into TableEntriesSnapshot. But this should be ok the vast majority of hte time. We're only updating this snapshot at most 4 times a second. So the amount of copies made by this is very small (didn't show up in any measurements). And the wins we get by just working with List vs an ImmutableList (which internally is an AVL tree) is substantive.


_lastSnapshot = new TableEntriesSnapshot(entries, CurrentVersionNumber);
}
Expand Down
15 changes: 10 additions & 5 deletions src/VisualStudio/Core/Def/FindReferences/Contexts/WithReferencesFindUsagesContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private async Task AddDeclarationEntriesAsync(DefinitionItem definition, bool ex
{
// We only include declaration entries in the entries we show when
// not grouping by definition.
EntriesWhenNotGroupingByDefinition = EntriesWhenNotGroupingByDefinition.AddRange(entries);
AddRange(EntriesWhenNotGroupingByDefinition, entries);
CurrentVersionNumber++;
changed = true;
}
Expand All @@ -99,8 +99,13 @@ private bool HasDeclarationEntries(DefinitionItem definition)
{
lock (Gate)
{
return EntriesWhenNotGroupingByDefinition.Any(
e => e.DefinitionBucket.DefinitionItem == definition);
foreach (var entry in EntriesWhenNotGroupingByDefinition)
{
if (entry.DefinitionBucket.DefinitionItem == definition)
return true;
}

return false;
}
}

Expand Down Expand Up @@ -152,10 +157,10 @@ private async ValueTask OnEntryFoundAsync(
{
// Once we can make the new entry, add it to the appropriate list.
if (addToEntriesWhenGroupingByDefinition)
EntriesWhenGroupingByDefinition = EntriesWhenGroupingByDefinition.Add(entry);
EntriesWhenGroupingByDefinition.Add(entry);

if (addToEntriesWhenNotGroupingByDefinition)
EntriesWhenNotGroupingByDefinition = EntriesWhenNotGroupingByDefinition.Add(entry);
EntriesWhenNotGroupingByDefinition.Add(entry);
}

CurrentVersionNumber++;
Expand Down
8 changes: 4 additions & 4 deletions src/VisualStudio/Core/Def/FindReferences/Contexts/WithoutReferencesFindUsagesContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ private async Task CreateNoResultsFoundEntryIfNecessaryAsync()

lock (Gate)
{
EntriesWhenGroupingByDefinition = EntriesWhenGroupingByDefinition.Add(entry);
EntriesWhenNotGroupingByDefinition = EntriesWhenNotGroupingByDefinition.Add(entry);
EntriesWhenGroupingByDefinition.Add(entry);
EntriesWhenNotGroupingByDefinition.Add(entry);
CurrentVersionNumber++;
}

Expand Down Expand Up @@ -108,8 +108,8 @@ protected override async ValueTask OnDefinitionFoundWorkerAsync(DefinitionItem d
{
lock (Gate)
{
EntriesWhenGroupingByDefinition = EntriesWhenGroupingByDefinition.AddRange(entries);
EntriesWhenNotGroupingByDefinition = EntriesWhenNotGroupingByDefinition.AddRange(entries);
AddRange(EntriesWhenGroupingByDefinition, entries);
AddRange(EntriesWhenNotGroupingByDefinition, entries);
CurrentVersionNumber++;
}

Expand Down
15 changes: 4 additions & 11 deletions src/VisualStudio/Core/Def/FindReferences/TableEntriesSnapshot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,14 @@ internal partial class StreamingFindUsagesPresenter
// Name of the key used to retireve the whole entry object.
internal const string SelfKeyName = "self";

private class TableEntriesSnapshot : WpfTableEntriesSnapshotBase
private class TableEntriesSnapshot(ImmutableArray<Entry> entries, int versionNumber) : WpfTableEntriesSnapshotBase
{
private readonly int _versionNumber;

private readonly ImmutableList<Entry> _entries;

public TableEntriesSnapshot(ImmutableList<Entry> entries, int versionNumber)
{
_entries = entries;
_versionNumber = versionNumber;
}
private readonly int _versionNumber = versionNumber;
private readonly ImmutableArray<Entry> _entries = entries;

public override int VersionNumber => _versionNumber;

public override int Count => _entries.Count;
public override int Count => _entries.Length;

public override int IndexOf(int currentIndex, ITableEntriesSnapshot newSnapshot)
{
Expand Down
Loading