Browse Source

Another fix for WeakHashList. Added basic unit tests.

Dariusz Komosinski 3 years ago
parent
commit
066c81b1ac

+ 8 - 5
src/Avalonia.Base/Utilities/WeakHashList.cs

@@ -6,6 +6,8 @@ namespace Avalonia.Utilities;
 
 internal class WeakHashList<T> where T : class
 {
+    public const int DefaultArraySize = 8;
+    
     private struct Key
     {
         public WeakReference<T>? Weak;
@@ -63,7 +65,8 @@ internal class WeakHashList<T> where T : class
     WeakReference<T>?[]? _arr;
     int _arrCount;
     
-    public bool IsEmpty => _dic == null || _dic.Count == 0;
+    public bool IsEmpty => _dic is not null ? _dic.Count == 0 : _arrCount == 0;
+
     public bool NeedCompact { get; private set; }
     
     public void Add(T item)
@@ -79,7 +82,7 @@ internal class WeakHashList<T> where T : class
         }
 
         if (_arr == null)
-            _arr = new WeakReference<T>[8];
+            _arr = new WeakReference<T>[DefaultArraySize];
 
         if (_arrCount < _arr.Length)
         {
@@ -108,6 +111,7 @@ internal class WeakHashList<T> where T : class
         Add(item);
 
         _arr = null;
+        _arrCount = 0;
     }
 
     public void Remove(T item)
@@ -119,7 +123,7 @@ internal class WeakHashList<T> where T : class
                 if (_arr[c]?.TryGetTarget(out var target) == true && target == item)
                 {
                     _arr[c] = null;
-                    Compact();
+                    ArrCompact();
                     return;
                 }
             }
@@ -219,7 +223,6 @@ internal class WeakHashList<T> where T : class
         }
         if (_dic != null)
         {
-            
             foreach (var kvp in _dic)
             {
                 if (kvp.Key.Weak?.TryGetTarget(out var target) == true)
@@ -235,4 +238,4 @@ internal class WeakHashList<T> where T : class
 
         return pooled;
     }
-}
+}

+ 66 - 0
tests/Avalonia.Base.UnitTests/Utilities/WeakHashListTests.cs

@@ -0,0 +1,66 @@
+using Avalonia.Utilities;
+using Xunit;
+
+namespace Avalonia.Base.UnitTests.Utilities;
+
+public class WeakHashListTests
+{
+    [Fact]
+    public void Is_Empty_Works()
+    {
+        var target = new WeakHashList<string>();
+        
+        Assert.True(target.IsEmpty);
+        
+        target.Add("1");
+        
+        Assert.False(target.IsEmpty);
+        
+        target.Remove("1");
+        
+        Assert.True(target.IsEmpty);
+
+        // Fill array storage.
+        var arrMaxSize = WeakHashList<string>.DefaultArraySize;
+
+        for (int i = 0; i < arrMaxSize; i++)
+        {
+            target.Add(i.ToString());
+        }
+        
+        Assert.False(target.IsEmpty);
+        
+        // This goes above array storage and upgrades to a dictionary.
+        target.Add(arrMaxSize.ToString());
+        
+        Assert.False(target.IsEmpty);
+
+        // Remove everything, this should still keep an empty dictionary.
+        for (int i = 0; i < arrMaxSize + 1; i++)
+        {
+            target.Remove(i.ToString());
+        }
+        
+        Assert.True(target.IsEmpty);
+    }
+
+    [Fact]
+    public void Array_Compact_After_Remove_Works()
+    {
+        var target = new WeakHashList<string>();
+        
+        // Use all slots in array storage.
+        var arrMaxSize = WeakHashList<string>.DefaultArraySize;
+
+        for (int i = 0; i < arrMaxSize; i++)
+        {
+            target.Add(i.ToString());
+        }
+        
+        // This should compact the array.
+        target.Remove("3");
+        
+        // And new value should fill empty space.
+        target.Add("42");
+    }
+}