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

Remove disposable allocations from routed event AddHandler in the common case. #3651

Merged
merged 3 commits into from
Mar 11, 2020
Merged
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
2 changes: 1 addition & 1 deletion src/Avalonia.Controls/Calendar/DatePicker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ protected override void OnTemplateApplied(TemplateAppliedEventArgs e)
{
_dropDownButton.Click += DropDownButton_Click;
_buttonPointerPressedSubscription =
_dropDownButton.AddHandler(PointerPressedEvent, DropDownButton_PointerPressed, handledEventsToo: true);
_dropDownButton.AddDisposableHandler(PointerPressedEvent, DropDownButton_PointerPressed, handledEventsToo: true);
}

if (_textBox != null)
Expand Down
3 changes: 2 additions & 1 deletion src/Avalonia.Controls/ComboBox.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Avalonia.Controls.Shapes;
using Avalonia.Controls.Templates;
using Avalonia.Input;
using Avalonia.Interactivity;
using Avalonia.LogicalTree;
using Avalonia.Media;
using Avalonia.VisualTree;
Expand Down Expand Up @@ -265,7 +266,7 @@ private void PopupOpened(object sender, EventArgs e)
var toplevel = this.GetVisualRoot() as TopLevel;
if (toplevel != null)
{
_subscriptionsOnOpen = toplevel.AddHandler(PointerWheelChangedEvent, (s, ev) =>
_subscriptionsOnOpen = toplevel.AddDisposableHandler(PointerWheelChangedEvent, (s, ev) =>
{
//eat wheel scroll event outside dropdown popup while it's open
if (IsDropDownOpen && (ev.Source as IVisual).GetVisualRoot() == toplevel)
Expand Down
2 changes: 1 addition & 1 deletion src/Avalonia.Controls/Primitives/Popup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ void DeferCleanup(IDisposable? disposable)
}
}

DeferCleanup(topLevel.AddHandler(PointerPressedEvent, PointerPressedOutside, RoutingStrategies.Tunnel));
DeferCleanup(topLevel.AddDisposableHandler(PointerPressedEvent, PointerPressedOutside, RoutingStrategies.Tunnel));

DeferCleanup(InputManager.Instance?.Process.Subscribe(ListenForNonClientClick));

Expand Down
2 changes: 1 addition & 1 deletion src/Avalonia.Diagnostics/Diagnostics/DevTools.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void PreviewKeyDown(object sender, KeyEventArgs e)
}
}

