Skip to content

Commit

Permalink
perf: Reduce memory indirect allocations from the use of DispatcherCo…
Browse files Browse the repository at this point in the history 
…nditionalDisposable

This change:
- Removes the use of delegates when interacting with DispatcherConditionalDisposable, which the original version required to deal with `Action` instances.
- Removes the use of Action in CreateWeakDelegate in favor a specialized dispose struct
  • Loading branch information
@jeromelaban
jeromelaban committed Jan 20, 2022
1 parent 6245c41 commit a5751f5
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 102 deletions.
263 changes: 168 additions & 95 deletions src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,33 +691,60 @@ internal IDisposable RegisterPropertyChangedCallback(DependencyProperty property
}
else
{
var weakDelegate = CreateWeakDelegate(callback);
CreateWeakDelegate(callback, out var weakCallback, out var weakDelegateRelease);

var cookie = propertyDetails.CallbackManager.RegisterCallback(weakDelegate.callback);
var cookie = propertyDetails.CallbackManager.RegisterCallback(weakCallback);

// Capture the weak reference to this instance.
var instanceRef = ThisWeakReference;
return new RegisterPropertyChangedCallbackForPropertyConditionalDisposable(
callback,
weakDelegateRelease,
cookie,
ThisWeakReference
);
}
}

return new DispatcherConditionalDisposable(
callback.Target,
instanceRef.CloneWeakReference(),
() =>
{
// This weak reference ensure that the closure will not link
// the caller and the callee, in the same way "newValueActionWeak"
// does not link the callee to the caller.
var that = instanceRef.Target as DependencyObjectStore;
/// <summary>
/// Specialized <see cref="DispatcherConditionalDisposable"/> for
/// <see cref="RegisterPropertyChangedCallback(DependencyProperty, PropertyChangedCallback, DependencyPropertyDetails?)"/>.
/// </summary>
/// <remarks>
/// This class is used to avoid the creation of a set of <see cref="Action"/> instances, as well as delegate invocations.
/// </remarks>
private class RegisterPropertyChangedCallbackForPropertyConditionalDisposable : DispatcherConditionalDisposable
{
private PropertyChangedCallback _callback;
private WeakReferenceReturnDisposable _releaseWeakDelegate;
private IDisposable _callbackManagedCookie;
private ManagedWeakReference _doStoreRef;

if (that != null)
{
cookie.Dispose();
weakDelegate.release.Dispose();
public RegisterPropertyChangedCallbackForPropertyConditionalDisposable(
PropertyChangedCallback callback,
WeakReferenceReturnDisposable releaseWeakDelegate,
IDisposable callbackManagerCookie,
ManagedWeakReference doStoreRef)
: base(callback.Target, doStoreRef.CloneWeakReference())
{
_callback = callback;
_releaseWeakDelegate = releaseWeakDelegate;
_callbackManagedCookie = callbackManagerCookie;
_doStoreRef = doStoreRef;
}

// Force a closure on the callback, to make its lifetime as long
// as the subscription being held by the callee.
callback = null!;
}
});
protected override void DispatchedTargetFinalized()
{
// This weak reference ensure that the closure will not link
// the caller and the callee, in the same way "newValueActionWeak"
// does not link the callee to the caller.
if (_doStoreRef.Target is DependencyObjectStore that)
{
_callbackManagedCookie.Dispose();
_releaseWeakDelegate.Dispose();

// Force a closure on the callback, to make its lifetime as long
// as the subscription being held by the callee.
_callback = null!;
}
}
}

Expand All @@ -731,39 +758,62 @@ internal IDisposable RegisterPropertyChangedCallback(ExplicitPropertyChangedCall
}
else
{
var weakDelegate = CreateWeakDelegate(handler);
CreateWeakDelegate(handler, out var weakHandler, out var weakHandlerRelease);

// Delegates integrate a null check when adding new delegates.
_genericCallbacks = _genericCallbacks.Add(weakDelegate.callback);
_genericCallbacks = _genericCallbacks.Add(weakHandler);

return new RegisterPropertyChangedCallbackConditionalDisposable(
weakHandler,
weakHandlerRelease,
ThisWeakReference,
handler
);
}
}

