-
Notifications
You must be signed in to change notification settings - Fork 581
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
8.0 Discussion about public events #973
Comments
I've done more investigation into this. Here's the outcome: Invocation:
Add/remove Handler:
Sources: internal struct EventActionInvoker<T>
{
private event Action<T>? _event;
private Delegate[]? _handlers;
private Action<Exception>? _onExceptionAction;
public bool IsEmpty => _event is null;
public EventActionInvoker(Action<Exception> onExceptionAction)
{
_event = null;
_handlers = null;
_onExceptionAction = onExceptionAction;
}
public void AddHandler(Action<T>? handler)
{
_event += handler;
_handlers = null;
}
public void RemoveHandler(Action<T>? handler)
{
_event -= handler;
_handlers = null;
}
public void Invoke(T parameter)
{
var handlers = _handlers;
if (handlers is null)
{
handlers = _event?.GetInvocationList();
if (handlers is null)
{
return;
}
_handlers = handlers;
}
foreach (Action<T> action in handlers)
{
try
{
action(parameter);
}
catch (Exception e)
{
_onExceptionAction?.Invoke(e);
}
}
}
public void Takeover(in EventActionInvoker<T> other)
{
_event = other._event;
_handlers = other._handlers;
_onExceptionAction = other._onExceptionAction;
}
} Bench - Add/Remove Handler using System;
using System.Collections.Generic;
using BenchmarkDotNet.Attributes;
using RabbitMQ.Client.client.impl.Channel;
namespace Benchmarks.Eventing
{
[ShortRunJob]
[MemoryDiagnoser]
public class Eventing_AddRemove
{
private readonly Action<ulong> _action = _ => { };
private List<Action<ulong>> _list;
private event Action<ulong> _event;
private EventActionInvoker<ulong> _invoker;
[Benchmark(Baseline = true)]
[Arguments(1)]
[Arguments(5)]
public void Event(int count)
{
for (int i = 0; i < count; i++)
{
_event += _action;
}
for (int i = 0; i < count; i++)
{
_event -= _action;
}
}
[Benchmark]
[Arguments(1)]
[Arguments(5)]
public void Invoker(int count)
{
for (int i = 0; i < count; i++)
{
_invoker.AddHandler(_action);
}
for (int i = 0; i < count; i++)
{
_invoker.RemoveHandler(_action);
}
}
[Benchmark]
[Arguments(1)]
[Arguments(5)]
public void List(int count)
{
_list = new List<Action<ulong>>(1);
for (int i = 0; i < count; i++)
{
_list.Add(_action);
}
for (int i = 0; i < count; i++)
{
_list.Remove(_action);
}
}
}
} Bench - Invoke using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using BenchmarkDotNet.Attributes;
using RabbitMQ.Client.client.impl.Channel;
namespace Benchmarks.WireFormatting
{
[ShortRunJob]
[MemoryDiagnoser]
[DisassemblyDiagnoser]
public class Eventing_Invoke
{
private List<Action<ulong>> _list;
private event Action<ulong> _event;
private EventActionInvoker<ulong> _invoker;
public Eventing_Invoke()
{
_invoker = new EventActionInvoker<ulong>(ex => OnUnhandledExceptionOccurred(ex, "ContextString"));
}
[Params(1, 2)]
public int Length
{
set
{
_list = new List<Action<ulong>>(value);
var action = new Action<ulong>(tag => { });
for (int i = 0; i < value; i++)
{
_event += action;
_invoker.AddHandler(action);
_list.Add(action);
}
}
}
[Benchmark(Baseline = true)]
public void SingleInvoke()
{
_event?.Invoke(5UL);
}
[Benchmark]
public void Original_GetInvocationList()
{
var handler = _event;
if (handler != null)
{
foreach (Action<ulong> action in handler.GetInvocationList())
{
try
{
action(5UL);
}
catch (Exception e)
{
OnUnhandledExceptionOccurred(e, "ContextString");
}
}
}
}
[Benchmark]
public void List()
{
var handler = _event;
if (handler != null)
{
foreach (Action<ulong> action in _list)
{
try
{
action(5UL);
}
catch (Exception e)
{
OnUnhandledExceptionOccurred(e, "ContextString");
}
}
}
}
[MethodImpl(MethodImplOptions.NoInlining)]
private void OnUnhandledExceptionOccurred(Exception exception, string context)
{
}
[Benchmark]
public void Invoker()
{
_invoker.Invoke(5UL);
}
}
} Sidenote: The List approach is not thread safe while the others are. (Implementing it would degregate the performance to some degree I suppose) |
Originating from #970 the discussion is about how we deal with the currently public events.
Starter was the post from @stebet
I've done some measurements today regarding performance of various ways of invoking actions To have also an idea about what this means.
Where
And the first entry always being for one attached event handler and the 2nd one for two handlers (action += handler) (Or entries in the list)
I think the relevant notes are:
This issue should be taken as the place to dicuss where we want to move to with the events in our public API.
The text was updated successfully, but these errors were encountered: