Merge pull request #3552 from BDisp/v2_3551_combobox-listview-performance-fix

Fixes #3551. v2: Big performance impact on ListView/ComboBox's that contain a lot of items
This commit is contained in:
Tig
2024-06-20 16:01:40 -07:00
committed by GitHub
4 changed files with 149 additions and 5 deletions

View File

@@ -187,7 +187,7 @@ public class ComboBox : View
{
SelectedItem = -1;
_search.Text = string.Empty;
Search_Changed (this, new StateEventArgs<string> (string.Empty, _search.Text));
Search_Changed (this, new StateEventArgs<string> (string.Empty, _search.Text));
SetNeedsDisplay ();
}
}
@@ -645,7 +645,9 @@ public class ComboBox : View
private void ResetSearchSet (bool noCopy = false)
{
_listview.SuspendCollectionChangedEvent ();
_searchSet.Clear ();
_listview.ResumeSuspendCollectionChangedEvent ();
if (_autoHide || noCopy)
{
@@ -682,6 +684,8 @@ public class ComboBox : View
if (!string.IsNullOrEmpty (_search.Text))
{
_listview.SuspendCollectionChangedEvent ();
foreach (object item in _source.ToList ())
{
// Iterate to preserver object type and force deep copy
@@ -694,6 +698,8 @@ public class ComboBox : View
_searchSet.Add (item);
}
}
_listview.ResumeSuspendCollectionChangedEvent ();
}
}
@@ -738,11 +744,16 @@ public class ComboBox : View
return;
}
// PERF: At the request of @dodexahedron in the comment https://github.com/gui-cs/Terminal.Gui/pull/3552#discussion_r1648112410.
_listview.SuspendCollectionChangedEvent ();
// force deep copy
foreach (object item in Source.ToList ())
{
_searchSet.Add (item);
}
_listview.ResumeSuspendCollectionChangedEvent ();
}
private void SetSearchText (string value) { _search.Text = _text = value; }
@@ -765,8 +776,11 @@ public class ComboBox : View
/// Consider making public
private void ShowList ()
{
_listview.SuspendCollectionChangedEvent ();
_listview.SetSource (_searchSet);
_listview.Clear ();
_listview.ResumeSuspendCollectionChangedEvent ();
_listview.Clear ();
_listview.Height = CalculatetHeight ();
SuperView?.BringSubviewToFront (this);
}

View File

@@ -19,6 +19,12 @@ public interface IListDataSource: IDisposable
/// <summary>Returns the maximum length of elements to display</summary>
int Length { get; }
/// <summary>
/// Allow suspending the <see cref="CollectionChanged"/> event from being invoked,
/// if <see langword="true"/>, otherwise is <see langword="false"/>.
/// </summary>
bool SuspendCollectionChangedEvent { get; set; }
/// <summary>Should return whether the specified item is currently marked.</summary>
/// <returns><see langword="true"/>, if marked, <see langword="false"/> otherwise.</returns>
/// <param name="item">Item index.</param>
@@ -893,6 +899,28 @@ public class ListView : View
base.Dispose (disposing);
}
/// <summary>
/// Allow suspending the <see cref="CollectionChanged"/> event from being invoked,
/// </summary>
public void SuspendCollectionChangedEvent ()
{
if (Source is { })
{
Source.SuspendCollectionChangedEvent = true;
}
}
/// <summary>
/// Allow resume the <see cref="CollectionChanged"/> event from being invoked,
/// </summary>
public void ResumeSuspendCollectionChangedEvent ()
{
if (Source is { })
{
Source.SuspendCollectionChangedEvent = false;
}
}
}
/// <summary>
@@ -920,8 +948,11 @@ public class ListWrapper<T> : IListDataSource, IDisposable
private void Source_CollectionChanged (object sender, NotifyCollectionChangedEventArgs e)
{
CheckAndResizeMarksIfRequired ();
CollectionChanged?.Invoke (sender, e);
if (!SuspendCollectionChangedEvent)
{
CheckAndResizeMarksIfRequired ();
CollectionChanged?.Invoke (sender, e);
}
}
/// <inheritdoc />
@@ -933,7 +964,24 @@ public class ListWrapper<T> : IListDataSource, IDisposable
/// <inheritdoc/>
public int Length { get; private set; }
void CheckAndResizeMarksIfRequired ()
private bool _suspendCollectionChangedEvent;
/// <inheritdoc />
public bool SuspendCollectionChangedEvent
{
get => _suspendCollectionChangedEvent;
set
{
_suspendCollectionChangedEvent = value;
if (!_suspendCollectionChangedEvent)
{
CheckAndResizeMarksIfRequired ();
}
}
}
private void CheckAndResizeMarksIfRequired ()
{
if (_source != null && _count != _source.Count)
{

View File

@@ -207,6 +207,7 @@ public class ListViewWithSelection : Scenario
public event NotifyCollectionChangedEventHandler CollectionChanged;
public int Count => Scenarios != null ? Scenarios.Count : 0;
public int Length { get; private set; }
public bool SuspendCollectionChangedEvent { get => throw new System.NotImplementedException (); set => throw new System.NotImplementedException (); }
public void Render (
ListView container,

View File

@@ -679,6 +679,9 @@ Item 6",
public int Count => 0;
public int Length => 0;
public bool SuspendCollectionChangedEvent { get => throw new NotImplementedException (); set => throw new NotImplementedException (); }
public bool IsMarked (int item) { throw new NotImplementedException (); }
public void Render (
@@ -1075,4 +1078,82 @@ Item 6",
}
}
}
[Fact]
public void ListWrapper_SuspendCollectionChangedEvent_ResumeSuspendCollectionChangedEvent_Tests ()
{
var added = 0;
ObservableCollection<string> source = [];
ListWrapper<string> lw = new (source);
lw.CollectionChanged += Lw_CollectionChanged;
lw.SuspendCollectionChangedEvent = true;
for (int i = 0; i < 3; i++)
{
source.Add ($"Item{i}");
}
Assert.Equal (0, added);
Assert.Equal (3, lw.Count);
Assert.Equal (3, source.Count);
lw.SuspendCollectionChangedEvent = false;
for (int i = 3; i < 6; i++)
{
source.Add ($"Item{i}");
}
Assert.Equal (3, added);
Assert.Equal (6, lw.Count);
Assert.Equal (6, source.Count);
void Lw_CollectionChanged (object sender, NotifyCollectionChangedEventArgs e)
{
if (e.Action == NotifyCollectionChangedAction.Add)
{
added++;
}
}
}
[Fact]
public void ListView_SuspendCollectionChangedEvent_ResumeSuspendCollectionChangedEvent_Tests ()
{
var added = 0;
ObservableCollection<string> source = [];
ListView lv = new ListView { Source = new ListWrapper<string> (source) };
lv.CollectionChanged += Lw_CollectionChanged;
lv.SuspendCollectionChangedEvent ();
for (int i = 0; i < 3; i++)
{
source.Add ($"Item{i}");
}
Assert.Equal (0, added);
Assert.Equal (3, lv.Source.Count);
Assert.Equal (3, source.Count);
lv.ResumeSuspendCollectionChangedEvent ();
for (int i = 3; i < 6; i++)
{
source.Add ($"Item{i}");
}
Assert.Equal (3, added);
Assert.Equal (6, lv.Source.Count);
Assert.Equal (6, source.Count);
void Lw_CollectionChanged (object sender, NotifyCollectionChangedEventArgs e)
{
if (e.Action == NotifyCollectionChangedAction.Add)
{
added++;
}
}
}
}