Skip to content

Commit

Permalink
perf(memory): Reduce ConditionalDisposable pressure
Browse files Browse the repository at this point in the history
- Remove indirect delegates created from `ConditionalDisposable`
- Use strong references for property changes registrations to avoid the creation of `ConditionalDisposable` instances
- If `RegisterPropertyChangedCallback` is used and is registered on self, don't use `DispatcherConditionalDisposable`
- Generate `OnPropertyChanged2` for all uno-internal DependencyObject to avoid delegates creation. DependencyObject now invokes this method through `IDependencyObjectInternal`.
  • Loading branch information
jeromelaban committed Jan 18, 2022
1 parent d4544e6 commit 3b4bf41
Show file tree
Hide file tree
Showing 14 changed files with 196 additions and 121 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ private class SerializationMethodsGenerator : SymbolVisitor
private readonly INamedTypeSymbol? _bindableAttributeSymbol;
private readonly INamedTypeSymbol? _iFrameworkElementSymbol;
private readonly INamedTypeSymbol? _frameworkElementSymbol;
private readonly bool _isUnoSolution;

public SerializationMethodsGenerator(GeneratorExecutionContext context)
{
Expand All @@ -66,6 +67,7 @@ public SerializationMethodsGenerator(GeneratorExecutionContext context)
_bindableAttributeSymbol = comp.GetTypeByMetadataName("Windows.UI.Xaml.Data.BindableAttribute");
_iFrameworkElementSymbol = comp.GetTypeByMetadataName(XamlConstants.Types.IFrameworkElement);
_frameworkElementSymbol = comp.GetTypeByMetadataName("Windows.UI.Xaml.FrameworkElement");
_isUnoSolution = _context.GetMSBuildPropertyValue("_IsUnoUISolution") == "true";
}

public override void VisitNamedType(INamedTypeSymbol type)
Expand Down Expand Up @@ -116,7 +118,7 @@ private void ProcessType(INamedTypeSymbol typeSymbol)

if (isDependencyObject)
{
if (_context.GetMSBuildPropertyValue("_IsUnoUISolution") != "true")
if (!_isUnoSolution)
{
if (typeSymbol.Is(_iosViewSymbol))
{
Expand Down Expand Up @@ -162,9 +164,11 @@ private void ProcessType(INamedTypeSymbol typeSymbol)
builder.AppendLineInvariant(@"[global::Windows.UI.Xaml.Data.Bindable]");
}

using (builder.BlockInvariant($"partial class {typeSymbol.Name} : IDependencyObjectStoreProvider, IWeakReferenceProvider"))
var internalDependencyObject = _isUnoSolution && !typeSymbol.IsSealed ? ", IDependencyObjectInternal" : "";

using (builder.BlockInvariant($"partial class {typeSymbol.Name} : IDependencyObjectStoreProvider, IWeakReferenceProvider{internalDependencyObject}"))
{
GenerateDependencyObjectImplementation(builder);
GenerateDependencyObjectImplementation(typeSymbol, builder);
GenerateIBinderImplementation(typeSymbol, builder);
}
}
Expand Down Expand Up @@ -851,7 +855,7 @@ public override bool Equals(object other)
}
}

