From 05fdbd88ef94ecf90edc7daad9cc3fb354fa2244 Mon Sep 17 00:00:00 2001 From: usUyGBx <64971385+usUyGBx@users.noreply.github.com> Date: Tue, 7 Dec 2021 02:33:06 +0300 Subject: [PATCH] Send one notifi per seq range in RemoveAll method of AvaloniaList (optimized) (#5754) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Send one notification per sequential range in RemoveAll method of AvaloniaList (optimized) * Simplified * Make unit tests a bit more understandable. Co-authored-by: Gitea Co-authored-by: Dariusz Komosiński Co-authored-by: Steven Kirk Co-authored-by: Steven Kirk --- src/Avalonia.Base/Collections/AvaloniaList.cs | 19 ++- .../Collections/AvaloniaListTests.cs | 152 ++++++++++++++++++ 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Base/Collections/AvaloniaList.cs b/src/Avalonia.Base/Collections/AvaloniaList.cs index 2f1cb2888e..5782159813 100644 --- a/src/Avalonia.Base/Collections/AvaloniaList.cs +++ b/src/Avalonia.Base/Collections/AvaloniaList.cs @@ -503,11 +503,24 @@ namespace Avalonia.Collections { Contract.Requires(items != null); - foreach (var i in items) + var hItems = new HashSet(items); + + int counter = 0; + for (int i = _inner.Count - 1; i >= 0; --i) { - // TODO: Optimize to only send as many notifications as necessary. - Remove(i); + if (hItems.Contains(_inner[i])) + { + counter += 1; + } + else if(counter > 0) + { + RemoveRange(i + 1, counter); + counter = 0; + } } + + if (counter > 0) + RemoveRange(0, counter); } /// diff --git a/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs b/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs index d5ac01a092..f5ae4cc1e0 100644 --- a/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs +++ b/tests/Avalonia.Base.UnitTests/Collections/AvaloniaListTests.cs @@ -357,5 +357,157 @@ namespace Avalonia.Base.UnitTests.Collections Assert.Equal(target, result); } + + [Fact] + public void RemoveAll_Should_Send_Single_Notification_For_Sequential_Range() + { + var target = new AvaloniaList(Enumerable.Range(0, 10).Select(x => $"Item {x}")); + var toRemove = new[] { "Item 5", "Item 6", "Item 7" }; + var raised = 0; + + target.CollectionChanged += (s, e) => + { + Assert.Equal(NotifyCollectionChangedAction.Remove, e.Action); + Assert.Equal(5, e.OldStartingIndex); + Assert.Equal(toRemove, e.OldItems); + ++raised; + }; + + target.RemoveAll(toRemove); + + Assert.Equal(1, raised); + } + + [Fact] + public void RemoveAll_Should_Send_Single_Notification_For_Sequential_Range_With_Duplicate_Source_Items() + { + var items = Enumerable.Range(0, 20).Select(x => $"Item {x / 2}"); + var target = new AvaloniaList(items); + var toRemove = new[] { "Item 5", "Item 6", "Item 7" }; + var raised = 0; + + target.CollectionChanged += (s, e) => + { + Assert.Equal(NotifyCollectionChangedAction.Remove, e.Action); + Assert.Equal(10, e.OldStartingIndex); + + Assert.Equal(new[] + { + "Item 5", + "Item 5", + "Item 6", + "Item 6", + "Item 7", + "Item 7", + }, e.OldItems); + ++raised; + }; + + target.RemoveAll(toRemove); + + Assert.Equal(1, raised); + } + + [Fact] + public void RemoveAll_Should_Send_Multiple_Notifications_For_Non_Sequential_Range() + { + var target = new AvaloniaList(Enumerable.Range(0, 10).Select(x => $"Item {x}")); + var raised = 0; + var toRemove = new[] + { + new[] { "Item 2", "Item 3" }, + new[] { "Item 5", "Item 6" } + }; + + target.CollectionChanged += (s, e) => + { + Assert.Equal(NotifyCollectionChangedAction.Remove, e.Action); + + if (raised == 0) + { + Assert.Equal(5, e.OldStartingIndex); + Assert.Equal(toRemove[1], e.OldItems); + } + else + { + Assert.Equal(2, e.OldStartingIndex); + Assert.Equal(toRemove[0], e.OldItems); + } + + ++raised; + }; + + target.RemoveAll(toRemove[0].Concat(toRemove[1])); + + Assert.Equal(2, raised); + } + + [Fact] + public void RemoveAll_Should_Send_Multiple_Notifications_For_Sequential_Range_With_Nonsequential_Duplicate_Source_Items() + { + var items = Enumerable.Range(0, 10).Select(x => $"Item {x}"); + var target = new AvaloniaList(items.Concat(items)); + var raised = 0; + var toRemove = new[] { "Item 5", "Item 6", "Item 7" }; + + target.CollectionChanged += (s, e) => + { + Assert.Equal(NotifyCollectionChangedAction.Remove, e.Action); + + if (raised == 0) + { + Assert.Equal(15, e.OldStartingIndex); + Assert.Equal(toRemove, e.OldItems); + } + else + { + Assert.Equal(5, e.OldStartingIndex); + Assert.Equal(toRemove, e.OldItems); + } + + ++raised; + }; + + target.RemoveAll(toRemove); + + Assert.Equal(2, raised); + } + + [Fact] + public void RemoveAll_Should_Not_Send_Notification_For_Items_Not_Present() + { + var target = new AvaloniaList(Enumerable.Range(0, 10).Select(x => $"Item {x}")); + var toRemove = new[] { "Item 5", "Item 6", "Item 7", "Not present" }; + var raised = 0; + + target.CollectionChanged += (s, e) => + { + Assert.Equal(NotifyCollectionChangedAction.Remove, e.Action); + Assert.Equal(5, e.OldStartingIndex); + Assert.Equal(toRemove.Take(3).ToArray(), e.OldItems); + ++raised; + }; + + target.RemoveAll(toRemove); + + Assert.Equal(1, raised); + } + + [Fact] + public void RemoveAll_Should_Handle_Empty_List() + { + var target = new AvaloniaList(); + var toRemove = new[] { "Item 5", "Item 6", "Item 7" }; + var raised = 0; + + target.CollectionChanged += (s, e) => + { + ++raised; + }; + + target.RemoveAll(toRemove); + + Assert.Equal(0, raised); + } } }