return root.AddHandler(
return root.AddDisposableHandler(
InputElement.KeyDownEvent,
PreviewKeyDown,
RoutingStrategies.Tunnel);
Expand Down
4 changes: 2 additions & 2 deletions src/Avalonia.Interactivity/IInteractive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public interface IInteractive
/// <param name="routes">The routing strategies to listen to.</param>
/// <param name="handledEventsToo">Whether handled events should also be listened for.</param>
/// <returns>A disposable that terminates the event subscription.</returns>
IDisposable AddHandler(
void AddHandler(
RoutedEvent routedEvent,
Delegate handler,
RoutingStrategies routes = RoutingStrategies.Direct | RoutingStrategies.Bubble,
Expand All @@ -38,7 +38,7 @@ IDisposable AddHandler(
/// <param name="routes">The routing strategies to listen to.</param>
/// <param name="handledEventsToo">Whether handled events should also be listened for.</param>
/// <returns>A disposable that terminates the event subscription.</returns>
IDisposable AddHandler<TEventArgs>(
void AddHandler<TEventArgs>(
RoutedEvent<TEventArgs> routedEvent,
EventHandler<TEventArgs> handler,
RoutingStrategies routes = RoutingStrategies.Direct | RoutingStrategies.Bubble,
Expand Down
32 changes: 6 additions & 26 deletions src/Avalonia.Interactivity/Interactive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ public class Interactive : Layoutable, IInteractive
/// <param name="handler">The handler.</param>
/// <param name="routes">The routing strategies to listen to.</param>
/// <param name="handledEventsToo">Whether handled events should also be listened for.</param>
/// <returns>A disposable that terminates the event subscription.</returns>
public IDisposable AddHandler(
public void AddHandler(
RoutedEvent routedEvent,
Delegate handler,
RoutingStrategies routes = RoutingStrategies.Direct | RoutingStrategies.Bubble,
Expand All @@ -38,7 +37,8 @@ public IDisposable AddHandler(
handler = handler ?? throw new ArgumentNullException(nameof(handler));

var subscription = new EventSubscription(handler, routes, handledEventsToo);
return AddEventSubscription(routedEvent, subscription);

AddEventSubscription(routedEvent, subscription);
}

/// <summary>
Expand All @@ -49,8 +49,7 @@ public IDisposable AddHandler(
/// <param name="handler">The handler.</param>
/// <param name="routes">The routing strategies to listen to.</param>
/// <param name="handledEventsToo">Whether handled events should also be listened for.</param>
/// <returns>A disposable that terminates the event subscription.</returns>
public IDisposable AddHandler<TEventArgs>(
public void AddHandler<TEventArgs>(
RoutedEvent<TEventArgs> routedEvent,
EventHandler<TEventArgs> handler,
RoutingStrategies routes = RoutingStrategies.Direct | RoutingStrategies.Bubble,
Expand All @@ -69,7 +68,7 @@ static void InvokeAdapter(Delegate baseHandler, object sender, RoutedEventArgs a

var subscription = new EventSubscription(handler, routes, handledEventsToo, (baseHandler, sender, args) => InvokeAdapter(baseHandler, sender, args));

return AddEventSubscription(routedEvent, subscription);
AddEventSubscription(routedEvent, subscription);
}

/// <summary>
Expand Down Expand Up @@ -188,7 +187,7 @@ protected EventRoute BuildEventRoute(RoutedEvent e)
return result;
}

private IDisposable AddEventSubscription(RoutedEvent routedEvent, EventSubscription subscription)
private void AddEventSubscription(RoutedEvent routedEvent, EventSubscription subscription)
{
_eventHandlers ??= new Dictionary<RoutedEvent, List<EventSubscription>>();

Expand All @@ -199,8 +198,6 @@ private IDisposable AddEventSubscription(RoutedEvent routedEvent, EventSubscript
}

subscriptions.Add(subscription);

return new UnsubscribeDisposable(subscriptions, subscription);
}

private readonly struct EventSubscription
Expand All @@ -225,22 +222,5 @@ public EventSubscription(

public bool HandledEventsToo { get; }
}

private sealed class UnsubscribeDisposable : IDisposable
{
private readonly List<EventSubscription> _subscriptions;
private readonly EventSubscription _subscription;

public UnsubscribeDisposable(List<EventSubscription> subscriptions, EventSubscription subscription)
{
_subscriptions = subscriptions;
_subscription = subscription;
}

public void Dispose()
{
_subscriptions.Remove(_subscription);
}
}
}
}
25 changes: 24 additions & 1 deletion src/Avalonia.Interactivity/InteractiveExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See licence.md file in the project root for full license information.

using System;
using System.Reactive.Disposables;
using System.Reactive.Linq;

namespace Avalonia.Interactivity
Expand All @@ -11,6 +12,28 @@ namespace Avalonia.Interactivity
/// </summary>
public static class InteractiveExtensions
{
/// <summary>
/// Adds a handler for the specified routed event and returns a disposable that can terminate the event subscription.
/// </summary>
/// <typeparam name="TEventArgs">The type of the event's args.</typeparam>
/// <param name="o">Target for adding given event handler.</param>
/// <param name="routedEvent">The routed event.</param>
/// <param name="handler">The handler.</param>
/// <param name="routes">The routing strategies to listen to.</param>
/// <param name="handledEventsToo">Whether handled events should also be listened for.</param>
/// <returns>A disposable that terminates the event subscription.</returns>
public static IDisposable AddDisposableHandler<TEventArgs>(this IInteractive o, RoutedEvent<TEventArgs> routedEvent,
EventHandler<TEventArgs> handler,
RoutingStrategies routes = RoutingStrategies.Direct | RoutingStrategies.Bubble,
bool handledEventsToo = false) where TEventArgs : RoutedEventArgs
{
o.AddHandler(routedEvent, handler, routes, handledEventsToo);

return Disposable.Create(
(instance: o, handler, routedEvent),
state => state.instance.RemoveHandler(state.routedEvent, state.handler));
}

/// <summary>
/// Gets an observable for a <see cref="RoutedEvent{TEventArgs}"/>.
/// </summary>
Expand All @@ -31,7 +54,7 @@ public static IObservable<TEventArgs> GetObservable<TEventArgs>(
o = o ?? throw new ArgumentNullException(nameof(o));
routedEvent = routedEvent ?? throw new ArgumentNullException(nameof(routedEvent));

return Observable.Create<TEventArgs>(x => o.AddHandler(
return Observable.Create<TEventArgs>(x => o.AddDisposableHandler(
routedEvent,
(_, e) => x.OnNext(e),
routes,
Expand Down
11 changes: 11 additions & 0 deletions tests/Avalonia.Benchmarks/NullGlyphRun.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using Avalonia.Platform;

namespace Avalonia.Benchmarks
{
internal class NullGlyphRun : IGlyphRunImpl
{
public void Dispose()
{
}
}
}
4 changes: 3 additions & 1 deletion tests/Avalonia.Benchmarks/NullRenderingPlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ public IFontManagerImpl CreateFontManager()

public IGlyphRunImpl CreateGlyphRun(GlyphRun glyphRun, out double width)
{
throw new NotImplementedException();
width = default;

return new NullGlyphRun();
}
}
}