[UWP] Reduce allocations and fix memory leak in CollectionView (#7608)

* Checkpoint

* Fix leak caused by CollectionViewSource

* Simplify ItemTemplateEnumerator

* Reduce allocations in simple IList source scenarios on UWP

* Revert debugging changes

diff --git a/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContext.cs b/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContext.cs
index 3f4b6fa..255e64b 100644 (file)
--- a/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContext.cs
+++ b/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContext.cs
@@ -2,7 +2,8 @@
 {
        internal class ItemTemplateContext
        {
-               public ItemTemplateContext(DataTemplate formsDataTemplate, object item, BindableObject container, double? height = null, double? width = null, Thickness? itemSpacing = null)
+               public ItemTemplateContext(DataTemplate formsDataTemplate, object item, BindableObject container, 
+                       double? height = null, double? width = null, Thickness? itemSpacing = null)
                {
                        FormsDataTemplate = formsDataTemplate;
                        Item = item;

diff --git a/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContextEnumerable.cs b/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContextEnumerable.cs
new file mode 100644 (file)
index 0000000..7012cac
--- /dev/null
+++ b/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContextEnumerable.cs
@@ -0,0 +1,39 @@
+using System.Collections;
+
+namespace Xamarin.Forms.Platform.UWP
+{
+       internal class ItemTemplateContextEnumerable : IEnumerable
+       {
+               readonly IEnumerable _itemsSource;
+               readonly DataTemplate _formsDataTemplate;
+               readonly BindableObject _container;
+               readonly double _itemHeight;
+               readonly double _itemWidth;
+               readonly Thickness _itemSpacing;
+
+               public ItemTemplateContextEnumerable(IEnumerable itemsSource, DataTemplate formsDataTemplate, BindableObject container, 
+                       double? itemHeight = null, double? itemWidth = null, Thickness? itemSpacing = null)
+               {
+                       _itemsSource = itemsSource;
+                       _formsDataTemplate = formsDataTemplate;
+                       _container = container;
+
+                       if (itemHeight.HasValue)
+                               _itemHeight = itemHeight.Value;
+
+                       if (itemWidth.HasValue)
+                               _itemWidth = itemWidth.Value;
+
+                       if (itemSpacing.HasValue)
+                               _itemSpacing = itemSpacing.Value;
+               }
+
+               public IEnumerator GetEnumerator()
+               {
+                       foreach (var item in _itemsSource)
+                       {
+                               yield return new ItemTemplateContext(_formsDataTemplate, item, _container, _itemHeight, _itemWidth, _itemSpacing);
+                       }
+               }
+       }
+}
\ No newline at end of file

diff --git a/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContextList.cs b/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContextList.cs
new file mode 100644 (file)
index 0000000..760b9c8
--- /dev/null
+++ b/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateContextList.cs
@@ -0,0 +1,101 @@
+using System.Collections;
+using System.Collections.Generic;
+
+namespace Xamarin.Forms.Platform.UWP
+{
+       internal class ItemTemplateContextList : IReadOnlyList<ItemTemplateContext>
+       {
+               readonly IList _itemsSource;
+               readonly DataTemplate _itemTemplate;
+               readonly BindableObject _container;
+               readonly double _itemHeight;
+               readonly double _itemWidth;
+               readonly Thickness _itemSpacing;
+
+               readonly List<ItemTemplateContext> _itemTemplateContexts;
+
+               public int Count => _itemsSource.Count;
+
+               public ItemTemplateContext this[int index]
+               {
+                       get
+                       {
+                               if (_itemTemplateContexts[index] == null)
+                               {
+                                       _itemTemplateContexts[index] = new ItemTemplateContext(_itemTemplate, _itemsSource[index], 
+                                               _container, _itemHeight, _itemWidth, _itemSpacing);
+                               }
+
+                               return _itemTemplateContexts[index];
+                       }
+               }
+
+               public ItemTemplateContextList(IList itemsSource, DataTemplate itemTemplate, BindableObject container,
+                       double? itemHeight = null, double? itemWidth = null, Thickness? itemSpacing = null)
+               {
+                       _itemsSource = itemsSource;
+                       _itemTemplate = itemTemplate;
+                       _container = container;
+
+                       if (itemHeight.HasValue)
+                               _itemHeight = itemHeight.Value;
+
+                       if (itemWidth.HasValue)
+                               _itemWidth = itemWidth.Value;
+
+                       if (itemSpacing.HasValue)
+                               _itemSpacing = itemSpacing.Value;
+
+                       _itemTemplateContexts = new List<ItemTemplateContext>(_itemsSource.Count);
+
+                       for (int n = 0; n < _itemsSource.Count; n++)
+                       {
+                               _itemTemplateContexts.Add(null);
+                       }
+               }
+
+               public IEnumerator<ItemTemplateContext> GetEnumerator()
+               {
+                       return new ItemTemplateContextListEnumerator(this);
+               }
+
+               IEnumerator IEnumerable.GetEnumerator()
+               {
+                       return GetEnumerator();
+               }
+
+               internal class ItemTemplateContextListEnumerator : IEnumerator<ItemTemplateContext>
+               {
+                       public ItemTemplateContext Current { get; private set; }
+                       object IEnumerator.Current { get; }
+                       int _currentIndex = -1;
+                       private ItemTemplateContextList _itemTemplateContextList;
+
+                       public ItemTemplateContextListEnumerator(ItemTemplateContextList observableItemTemplateCollection) => 
+                               _itemTemplateContextList = observableItemTemplateCollection;
+
+                       public void Dispose()
+                       {
+                       }
+
+                       public bool MoveNext()
+                       {
+                               if (_currentIndex >= _itemTemplateContextList.Count - 1)
+                               {
+                                       return false;
+                               }
+
+                               _currentIndex += 1;
+                               Current = _itemTemplateContextList[_currentIndex];
+
+                               return true;
+                       }
+
+                       public void Reset()
+                       {
+                               Current = null;
+                               _currentIndex = -1;
+                       }
+               }
+       }
+}
\ No newline at end of file

diff --git a/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateEnumerator.cs b/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateEnumerator.cs
deleted file mode 100644 (file)
index 62cced9..0000000
--- a/Xamarin.Forms.Platform.UAP/CollectionView/ItemTemplateEnumerator.cs
+++ /dev/null
@@ -1,54 +0,0 @@
-using System.Collections;
-
-namespace Xamarin.Forms.Platform.UWP
-{
-       internal class ItemTemplateEnumerator : IEnumerable, IEnumerator
-       {
-               readonly DataTemplate _formsDataTemplate;
-               readonly IEnumerator _innerEnumerator;
-               readonly BindableObject _container;
-               readonly double _itemHeight;
-               readonly double _itemWidth;
-               readonly Thickness _itemSpacing;
-
-               public ItemTemplateEnumerator(IEnumerable itemsSource, DataTemplate formsDataTemplate, BindableObject container, double? itemHeight = null, double? itemWidth = null, Thickness? itemSpacing = null)
-               {
-                       _formsDataTemplate = formsDataTemplate;
-                       _container = container;
-                       _innerEnumerator = itemsSource.GetEnumerator();
-
-                       if (itemHeight.HasValue)
-                               _itemHeight = itemHeight.Value;
-
-                       if (itemWidth.HasValue)
-                               _itemWidth = itemWidth.Value;
-
-                       if (itemSpacing.HasValue)
-                               _itemSpacing = itemSpacing.Value;
-               }
-
-               public IEnumerator GetEnumerator()
-               {
-                       return this;
-               }
-
-               public bool MoveNext()
-               {
-                       var moveNext = _innerEnumerator.MoveNext();
-                       
-                       if (moveNext)
-                       {
-                               Current = new ItemTemplateContext(_formsDataTemplate, _innerEnumerator.Current, _container, _itemHeight, _itemWidth, _itemSpacing);
-                       }
-
-                       return moveNext;
-               }
-
-               public void Reset()
-               {
-                       _innerEnumerator.Reset();
-               }
-
-               public object Current { get; private set; }
-       }
-}
\ No newline at end of file

diff --git a/Xamarin.Forms.Platform.UAP/CollectionView/ItemsViewRenderer.cs b/Xamarin.Forms.Platform.UAP/CollectionView/ItemsViewRenderer.cs
index f1333b0..ece3e2e 100644 (file)
--- a/Xamarin.Forms.Platform.UAP/CollectionView/ItemsViewRenderer.cs
+++ b/Xamarin.Forms.Platform.UAP/CollectionView/ItemsViewRenderer.cs
@@ -89,7 +89,13 @@ namespace Xamarin.Forms.Platform.UWP
                                        incc.CollectionChanged -= ItemsChanged;
                                }
 
+                               if (_collectionViewSource != null)
+                               {
+                                       _collectionViewSource.Source = null;
+                               }
+
                                _collectionViewSource = null;
+                               ListViewBase.ItemsSource = null;
                                return;
                        }
 
@@ -124,7 +130,7 @@ namespace Xamarin.Forms.Platform.UWP
                                        IsSourceGrouped = false
                                };
                        }
-                       
+
                        ListViewBase.ItemsSource = _collectionViewSource.View;
 
                        UpdateEmptyViewVisibility();
@@ -197,6 +203,17 @@ namespace Xamarin.Forms.Platform.UWP
 
                        // Stop listening for ScrollTo requests
                        oldElement.ScrollToRequested -= ScrollToRequested;
+
+                       if (ListViewBase != null)
+                       {
+                               ListViewBase.ItemsSource = null;
+                       }
+
+                       if (_collectionViewSource != null)
+                       {
+                               _collectionViewSource.Source = null;
+                       }
+
                }
 
                void UpdateVerticalScrollBarVisibility()

diff --git a/Xamarin.Forms.Platform.UAP/CollectionView/ObservableItemTemplateCollection.cs b/Xamarin.Forms.Platform.UAP/CollectionView/ObservableItemTemplateCollection.cs
index 006b3ab..0a5af80 100644 (file)
--- a/Xamarin.Forms.Platform.UAP/CollectionView/ObservableItemTemplateCollection.cs
+++ b/Xamarin.Forms.Platform.UAP/CollectionView/ObservableItemTemplateCollection.cs
@@ -15,7 +15,8 @@ namespace Xamarin.Forms.Platform.UWP
                readonly Thickness _itemSpacing;
                readonly INotifyCollectionChanged _notifyCollectionChanged;
 
