Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Changes disposing items in CollectionView #10590

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public CollectionViewGallery()
GalleryBuilder.NavButton("Alternate Layout Galleries", () => new AlternateLayoutGallery(), Navigation),
GalleryBuilder.NavButton("Header/Footer Galleries", () => new HeaderFooterGallery(), Navigation),
GalleryBuilder.NavButton("Nested CollectionViews", () => new NestedGalleries.NestedCollectionViewGallery(), Navigation),
GalleryBuilder.NavButton("CollectionView Memory Benchmark", () => new CollectionViewMemoryBenchmark(), Navigation),
}
}
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage
xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Maui.Controls.Sample.Pages.CollectionViewGalleries.CollectionViewMemoryBenchmark"
xmlns:local="clr-namespace:Maui.Controls.Sample.Pages.CollectionViewGalleries"
Title="CollectionView Memory Benchmark">
<ContentPage.Resources>
<ResourceDictionary>

<Color x:Key="Gray300">#ACACAC</Color>
<SolidColorBrush x:Key="Gray300Brush" Color="{StaticResource Gray300}"/>

</ResourceDictionary>
</ContentPage.Resources>
<ContentPage.Content>
<Grid
ColumnDefinitions="*,*,*"
ColumnSpacing="5"
RowDefinitions="40,60,20,20,*"
RowSpacing="5">

<Button
Grid.Column="0"
Clicked="Button_Clicked_1"
Text="Test 1" />
<Button
Grid.Column="1"
Clicked="Button_Clicked_2"
Text="Test 2" />
<Button
Grid.Column="2"
Clicked="Button_Clicked_3"
Text="Test 3" />

<Label
Grid.Row="1"
Grid.Column="0"
FontSize="Caption"
HorizontalTextAlignment="Center"
Text="BackgroundColor=&quot;{StaticResource Gray300}&quot;" />

<Label
Grid.Row="1"
Grid.Column="1"
FontSize="Caption"
HorizontalTextAlignment="Center"
Text="Background=&quot;{StaticResource Gray300}&quot;" />

<Label
Grid.Row="1"
Grid.Column="2"
FontSize="Caption"
HorizontalTextAlignment="Center"
Text="Background=&quot;{StaticResource Gray300Brush}&quot;" />

<Label
x:Name="LabelMemory"
Grid.Row="2"
Grid.ColumnSpan="3"
HorizontalTextAlignment="Center"
Text="Memory" />

<Label
x:Name="LabelAlive"
Grid.Row="3"
Grid.ColumnSpan="3"
HorizontalTextAlignment="Center"
Text="Objects alive" />

<CollectionView
x:Name="CollectionView1"
Grid.Row="4"
Grid.Column="0">
<CollectionView.ItemTemplate>
<DataTemplate x:DataType="local:DataModel">
<VerticalStackLayout
local:References.IsWatched="True"
BackgroundColor="{StaticResource Gray300}">
<Button Text="{Binding ID}" />
<Label
Margin="10"
Text="{Binding Name}" />
<Label
Margin="10"
Text="{Binding Data1}" />
</VerticalStackLayout>
</DataTemplate>
</CollectionView.ItemTemplate>
</CollectionView>

<CollectionView
x:Name="CollectionView2"
Grid.Row="4"
Grid.Column="1">
<CollectionView.ItemTemplate>
<DataTemplate x:DataType="local:DataModel">
<VerticalStackLayout
local:References.IsWatched="True"
Background="{StaticResource Gray300}">
<Button Text="{Binding ID}" />
<Label
Margin="10"
Text="{Binding Name}" />
<Label
Margin="10"
Text="{Binding Data1}" />
</VerticalStackLayout>
</DataTemplate>
</CollectionView.ItemTemplate>
</CollectionView>

<CollectionView
x:Name="CollectionView3"
Grid.Row="4"
Grid.Column="2">
<CollectionView.ItemTemplate>
<DataTemplate x:DataType="local:DataModel">
<VerticalStackLayout
local:References.IsWatched="True"
Background="{StaticResource Gray300Brush}">
<Button Text="{Binding ID}" />
<Label
Margin="10"
Text="{Binding Name}" />
<Label
Margin="10"
Text="{Binding Data1}" />
</VerticalStackLayout>
</DataTemplate>
</CollectionView.ItemTemplate>
</CollectionView>

</Grid>
</ContentPage.Content>
</ContentPage>
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
using System;
using System.Diagnostics;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.Maui.Controls;

namespace Maui.Controls.Sample.Pages.CollectionViewGalleries
{
public partial class CollectionViewMemoryBenchmark : ContentPage
{
int _id;

public CollectionViewMemoryBenchmark()
{
InitializeComponent();
}

async void Button_Clicked_1(object sender, EventArgs e)
{
CollectionView1.ItemsSource = GetNewData();
await UpdateMemoryAsync(1);
}

async void Button_Clicked_2(object sender, EventArgs e)
{
CollectionView2.ItemsSource = GetNewData();
await UpdateMemoryAsync(2);
}

async void Button_Clicked_3(object sender, EventArgs e)
{
CollectionView3.ItemsSource = GetNewData();
await UpdateMemoryAsync(3);
}

List<DataModel> GetNewData()
{
List<DataModel> data = new();

for (int i = 0; i < 5; i++)
{
_id += 1;
data.Add(new DataModel() { ID = _id, Name = $"Name{_id}", Data1 = "data1", Data2 = "data2", Data3 = "data3" });
}

return data;
}

async Task UpdateMemoryAsync(int testid)
{
await Task.Delay(500);

GC.Collect();
GC.WaitForPendingFinalizers();

LabelMemory.Text = $"Memory {GC.GetTotalMemory(true):N0} after test {testid}";
LabelAlive.Text = $"Objects alive {References.GetAliveCount()}";

References.Print();
}
}

public class DataModel
{
public int ID { get; set; }
public string Name { get; set; }
public string Data1 { get; set; }
public string Data2 { get; set; }
public string Data3 { get; set; }
}

public static class References
{
static readonly List<WeakReference> Refs = new();

public static readonly BindableProperty IsWatchedProperty =
BindableProperty.CreateAttached("IsWatched", typeof(bool), typeof(References), false, propertyChanged: OnIsWatchedChanged);

public static bool GetIsWatched(BindableObject obj)
{
return (bool)obj.GetValue(IsWatchedProperty);
}

public static void SetIsWatched(BindableObject obj, bool value)
{
obj.SetValue(IsWatchedProperty, value);
}

static void OnIsWatchedChanged(BindableObject bindable, object oldValue, object newValue)
{
AddRef(bindable);
}

public static void AddRef(object p)
{
Refs.Add(new WeakReference(p));
}

public static void Print()
{
GC.Collect();
GC.WaitForPendingFinalizers();

Refs.RemoveAll(a => !a.IsAlive);
foreach (WeakReference weakReference in Refs)
Debug.WriteLine("IsAlive: " + weakReference.Target?.GetType().Name);

Debug.WriteLine($"Total Refs still alive: {Refs.Count}");
Debug.WriteLine("---------------");
}

public static int GetAliveCount()
{
return Refs.Count(weakReference => weakReference.IsAlive);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using Android.Content;
using Android.Widget;
using AndroidX.RecyclerView.Widget;
Expand All @@ -13,6 +14,7 @@ public class ItemsViewAdapter<TItemsView, TItemsViewSource> : RecyclerView.Adapt
where TItemsViewSource : IItemsViewSource
{
protected readonly TItemsView ItemsView;
readonly List<ItemContentView> ContentViews;
readonly Func<View, Context, ItemContentView> _createItemContentView;
protected internal TItemsViewSource ItemsSource;

Expand All @@ -22,6 +24,7 @@ public class ItemsViewAdapter<TItemsView, TItemsViewSource> : RecyclerView.Adapt
protected internal ItemsViewAdapter(TItemsView itemsView, Func<View, Context, ItemContentView> createItemContentView = null)
{
ItemsView = itemsView ?? throw new ArgumentNullException(nameof(itemsView));
ContentViews = new List<ItemContentView>();

UpdateUsingItemTemplate();

Expand Down Expand Up @@ -85,6 +88,7 @@ public override RecyclerView.ViewHolder OnCreateViewHolder(ViewGroup parent, int
}

var itemContentView = _createItemContentView.Invoke(ItemsView, context);
ContentViews.Add(itemContentView);

return new TemplatedItemViewHolder(itemContentView, ItemsView.ItemTemplate, IsSelectionEnabled(parent, viewType));
}
Expand All @@ -108,6 +112,12 @@ protected override void Dispose(bool disposing)
{
if (disposing)
{
foreach (var contentView in ContentViews)
{
contentView.Recycle();
}
ContentViews.Clear();

ItemsSource?.Dispose();
ItemsView.PropertyChanged -= ItemsViewPropertyChanged;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ protected virtual void CleanUpCollectionViewSource()
CollectionViewSource = null;
}

var itemContentControls = ListViewBase.GetChildren<ItemContentControl>();

foreach (ItemContentControl itemContentControl in itemContentControls)
{
itemContentControl.Dispose();
}

if (VirtualView?.ItemsSource == null)
{
ListViewBase.ItemsSource = null;
Expand Down
26 changes: 26 additions & 0 deletions src/Controls/src/Core/Items/ItemsView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,32 @@ public void RemoveLogicalChild(Element element)
_logicalChildren.Remove(element);
OnChildRemoved(element, oldLogicalIndex);
VisualDiagnostics.OnChildRemoved(this, element, oldLogicalIndex);
DisconnectLogicalChild(element);
}


void DisconnectLogicalChild(Element element)
{
#if ANDROID || IOS || MACCATALYST || WINDOWS
if (element.Handler is IPlatformViewHandler platformHandler)
{
foreach (Element child in element.Descendants())
{
if (child is VisualElement ve)
{
ve.Handler?.DisconnectHandler();

if (ve.Handler is IDisposable veHandlerDisposable)
veHandlerDisposable.Dispose();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you remove all the dispose code does that still work?

I don't think we want to start pulling dispose patterns in the code here.

}
}

platformHandler.DisconnectHandler();

if (platformHandler is IDisposable platformHandlerDisposable)
platformHandlerDisposable.Dispose();
}
#endif
}

internal override IReadOnlyList<Element> LogicalChildrenInternal => _logicalChildren.AsReadOnly();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace Microsoft.Maui.Controls.Platform
{
public class ItemContentControl : ContentControl
public class ItemContentControl : ContentControl, IDisposable
{
VisualElement _visualElement;
IViewHandler _renderer;
Expand Down Expand Up @@ -143,6 +143,24 @@ protected override void OnContentChanged(object oldContent, object newContent)
}
}

public void Dispose()
{
if (FormsContainer is ItemsView itemsView && _renderer?.VirtualView is Element element)
{
itemsView.RemoveLogicalChild(element);
element = null;
}

FormsDataContext = null;
FormsDataTemplate = null;
FormsContainer = null;
DataContext = null;
MauiContext = null;

_renderer.DisconnectHandler();
_renderer = null;
}

internal void Realize()
{
var dataContext = FormsDataContext;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Microsoft.Maui.Controls.MenuFlyout.MenuFlyout() -> void
Microsoft.Maui.Controls.MenuFlyout.Count.get -> int
Microsoft.Maui.Controls.MenuFlyout.IsReadOnly.get -> bool
Microsoft.Maui.Controls.MenuFlyout.RemoveAt(int index) -> void
Microsoft.Maui.Controls.Platform.ItemContentControl.Dispose() -> void
Microsoft.Maui.Controls.Platform.ShellNavigationViewItem
Microsoft.Maui.Controls.Platform.ShellNavigationViewItem.ShellNavigationViewItem() -> void
Microsoft.Maui.Controls.Platform.ShellNavigationViewItemAutomationPeer
Expand Down
Loading