private static void GenerateDependencyObjectImplementation(IndentedStringBuilder builder)
private void GenerateDependencyObjectImplementation(INamedTypeSymbol typeSymbol, IndentedStringBuilder builder)
{
builder.AppendLineInvariant(@"private DependencyObjectStore __storeBackingField;");
builder.AppendLineInvariant(@"public Windows.UI.Core.CoreDispatcher Dispatcher => Windows.ApplicationModel.Core.CoreApplication.MainView.Dispatcher;");
Expand Down Expand Up @@ -890,6 +894,16 @@ private static void GenerateDependencyObjectImplementation(IndentedStringBuilder
builder.AppendLineInvariant("public long RegisterPropertyChangedCallback(DependencyProperty dp, DependencyPropertyChangedCallback callback) => __Store.RegisterPropertyChangedCallback(dp, callback);");

builder.AppendLineInvariant("public void UnregisterPropertyChangedCallback(DependencyProperty dp, long token) => __Store.UnregisterPropertyChangedCallback(dp, token);");

if (_isUnoSolution && !typeSymbol.IsSealed)
{
builder.AppendLineInvariant("void IDependencyObjectInternal.OnPropertyChanged2(global::Windows.UI.Xaml.DependencyPropertyChangedEventArgs args) => OnPropertyChanged2(args);");

if (typeSymbol.GetMethodsWithName("OnPropertyChanged2").None(m => m.Parameters.Length == 1))
{
builder.AppendLineInvariant("internal virtual void OnPropertyChanged2(global::Windows.UI.Xaml.DependencyPropertyChangedEventArgs args) {{ }}");
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ namespace Uno.Disposables
/// <summary>
/// A disposable that can call an action when a dependent object has been collected.
/// </summary>
internal class ConditionalDisposable : IDisposable
internal abstract class ConditionalDisposable : IDisposable
{
private static ConditionalWeakTable<object, List<IDisposable>> _registrations = new ConditionalWeakTable<object, List<IDisposable>>();

private readonly Action _action;
private bool _disposed;
private readonly WeakReference? _conditionSource;
private readonly List<IDisposable> _list;
Expand All @@ -43,12 +42,10 @@ internal class ConditionalDisposable : IDisposable
/// Creates a <see cref="ConditionalDisposable"/> instance using
/// <paramref name="target"/> as a reference for its lifetime.
/// </summary>
/// <param name="action">The action to be executed when target has been collected</param>
/// <param name="conditionSource">An optional secondary reference, used to avoid calling action if it has been collected</param>
/// <param name="target">The instance to use to keep the disposable alive</param>
public ConditionalDisposable(object target, Action action, WeakReference? conditionSource = null)
public ConditionalDisposable(object target, WeakReference? conditionSource = null)
{
_action = action;
_conditionSource = conditionSource;

#if DEBUG
Expand All @@ -74,6 +71,8 @@ public void Dispose()
Dispose(true);
}

protected abstract void TargetFinalized();

private void Dispose(bool disposing)
{
if(disposing)
Expand All @@ -94,7 +93,7 @@ private void Dispose(bool disposing)
{
_disposed = true;

_action();
TargetFinalized();
}
}
}
Expand All @@ -105,5 +104,4 @@ private void Dispose(bool disposing)
Dispose(false);
}
}

}
12 changes: 10 additions & 2 deletions src/Uno.UI/UI/Xaml/Controls/DropDownButton/DropDownButton.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,19 @@ protected override void OnApplyTemplate()
{
base.OnApplyTemplate();

this.RegisterDisposablePropertyChangedCallback(Button.FlyoutProperty, OnFlyoutPropertyChanged);

RegisterFlyoutEvents();
}

internal override void OnPropertyChanged2(DependencyPropertyChangedEventArgs args)
{
if (args.Property == Button.FlyoutProperty)
{
OnFlyoutPropertyChanged(this, args);
}

base.OnPropertyChanged2(args);
}

private void RegisterFlyoutEvents()
{
if (Flyout != null)
Expand Down
11 changes: 7 additions & 4 deletions src/Uno.UI/UI/Xaml/Controls/Grid/ColumnDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ public ColumnDefinition()
{
InitializeBinder();
IsAutoPropertyInheritanceEnabled = false;
}

this.RegisterDisposablePropertyChangedCallback((i, p, args) =>
{
InvalidateDefinition();
});
/// <remarks>
/// This method is called from the generated IDependencyObjectInternal.OnPropertyChanged2 method
/// </summary>
internal void OnPropertyChanged2(DependencyPropertyChangedEventArgs args)
{
InvalidateDefinition();
}

#region Width DependencyProperty
Expand Down
11 changes: 7 additions & 4 deletions src/Uno.UI/UI/Xaml/Controls/Grid/RowDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ public RowDefinition()
{
InitializeBinder();
IsAutoPropertyInheritanceEnabled = false;
}

this.RegisterDisposablePropertyChangedCallback((i, p, args) =>
{
InvalidateDefinition();
});
/// <remarks>
/// This method is called from the generated IDependencyObjectInternal.OnPropertyChanged2 method
/// </summary>
internal void OnPropertyChanged2(DependencyPropertyChangedEventArgs args)
{
InvalidateDefinition();
}

#region Height DependencyProperty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ bool IScrollContentPresenter.ForceChangeToCurrentView

private void InitializeScrollContentPresenter()
{
this.RegisterParentChangedCallback(this, OnParentChanged);
this.RegisterParentChangedCallbackStrong(this, OnParentChanged);
}

private void OnParentChanged(object instance, object key, DependencyObjectParentChangedEventArgs args)
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public partial class TextBox : Control, IFrameworkTemplatePoolAware

public TextBox()
{
this.RegisterParentChangedCallback(this, OnParentChanged);
this.RegisterParentChangedCallbackStrong(this, OnParentChanged);

DefaultStyleKey = typeof(TextBox);
SizeChanged += OnSizeChanged;
Expand Down
10 changes: 10 additions & 0 deletions src/Uno.UI/UI/Xaml/DependencyObjectExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,16 @@ internal static IDisposable RegisterParentChangedCallback(this DependencyObject
return GetStore(instance).RegisterParentChangedCallback(key, handler);
}

/// <summary>
/// Registers to parent changes.
/// </summary>
/// <param name="instance">The target dependency object</param>
/// <param name="key">A key to be passed to the callback parameter.</param>
/// <param name="handler">A callback to be called</param>
/// <returns>A disposable that cancels the subscription.</returns>
internal static void RegisterParentChangedCallbackStrong(this DependencyObject instance, object key, ParentChangedCallback handler)
=> GetStore(instance).RegisterParentChangedCallback(key, handler);

/// <summary>
/// Determines if the specified dependency property is set.
/// A property is set whenever a value (including null) is assigned to it.
Expand Down

0 comments on commit 3b4bf41

Please sign in to comment.