-               public ObservableItemTemplateCollection(IList itemsSource, DataTemplate itemTemplate, BindableObject container, double? itemHeight = null, double? itemWidth = null, Thickness? itemSpacing = null)
+               public ObservableItemTemplateCollection(IList itemsSource, DataTemplate itemTemplate, BindableObject container, 
+                       double? itemHeight = null, double? itemWidth = null, Thickness? itemSpacing = null)
                {
                        if (!(itemsSource is INotifyCollectionChanged notifyCollectionChanged))
                        {
@@ -39,6 +40,9 @@ namespace Xamarin.Forms.Platform.UWP
 
                        for (int n = 0; n < itemsSource.Count; n++)
                        {
+                               // We're using this as a source for a ListViewBase, and we need INCC to work. So ListViewBase is going
+                               // to iterate over the entire source list right off the bat, no matter what we do. Creating one
+                               // ItemTemplateContext per item in the collection is unavoidable. Luckily, ITC is pretty cheap.
                                Add(new ItemTemplateContext(itemTemplate, itemsSource[n], container, _itemHeight, _itemWidth, _itemSpacing));
                        }
 

diff --git a/Xamarin.Forms.Platform.UAP/CollectionView/TemplatedItemSourceFactory.cs b/Xamarin.Forms.Platform.UAP/CollectionView/TemplatedItemSourceFactory.cs
index f5535bd..9009fc1 100644 (file)
--- a/Xamarin.Forms.Platform.UAP/CollectionView/TemplatedItemSourceFactory.cs
+++ b/Xamarin.Forms.Platform.UAP/CollectionView/TemplatedItemSourceFactory.cs
@@ -9,11 +9,13 @@ namespace Xamarin.Forms.Platform.UWP
                {
                        switch (itemsSource)
                        {
-                               case IList list when itemsSource is INotifyCollectionChanged:
-                                       return new ObservableItemTemplateCollection(list, itemTemplate, container, itemHeight, itemWidth, itemSpacing);
+                               case IList observable when itemsSource is INotifyCollectionChanged:
+                                       return new ObservableItemTemplateCollection(observable, itemTemplate, container, itemHeight, itemWidth, itemSpacing);
+                               case IList list:
+                                       return new ItemTemplateContextList(list, itemTemplate, container, itemHeight, itemWidth, itemSpacing);
                        }
 
-                       return new ItemTemplateEnumerator(itemsSource, itemTemplate, container, itemHeight, itemWidth, itemSpacing);
+                       return new ItemTemplateContextEnumerable(itemsSource, itemTemplate, container, itemHeight, itemWidth, itemSpacing);
                }
        }
 }
\ No newline at end of file

diff --git a/Xamarin.Forms.Platform.UAP/Xamarin.Forms.Platform.UAP.csproj b/Xamarin.Forms.Platform.UAP/Xamarin.Forms.Platform.UAP.csproj
index 72dfcdc..6074412 100644 (file)
--- a/Xamarin.Forms.Platform.UAP/Xamarin.Forms.Platform.UAP.csproj
+++ b/Xamarin.Forms.Platform.UAP/Xamarin.Forms.Platform.UAP.csproj
@@ -44,6 +44,7 @@
     <Compile Include="CollectionView\FormsListView.cs" />
     <Compile Include="CollectionView\IEmptyView.cs" />
     <Compile Include="CollectionView\ItemsViewRenderer.cs" />
+    <Compile Include="CollectionView\ItemTemplateContextList.cs" />
     <Compile Include="CollectionView\ScrollHelpers.cs" />
     <Compile Include="CollectionView\SelectableItemsViewRenderer.cs" />
     <Compile Include="CollectionView\StructuredItemsViewRenderer.cs" />
@@ -77,7 +78,7 @@
     <Compile Include="ITitleIconProvider.cs" />
     <Compile Include="ITitleViewProvider.cs" />
     <Compile Include="CollectionView\FormsGridView.cs" />
-    <Compile Include="CollectionView\ItemTemplateEnumerator.cs" />
+    <Compile Include="CollectionView\ItemTemplateContextEnumerable.cs" />
     <Compile Include="CollectionView\ItemTemplateContext.cs" />
     <Compile Include="ITitleProvider.cs" />
     <Compile Include="ITitleViewRendererController.cs" />