Switch to a HashSet<T> as backing for SafeEnumerableList (#16633)

* Switch to a set as backing for _listeners on Classes. Via the `SafeEnumerableHashSet`

* Update docs

src/Avalonia.Base/Controls/Classes.cs

@@ -13,7 +13,7 @@ namespace Avalonia.Controls
     /// </remarks>
     public class Classes : AvaloniaList<string>, IPseudoClasses
     {
-        private SafeEnumerableList<IClassesChangedListener>? _listeners;
+        private SafeEnumerableHashSet<IClassesChangedListener>? _listeners;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Classes"/> class.

src/Avalonia.Base/Utilities/SafeEnumerableHashSet.cs

@@ -0,0 +1,80 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+
+namespace Avalonia.Utilities
+{
+    /// <summary>
+    /// Implements a simple set which is safe to modify during enumeration.
+    /// </summary>
+    /// <typeparam name="T">The item type.</typeparam>
+    /// <remarks>
+    /// Implements a set which, when written to while enumerating, performs a copy of the set
+    /// items. Note this class doesn't actually implement <see cref="ISet{T}"/> as it's not
+    /// currently needed - feel free to add missing methods etc.
+    /// </remarks>
+    internal class SafeEnumerableHashSet<T> : IEnumerable<T>
+    {
+        private HashSet<T> _hashSet = new();
+        private int _generation;
+        private int _enumCount = 0;
+
+        public int Count => _hashSet.Count;
+        internal HashSet<T> Inner => _hashSet;
+
+        public void Add(T item) => GetSet().Add(item);
+        public bool Remove(T item) => GetSet().Remove(item);
+
+        public Enumerator GetEnumerator() => new(this, _hashSet);
+        IEnumerator<T> IEnumerable<T>.GetEnumerator() => GetEnumerator();
+        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+        
+        private HashSet<T> GetSet()
+        {
+            if (_enumCount > 0)
+            {
+                // .NET has a fastpath for cloning a hashset when passed in via the constructor
+                _hashSet = new(_hashSet);
+                ++_generation;
+                _enumCount = 0;
+            }
+
+            return _hashSet;
+        }
+
+        public struct Enumerator : IEnumerator<T>, IEnumerator
+        {
+            private readonly SafeEnumerableHashSet<T> _owner;
+            private readonly int _generation;
+            private HashSet<T>.Enumerator _enumerator;
+
+            internal Enumerator(SafeEnumerableHashSet<T> owner, HashSet<T> list)
+            {
+                _owner = owner;
+                _generation = owner._generation;
+                ++_owner._enumCount;
+                _enumerator = list.GetEnumerator();
+            }
+
+            public void Dispose()
+            {
+                _enumerator.Dispose();
+                if (_owner._generation == _generation)
+                    --_owner._enumCount;
+            }
+
+            public bool MoveNext()
+            {
+                return _enumerator.MoveNext();
+            }
+
+            public T Current => _enumerator.Current;
+            object? IEnumerator.Current => _enumerator.Current;
+
+            void IEnumerator.Reset()
+            {
+                throw new NotSupportedException();
+            }
+        }
+    }
+}

src/Avalonia.Base/Utilities/SafeEnumerableList.cs

@@ -1,89 +0,0 @@
-using System.Collections;
-using System.Collections.Generic;
-
-namespace Avalonia.Utilities
-{
-    /// <summary>
-    /// Implements a simple list which is safe to modify during enumeration.
-    /// </summary>
-    /// <typeparam name="T">The item type.</typeparam>
-    /// <remarks>
-    /// Implements a list which, when written to while enumerating, performs a copy of the list
-    /// items. Note this this class doesn't actually implement <see cref="IList{T}"/> as it's not
-    /// currently needed - feel free to add missing methods etc.
-    /// </remarks>
-    internal class SafeEnumerableList<T> : IEnumerable<T>
-    {
-        private List<T> _list = new();
-        private int _generation;
-        private int _enumCount = 0;
-
-        public int Count => _list.Count;
-        internal List<T> Inner => _list;
-
-        public void Add(T item) => GetList().Add(item);
-        public bool Remove(T item) => GetList().Remove(item);
-
-        public Enumerator GetEnumerator() => new(this, _list);
-        IEnumerator<T> IEnumerable<T>.GetEnumerator() => GetEnumerator();
-        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
-        
-        private List<T> GetList()
-        {
-            if (_enumCount > 0)
-            {
-                _list = new(_list);
-                ++_generation;
-                _enumCount = 0;
-            }
-
-            return _list;
-        }
-
-        public struct Enumerator : IEnumerator<T>, IEnumerator
-        {
-            private readonly SafeEnumerableList<T> _owner;
-            private readonly List<T> _list;
-            private readonly int _generation;
-            private int _index;
-            private T? _current;
-
-            internal Enumerator(SafeEnumerableList<T> owner, List<T> list)
-            {
-                _owner = owner;
-                _list = list;
-                _generation = owner._generation;
-                _index = 0;
-                _current = default;
-                ++_owner._enumCount;
-            }
-
-            public void Dispose()
-            {
-                if (_owner._generation == _generation)
-                    --_owner._enumCount;
-            }
-
-            public bool MoveNext()
-            {
-                if (_index < _list.Count)
-                {
-                    _current = _list[_index++];
-                    return true;
-                }
-
-                _current = default;
-                return false;
-            }
-
-            public T Current => _current!;
-            object? IEnumerator.Current => _current;
-
-            void IEnumerator.Reset()
-            {
-                _index = 0;
-                _current = default;
-            }
-        }
-    }
-}

tests/Avalonia.Controls.UnitTests/Utils/SafeEnumerableListTests.cs → tests/Avalonia.Controls.UnitTests/Utils/SafeEnumerableHashSetTests.cs

@@ -4,12 +4,12 @@ using Xunit;
 
 namespace Avalonia.Controls.UnitTests.Utils
 {
-    public class SafeEnumerableListTests
+    public class SafeEnumerableHashSetTests
     {
         [Fact]
-        public void List_Is_Not_Copied_Outside_Enumeration()
+        public void Set_Is_Not_Copied_Outside_Enumeration()
         {
-            var target = new SafeEnumerableList<string>();
+            var target = new SafeEnumerableHashSet<string>();
             var inner = target.Inner;
 
             target.Add("foo");
@@ -20,9 +20,9 @@ namespace Avalonia.Controls.UnitTests.Utils
         }
 
         [Fact]
-        public void List_Is_Copied_Outside_Enumeration()
+        public void Set_Is_Copied_Outside_Enumeration()
         {
-            var target = new SafeEnumerableList<string>();
+            var target = new SafeEnumerableHashSet<string>();
             var inner = target.Inner;
 
             target.Add("foo");
@@ -43,13 +43,13 @@ namespace Avalonia.Controls.UnitTests.Utils
                 Assert.NotSame(inner, target.Inner);
             }
 
-            Assert.Equal(new[] { "foo", "bar", "baz", "baz" }, target);
+            Assert.Equal(new HashSet<string> { "foo", "bar", "baz", "baz" }, target);
         }
 
         [Fact]
-        public void List_Is_Not_Copied_After_Enumeration()
+        public void Set_Is_Not_Copied_After_Enumeration()
         {
-            var target = new SafeEnumerableList<string>();
+            var target = new SafeEnumerableHashSet<string>();
             var inner = target.Inner;
 
             target.Add("foo");
@@ -67,9 +67,9 @@ namespace Avalonia.Controls.UnitTests.Utils
         }
 
         [Fact]
-        public void List_Is_Copied_Only_Once_During_Enumeration()
+        public void Set_Is_Copied_Only_Once_During_Enumeration()
         {
-            var target = new SafeEnumerableList<string>();
+            var target = new SafeEnumerableHashSet<string>();
             var inner = target.Inner;
 
             target.Add("foo");
@@ -87,14 +87,14 @@ namespace Avalonia.Controls.UnitTests.Utils
         }
 
         [Fact]
-        public void List_Is_Copied_During_Nested_Enumerations()
+        public void Set_Is_Copied_During_Nested_Enumerations()
         {
-            var target = new SafeEnumerableList<string>();
+            var target = new SafeEnumerableHashSet<string>();
             var initialInner = target.Inner;
-            var firstItems = new List<string>();
+            var firstItems = new HashSet<string>();
-            var secondItems = new List<string>();
+            var secondItems = new HashSet<string>();
-            List<string> firstInner;
+            HashSet<string> firstInner;
-            List<string> secondInner;
+            HashSet<string> secondInner;
 
             target.Add("foo");
 
@@ -118,9 +118,9 @@ namespace Avalonia.Controls.UnitTests.Utils
                 firstItems.Add(i);
             }
 
-            Assert.Equal(new[] { "foo" }, firstItems);
+            Assert.Equal(new HashSet<string> { "foo" }, firstItems);
-            Assert.Equal(new[] { "foo", "bar" }, secondItems);
+            Assert.Equal(new HashSet<string> { "foo", "bar" }, secondItems);
-            Assert.Equal(new[] { "foo", "bar", "baz", "baz" }, target);
+            Assert.Equal(new HashSet<string> { "foo", "bar", "baz", "baz" }, target);
 
             var finalInner = target.Inner;
             target.Add("final");