Reputation: 1277
In my application, some value can change at any time. A lot of components need to do something when this value changes. The number of components changes during the usage of the application (depending on what the user opens). Currently this is implemented with events: when value changes, an event is raised, on which all interested components are subscribed.
Of course, to avoid memory leaks, these components unsubscribe on dispose.
When we stress-test our application with lots of components (a few million), one of the biggest bottle necks (>90% cpu time during high loads) are these subscriptions: MyApp.ValueChanged -= TheValueChanged;
takes very long (multiple seconds).
I assume this is, because there are a lot of subscribers, and this unsubscribe needs to find the correct subscriber in the list of subscribers (worst case: a million time searching in a list of a million items).
Now, my question: What would be a good way to improve this?
I was thinking about weak event handlers, so unsubscribing isn't necessary, but there may be better solutions.
Upvotes: 1
Views: 704
Reputation: 19966
GC is the culprit. Period.
First of all, the default implementation of strong event handlers is not optimized for many listeners. To get decent performance, consider implementing the event handler explicitly. Using a hash set we can improve on the default implementation which internally uses a flat list.
For your event, implement add
and remove
using a HashSet
. Note that this implementation is not thread-safe. You might want to add a locking mechanism if multiple threads will be using the event.
class Publisher : INotifyPropertyChanged
{
private HashSet<PropertyChangedEventHandler> propertyChangedHandlers =
new HashSet<PropertyChangedEventHandler>();
public event PropertyChangedEventHandler PropertyChanged
{
add => propertyChangedHandlers.Add(value);
remove => propertyChangedHandlers.Remove(value);
}
public void Signal()
{
var args = new PropertyChangedEventArgs(null);
foreach (var handler in propertyChangedHandlers)
{
handler(this, new PropertyChangedEventArgs(null));
}
}
public void RemoveSubscribers(IEnumerable<Listener> listeners)
{
foreach (var listener in listeners)
{
listener.Subscribe(this);
}
}
}
This will perform a lot better. But why? Sure, removing items from a hash set is a lot faster than traversing a flat list, but it's not that slow. The extreme CPU time actually comes from GC
. When unsubscribing an event, the list of handlers will be re-allocated. The old list will then, quite soon, get collected by the Garbage Collector.
In your case, you have 1,000,000
event handlers. So each time you unsubscribe an event handler, a list of N
items (where N
is near 1000000) is unreferenced and a new list of N-1
handlers is allocated. Do this 100 times, and the GC
will have lots of data to memory to collect:
100 * 1000000 * 8 bytes = ~800 MB
We can easily prove this using the GC.TryStartNoGCRegion API. The sample below tries to unsubscribe 20 event handlers, but since this operation will indeed allocate more than the requested 128 MB, it will fail:
Unhandled Exception: System.InvalidOperationException: Allocated memory exceeds specified memory for NoGCRegion mode at System.GC.EndNoGCRegionWorker()
class Publisher : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged(string propertyName = null)
{
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
}
}
class Listener
{
public void Subscribe(Publisher publisher)
{
publisher.PropertyChanged += OnPropertyChanged;
}
public void Unsubscribe(Publisher publisher)
{
publisher.PropertyChanged -= OnPropertyChanged;
}
private static void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
Console.WriteLine($"OnPropertyChanged called for {nameof(Listener)} {sender}");
}
}
class Program
{
static void Main(string[] args)
{
var publisher = new Publisher();
var listeners = Enumerable.Range(0, 1000000)
.Select(p => new Listener())
.ToList();
foreach (var listener in listeners)
{
listener.Subscribe(publisher);
}
var watch = new System.Diagnostics.Stopwatch();
watch.Start();
const int toRemove = 20;
if (GC.TryStartNoGCRegion(128L * 1024L * 1024L, true))
{
try
{
for (int i = 0; i < toRemove; i++)
{
listeners[i].Unsubscribe(publisher);
}
}
finally
{
watch.Stop();
var time = watch.ElapsedMilliseconds;
Console.WriteLine($"Removing {toRemove} handlers: {time} ms");
GC.EndNoGCRegion();
}
}
}
}
If we run a memory profiler, we can see that removing event handlers does indeed cause allocations and GC
.
See MulticastDelegate.cs(360) for more details.
Upvotes: 1
Reputation: 1277
I found this answer, which suggest another possible improvement: https://stackoverflow.com/a/24239037/1851717
However, some experimentation led me to the decision to go with my first thought: use a weak event manager, to skip unsubscribing altogether. I've written a custom weak event manager, as there are some extra optimizations I could do, making use of some application-specifc characteristics.
So in general, summarizing the comments, and solutions, when your event unsubscribing is a bottleneck, check these things:
Upvotes: 0