Hunting Xamarin.Forms Shadows leaks on Android
So I was testing the brand new BitmapCache for Android I created for Sharpnado.Shadows. I was getting sure that the reference count for each shadow bitmaps was correctly decreased and/or removed (when count reaches 0).
I was happy with the result after stacking different MainPage of my Shadows sample app.
I could see the Bitmap reference count decreasing accordingly: if I had a bitmap shadow shared by 4 views in the page, the reference to this cached Bitmap increased by 4 when stacking the page, and then was back to where it was when I popped the page.
Until I decided to add the performance test...
Obviously the BitmapCache really shines for list view on Android.
The CollectionView is implemented with a RecyclerView on Android.
The recycler view will create let say 5 item views and recycle them to cover the whole screen.
So if each item has one Shadow, with the BitmapCache just one Bitmap will be created and shared by all the views.
Now if you are doing neumorphism, which is a stack of 2 shadows, you will have 2 Bitmap instead of 10.
You get the idea...
I created a ShadowList.xaml to test the performance of the BitmapCache.
<?xml version="1.0" encoding="utf-8" ?>
....
<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="200" />
<RowDefinition Height="*" />
</Grid.RowDefinitions>
<views:Logo Grid.Row="0"
Margin="0,0,0,60"
HorizontalOptions="Center"
VerticalOptions="End" />
<CollectionView Grid.Row="1" ItemSizingStrategy="MeasureFirstItem">
<CollectionView.Header>
<sh:Shadows Margin="20,20"
CornerRadius="10"
Shades="{sh:NeumorphismShades}"
StyleId="ButtonShadows">
<Button Style="{StaticResource TextHeadline}"
HorizontalOptions="Center"
VerticalOptions="Center"
BackgroundColor="{DynamicResource DynamicFrameBackgroundColor}"
Clicked="OnNavigateToMainPageClicked"
CornerRadius="10"
Text="Navigate to Main page"
TextColor="Gray" />
</sh:Shadows>
</CollectionView.Header>
...
<CollectionView.ItemTemplate>
<DataTemplate>
<ContentView>
<sh:Shadows Margin="20,10" StyleId="Cell">
<Frame IsClippedToBounds="False">
<Frame.BorderColor>
<OnPlatform x:TypeArguments="Color">
<OnPlatform.Platforms>
<On Platform="Tizen" Value="Transparent" />
</OnPlatform.Platforms>
</OnPlatform>
</Frame.BorderColor>
<Grid RowSpacing="10">
<Grid.RowDefinitions>
<RowDefinition Height="80" />
<RowDefinition Height="70" />
</Grid.RowDefinitions>
<Grid.ColumnDefinitions>
<ColumnDefinition Width="*" />
<ColumnDefinition Width="*" />
</Grid.ColumnDefinitions>
<Label Grid.Row="0"
Grid.Column="0"
Grid.ColumnSpan="2"
Style="{StaticResource TextBodySecondary}"
Margin="0,0,0,20"
Text="Since you can apply many shadows to any view, implementing #neumorphism is a walking piece of cake in the park!" />
<sh:Shadows x:Name="ButtonPlusNeuShadows"
Grid.Row="1"
Grid.Column="0"
CornerRadius="40"
Shades="{sh:NeumorphismShades}"
StyleId="CellImageButton">
<ImageButton WidthRequest="60"
HeightRequest="60"
Padding="20"
HorizontalOptions="Center"
VerticalOptions="Start"
BackgroundColor="{DynamicResource DynamicFrameBackgroundColor}"
CornerRadius="30"
Source="{StaticResource IconPlusGray}" />
</sh:Shadows>
<sh:Shadows Grid.Row="1"
Grid.Column="1"
CornerRadius="10"
Shades="{sh:NeumorphismShades}"
StyleId="CellButton">
<Button Style="{StaticResource TextHeadline}"
WidthRequest="120"
HeightRequest="60"
HorizontalOptions="Center"
VerticalOptions="Start"
BackgroundColor="{DynamicResource DynamicFrameBackgroundColor}"
CornerRadius="10"
Text="Reset"
TextColor="Gray" />
</sh:Shadows>
</Grid>
</Frame>
</sh:Shadows>
</ContentView>
</DataTemplate>
</CollectionView.ItemTemplate>
</CollectionView>
</Grid>
Resulting in this view:
I was very happy with the performance, that was a tremendous boost of the drawing time \o/
But something immediately tempered my joy: when I popped the page, the bitmap reference count wasn't decreasing...
Android CollectionView renderer memory leak?
Sharpnado.Shadows have an android ShadowsRenderer which creates one android ShadowView for each Xamarin.Forms Shadows instance.
This renderer override the Dispose method to be sure to release bitmaps (more precisely decrement BitmapCache reference count) when the native view is disposed.
In our case, it seems that this method is not called when we close a page containing a CollectionView.
To be sure of that, let's put debug WriteLine in the renderer's code, where the shadow renderer is disposed.
public class AndroidShadowsRenderer : ViewRenderer<Shadows, FrameLayout>
{
private static int instanceCount;
private ShadowView _shadowView;
private string _tag;
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
InternalLogger.Debug($"Renderer | {_tag}", $"Disposed( disposing: {disposing} )");
if (disposing)
{
if (!_shadowView.IsNullOrDisposed())
{
_shadowView.RemoveFromParent();
_shadowView.Dispose();
instanceCount--;
InternalLogger.Debug($"Renderer | {_tag}", $"now {instanceCount} instances");
}
}
}
...
protected override void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e)
{
base.OnElementPropertyChanged(sender, e);
switch (e.PropertyName)
{
case "Renderer":
var content = GetChildAt(0);
if (content == null)
{
return;
}
_tag = Element.StyleId;
if (_shadowView == null)
{
InternalLogger.Debug($"Renderer | {_tag}", $"Create ShadowView");
_shadowView = new ShadowView(Context, content, Context.ToPixels(Element.CornerRadius));
_shadowView.UpdateShades(Element.Shades);
AddView(_shadowView, 0);
instanceCount++;
InternalLogger.Debug($"Renderer | {_tag}", $"now {instanceCount} instances");
}
break;
}
}
...
}
We also add StyleId tag in our Xamarin.Forms views to trace them (go back to the ShadowList.xaml code and you will find them).
Then we run the code.
We can see that 19 android ShadowView are created on this first page.
We now tap on NAVIGATE TO LIST OF SHADOWS.
Now we can see that 21 more instances have been created for a total of 30 instances.
We can also see that 3 RecyclerView.ItemHolder has been built to recycle the views.
Thanks to the StyleId hack, we see 3 Cell ShadowView being instanced.
Now just pop/close this page and let's see what happens:
AHAHAH! I KNEW IT!
Only the ShadowView that was not in the CollectionView (the one used by the Logo) has been disposed.
The others, the header, and the items, have NOT been disposed.
It means that when the RecyclerView is disposed, it is not disposing it's underlying children.
Xamarin.Forms CollectionView
If we have a peak at the CollectionViewRenderer in the XF source code, we can see that the disposal of the RecyclerView's adapter (the one managing our native item views), is done in the TearDownOldElement method of the ItemsViewsRenderer.
protected override void Dispose(bool disposing)
{
if (_disposed)
{
return;
}
_disposed = true;
if (disposing)
{
_automationPropertiesProvider?.Dispose();
Tracker?.Dispose();
if (Element != null)
{
TearDownOldElement(Element as ItemsView);
if (Platform.GetRenderer(Element) == this)
{
Element.ClearValue(Platform.RendererProperty);
}
}
}
base.Dispose(disposing);
}
protected virtual void TearDownOldElement(ItemsView oldElement)
{
if (oldElement == null)
{
return;
}
// Stop listening for layout property changes
if (ItemsLayout != null)
{
ItemsLayout.PropertyChanged -= LayoutPropertyChanged;
}
// Stop listening for property changes
oldElement.PropertyChanged -= OnElementPropertyChanged;
// Stop listening for ScrollTo requests
oldElement.ScrollToRequested -= ScrollToRequested;
RemoveScrollListener();
if (ItemsViewAdapter != null)
{
// Stop watching for empty items or scroll adjustments
_emptyCollectionObserver.Stop(ItemsViewAdapter);
_itemsUpdateScrollObserver.Stop(ItemsViewAdapter);
// Unhook whichever adapter is active
SetAdapter(null);
_emptyViewAdapter?.Dispose();
ItemsViewAdapter?.Dispose();
}
if (_snapManager != null)
{
_snapManager.Dispose();
_snapManager = null;
}
if (_itemDecoration != null)
{
RemoveItemDecoration(_itemDecoration);
}
}
The code only dispose the Adapter, but it doesn't dispose the native item views that has been created by it.
We could think that there would be a mechanism that does that automatically for us, that the disposal of the RecyclerView automatically triggers the disposal of its children.
It works well for any other android GroupView renderers (like StackLayout or Grid renderers for example).
Unfortunately, from my experience, it doesn't seem to...
So let's fix the Collectio...BUT WAIT!
Is it a plane? Is it a bird? Is it a flying RISC Architecture?
NO IT'S SHANEMAN!
Meanwhile on #xamarin-forms on Discord...
So what Shane is saying, is that unlike other group views (views that have children), the collection view renderer rely only on the Garbage Collector to destroy its children.
So if the children are not being destroyed, it means that another object keeps a link to the children preventing them from being disposed.
Few hours later...
So that is super interesting.
First, as very very often, it is a damn event that prevents the item from being collected by the GC with this scenario:
Shadows is defined with an implicit Style
<sh:Shadows Margin="20,10" StyleId="Cell">
<Frame IsClippedToBounds="False">
<Grid RowSpacing="10">
....
which is defined in Shadows.xaml:
<ResourceDictionary xmlns="http://xamarin.com/schemas/2014/forms" xmlns:sh="clr-namespace:Sharpnado.Shades;assembly=Sharpnado.Shadows">
<Style ApplyToDerivedTypes="True" TargetType="sh:Shadows">
<Setter Property="CornerRadius" Value="10" />
<Setter Property="Shades">
<sh:ImmutableShades>
<sh:Shade BlurRadius="8"
Opacity="1"
Offset="-10,-10"
Color="White" />
<sh:Shade BlurRadius="10"
Opacity="1"
Offset="6, 6"
Color="#19000000" />
</sh:ImmutableShades>
</Setter>
</Style>
</ResourceDictionary>
The way style works, the ImmutableShades is in fact a static instance, so its lifetime is the application lifetime, and it will outlive any of our CollectionView children.
We end up with this link:
static ImmutableShades:Shade.PropertyChanged event => Android ShadowView.ShadePropertyChanged.
So we must:
- make sure each
Shadelifetime is the same as its parent lifetime - create a weak
INotifyPropertyChangedevent
The other great thing about Shane's answer is we now know that if our custom control can pass the CollectionView disposal test, it means that it is GC safe :)
1. Get rid of static Shade instances
Most of the time I define the shadows I will be using in my application in a ResourceDictionary.
<ResourceDictionary xmlns="http://xamarin.com/schemas/2014/forms"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:sh="clr-namespace:Sharpnado.Shades;assembly=Sharpnado.Shadows">
<sh:SingleShade x:Key="ShadowTop"
BlurRadius="6"
Opacity="0.15"
Offset="0,-8"
Color="{StaticResource ShadowsColor}" />
<sh:SingleShade x:Key="ShadowBottom"
BlurRadius="6"
Opacity="0.1"
Offset="0,5"
Color="{StaticResource ShadowsColor}" />
<sh:SingleShade x:Key="ShadowAccentBottom"
BlurRadius="6"
Opacity="0.4"
Offset="0,4"
Color="{StaticResource AgoraAccentColor}" />
<sh:ImmutableShades x:Key="ShadowNone" />
<sh:NeumorphismShades x:Key="ShadowNeumorphism" />
<sh:NeumorphismShades x:Key="ShadowThinNeumorphism"
LowerOffset="8, 6"
UpperOffset="-8,-6" />
</ResourceDictionary>
It's really convenient, but doing so, when we reference them as StaticResource, the same static instance will be added, creating the leak we saw earlier.
To break this behavior we'll just clone any ReadOnlyCollection<Shade> that is assigned to our Shadows, thanks to the coerceValue delegate:
public static readonly BindableProperty ShadesProperty = BindableProperty.Create(
nameof(Shades),
typeof(IEnumerable<Shade>),
typeof(Shadows),
defaultValueCreator: (bo) => new ObservableCollection<Shade> { new Shade { Parent = (Shadows)bo } },
validateValue: (bo, v) => v is IEnumerable<Shade>,
propertyChanged: ShadesPropertyChanged,
coerceValue: CoerceShades);
private static object CoerceShades(BindableObject bindable, object value)
{
if (!(value is ReadOnlyCollection<Shade> readonlyCollection))
{
return value;
}
return new ReadOnlyCollection<Shade>(
readonlyCollection.Select(s => s.Clone())
.ToList());
}
And now we are safe :)
2. Create weak events
Instead of having a strong relation between your event source and your subscribers, you can have a weak one, thanks to c# WeakReference.
Weak references don't prevent GC to reclaim the referenced object.
For Shadows I replaced all PropertyChanged events by WeakEvent thanks to Thomas Levesque package.
Testing it
To test our new implementation and the GC, we will add this code to the MainPage.xaml.cs:
protected override void OnAppearing()
{
base.OnAppearing();
BeCreative.OnAppearing();
Device.BeginInvokeOnMainThread(async () =>
{
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
await Task.Delay(500);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
});
}
It will make sure that the GC has all the time needed to reclaim all views.
Remember, on our main page we have 19 shadows renderer instances (see above).
When we load our shadows list, we have 30 instances loaded:
Now let's just back:
We have now 23 instances.
Well, weird but 4 instances weren't reclaimed by the GC...
After inspection, the button shadows of the header are not collected (1 renderer) + a complete cell (3 renderers per cell).
So maybe there are still some event leaks.
But if we go back and forth again:
We are back to 23!
Remark: After investigation, it seems the GC collects the 4 missing renderers during the next collection. So it is behind by 4 renderers in our case.
Testing it without forcing collections
Now let's try to test without the many GC calls in the OnAppearing() method of our main page.
When we go back from the shadows list page we have:
We have back to 29 instances. It means only one has been reclaimed, just like before...
Now let's go back and forth 3 times:
We have now 49 instances.
Since 10 shadow renderer views are created by the RecyclerView in the list page, it means that no recycled views from the CollectionView have been collected...
I even tried to make a lot of nested navigation:
MainPage => ListPage => MainPage => ListPage => MainPage => ListPage => MainPage => ListPage
And then pop all to close the app:
With the app closed, we have 34 instances not reclaimed by the GC...
Still an issue here
Unfortunately, it seems the GC cannot be trusted here to reclaim all CollectionView views.
It's not the first time that the Android GC seems to behave in an inconsistent way, you can have a look here:
https://www.sharpnado.com/gittrends-lags/
There is nothing much we can do from here...
I guess we need to be pragmatic here. For example, if your app runs into memory issues, you can try to force GC collections with all the code put in the OnAppearing() method.
Thank You Shane
I really wanted to thank Shane here that has been very available to make me understand how GC works with android's CollectionView.