/// <summary>
/// Specialized DispatcherConditionalDisposable for <see cref="RegisterPropertyChangedCallback(ExplicitPropertyChangedCallback)"/>
/// </summary>
/// <remarks>
/// This class is used to avoid the creation of a set of <see cref="Action"/> instances, as well as delegate invocations.
/// </remarks>
private class RegisterPropertyChangedCallbackConditionalDisposable : DispatcherConditionalDisposable
{
private ExplicitPropertyChangedCallback _weakCallback;
private WeakReferenceReturnDisposable _weakCallbackRelease;
private ManagedWeakReference _instanceRef;
private ExplicitPropertyChangedCallback _callback;

public RegisterPropertyChangedCallbackConditionalDisposable(
ExplicitPropertyChangedCallback weakCallback,
WeakReferenceReturnDisposable weakCallbackRelease,
ManagedWeakReference instanceRef,
ExplicitPropertyChangedCallback callback)
: base(callback.Target, instanceRef.CloneWeakReference())
{
_weakCallback = weakCallback;
_weakCallbackRelease = weakCallbackRelease;
_instanceRef = instanceRef;
_callback = callback;
}

protected override void DispatchedTargetFinalized()
{
// This weak reference ensure that the closure will not link
// the caller and the callee, in the same way "newValueActionWeak"
// does not link the callee to the caller.
var instanceRef = ThisWeakReference;
if (_instanceRef.Target is DependencyObjectStore that)
{
// Delegates integrate a null check when removing new delegates.
that._genericCallbacks = that._genericCallbacks.Remove(_weakCallback);
}

return new DispatcherConditionalDisposable(
handler.Target,
instanceRef.CloneWeakReference(),
() =>
{
// This weak reference ensure that the closure will not link
// the caller and the callee, in the same way "newValueActionWeak"
// does not link the callee to the caller.
var that = instanceRef.Target as DependencyObjectStore;
if (that != null)
{
// Delegates integrate a null check when removing new delegates.
that._genericCallbacks = that._genericCallbacks.Remove(weakDelegate.callback);
}
weakDelegate.release.Dispose();
// Force a closure on the callback, to make its lifetime as long
// as the subscription being held by the callee.
handler = null!;
}
);
_weakCallbackRelease.Dispose();

// Force a closure on the callback, to make its lifetime as long
// as the subscription being held by the callee.
_callback = null!;
}
}

Expand Down Expand Up @@ -848,40 +898,69 @@ internal IDisposable RegisterParentChangedCallback(object key, ParentChangedCall
}
else
{
var wr = WeakReferencePool.RentWeakReference(this, callback);
var weakCallbackRef = WeakReferencePool.RentWeakReference(this, callback);

ParentChangedCallback weakDelegate =
(s, _, e) => (wr.Target as ParentChangedCallback)?.Invoke(s, key, e);
ParentChangedCallback weakCallback =
(s, _, e) => (weakCallbackRef.Target as ParentChangedCallback)?.Invoke(s, key, e);

_parentChangedCallbacks = _parentChangedCallbacks.Add(weakDelegate);
_parentChangedCallbacks = _parentChangedCallbacks.Add(weakCallback);

// This weak reference ensure that the closure will not link
// the caller and the callee, in the same way "newValueActionWeak"
// does not link the callee to the caller.
var instanceRef = ThisWeakReference;

void Cleanup()
{
var that = instanceRef.Target as DependencyObjectStore;
return new RegisterParentChangedCallbackConditionalDisposable(
instanceRef.CloneWeakReference(),
instanceRef,
weakCallbackRef,
weakCallback,
callback
);
}
}

if (that != null)
{
// Delegates integrate a null check when removing new delegates.
that._parentChangedCallbacks = that._parentChangedCallbacks.Remove(weakDelegate);
}
/// <summary>
/// Specialized DispatcherConditionalDisposable for <see cref="RegisterParentChangedCallback(object, ParentChangedCallback)"/>
/// </summary>
/// <remarks>
/// This class is used to avoid the creation of a set of <see cref="Action"/> instances, as well as delegate invocations.
/// </remarks>
private class RegisterParentChangedCallbackConditionalDisposable : DispatcherConditionalDisposable
{
private ManagedWeakReference _doStoreRef;
private ManagedWeakReference _weakCallbackRef;
private ParentChangedCallback _weakCallback;
private ParentChangedCallback _callback;

WeakReferencePool.ReturnWeakReference(that, wr);
public RegisterParentChangedCallbackConditionalDisposable(
WeakReference conditionSource,
ManagedWeakReference doStoreRef,
ManagedWeakReference weakCallbackRef,
ParentChangedCallback weakCallback,
ParentChangedCallback callback) : base(callback.Target, conditionSource)
{
_doStoreRef = doStoreRef;
_weakCallbackRef = weakCallbackRef;
_weakCallback = weakCallback;
_callback = callback;
}

// Force a closure on the callback, to make its lifetime as long
// as the subscription being held by the callee.
callback = null!;
protected override void DispatchedTargetFinalized()
{
var that = _doStoreRef.Target as DependencyObjectStore;

if (that != null)
{
// Delegates integrate a null check when removing new delegates.
that._parentChangedCallbacks = that._parentChangedCallbacks.Remove(_weakCallback);
}

return new DispatcherConditionalDisposable(
callback.Target,
instanceRef.CloneWeakReference(),
Cleanup
);
WeakReferencePool.ReturnWeakReference(that, _weakCallbackRef);

// Force a closure on the callback, to make its lifetime as long
// as the subscription being held by the callee.
_callback = null!;
}
}

Expand Down Expand Up @@ -1547,18 +1626,21 @@ private bool HasInherits(DependencyPropertyDetails propertyDetails)
/// on Mono 4.2 and earlier, when Full AOT is enabled. This should be revised once this behavior is updated, or
/// the cost of calling generic delegates is lowered.
/// </remarks>
private static (PropertyChangedCallback callback, IDisposable release) CreateWeakDelegate(PropertyChangedCallback callback)
private static void CreateWeakDelegate(
PropertyChangedCallback callback,
out PropertyChangedCallback weakCallback,
out WeakReferenceReturnDisposable weakRelease)
{
var wr = WeakReferencePool.RentWeakReference(null, callback);

PropertyChangedCallback weakDelegate =
weakCallback =
(s, e) => (!wr.IsDisposed ? wr.Target as PropertyChangedCallback : null)?.Invoke(s, e);

return (weakDelegate, Disposable.Create(() => WeakReferencePool.ReturnWeakReference(null, wr)));
weakRelease = new WeakReferenceReturnDisposable(wr);
}

/// <summary>
/// Creates a weak delegate for the specified <see cref="Action"/> callback.
/// Creates a weak delegate for the specified ExplicitPropertyChangedCallback callback.
/// </summary>
/// <param name="callback">The callback to reference</param>
/// <remarks>
Expand All @@ -1570,37 +1652,28 @@ private static (PropertyChangedCallback callback, IDisposable release) CreateWea
/// on Mono 4.2 and earlier, when Full AOT is enabled. This should be revised once this behavior is updated, or
/// the cost of calling generic delegates is lowered.
/// </remarks>
private static (Action callback, IDisposable release) CreateWeakDelegate(Action callback)
private static void CreateWeakDelegate(
ExplicitPropertyChangedCallback callback,
out ExplicitPropertyChangedCallback weakDelegate,
out WeakReferenceReturnDisposable weakRelease)
{
var wr = WeakReferencePool.RentWeakReference(null, callback);

Action weakDelegate =
() => (wr.Target as Action)?.Invoke();
weakDelegate =
(instance, s, e) => (wr.Target as ExplicitPropertyChangedCallback)?.Invoke(instance, s, e);

return (weakDelegate, Disposable.Create(() => WeakReferencePool.ReturnWeakReference(null, wr)));
weakRelease = new WeakReferenceReturnDisposable(wr);
}

/// <summary>
/// Creates a weak delegate for the specified ExplicitPropertyChangedCallback callback.
/// </summary>
/// <param name="callback">The callback to reference</param>
/// <remarks>
/// This method is used to avoid creating a hard link between the source instance
/// and the stored delegate for the instance, thus avoid memory leaks.
/// We also do not need to clear the delegate created because it is already associated with the instance.
///
/// Note that this method is not generic to avoid the cost of trampoline resolution
/// on Mono 4.2 and earlier, when Full AOT is enabled. This should be revised once this behavior is updated, or
/// the cost of calling generic delegates is lowered.
/// </remarks>
private static (ExplicitPropertyChangedCallback callback, IDisposable release) CreateWeakDelegate(ExplicitPropertyChangedCallback callback)
private struct WeakReferenceReturnDisposable
{
var wr = WeakReferencePool.RentWeakReference(null, callback);
private readonly ManagedWeakReference _wr;

ExplicitPropertyChangedCallback weakDelegate =
(instance, s, e) => (wr.Target as ExplicitPropertyChangedCallback)?.Invoke(instance, s, e);
public WeakReferenceReturnDisposable(ManagedWeakReference wr)
=> _wr = wr;

return (weakDelegate, Disposable.Create(() => WeakReferencePool.ReturnWeakReference(null, wr)));
public void Dispose()
=> WeakReferencePool.ReturnWeakReference(null, _wr);
}

private void RaiseCallbacks(
Expand Down
13 changes: 6 additions & 7 deletions src/Uno.UI/UI/Xaml/DispatcherConditionalDisposable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,29 @@ namespace Windows.UI.Xaml
/// capturing lambda to be passed as a callback, and not have to unintended memory leaks
/// on either the sender or receiver of the callback.
/// </remarks>
internal class DispatcherConditionalDisposable : ConditionalDisposable
internal abstract class DispatcherConditionalDisposable : ConditionalDisposable
{
private readonly Action _action;

public DispatcherConditionalDisposable(object target, WeakReference conditionSource, Action action) : base(target, conditionSource)
public DispatcherConditionalDisposable(object target, WeakReference conditionSource) : base(target, conditionSource)
{
_action = action;
}

protected override void TargetFinalized()
{
if (CoreDispatcher.Main.HasThreadAccess)
{
_action();
DispatchedTargetFinalized();
}
else
{
Uno.UI.Dispatching.CoreDispatcher.Main.RunIdleAsync(
delegate
{
_action();
DispatchedTargetFinalized();
}
);
}
}

protected abstract void DispatchedTargetFinalized();
}
}

0 comments on commit a5751f5

Please sign in to comment.