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

perf: Don't use multicast delegates for DependencyObjectCollection.VectorChanged #18910

Merged
merged 3 commits into from
Nov 27, 2024
Merged
Changes from 1 commit
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
56 changes: 54 additions & 2 deletions src/Uno.UI/UI/Xaml/DependencyObjectCollection.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Buffers;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
Expand All @@ -25,7 +26,29 @@
public partial class DependencyObjectCollection<T> : DependencyObjectCollectionBase, IList<T>, IEnumerable<T>, IEnumerable, IObservableVector<T>
where T : DependencyObject
{
public event VectorChangedEventHandler<T> VectorChanged;
private object _vectorChangedHandlersLock = new();

Check warning on line 29 in src/Uno.UI/UI/Xaml/DependencyObjectCollection.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Uno.UI/UI/Xaml/DependencyObjectCollection.cs#L29

Make '_vectorChangedHandlersLock' 'readonly'.

// Explicit handlers list to avoid the cost of multicast delegates handling
private List<VectorChangedEventHandler<T>> _vectorChangedHandlers;

public event VectorChangedEventHandler<T> VectorChanged
{
add
{
lock (_vectorChangedHandlersLock)
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
{
(_vectorChangedHandlers ??= new()).Add(value);
}
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
}

remove
{
lock (_vectorChangedHandlersLock)
{
(_vectorChangedHandlers ??= new()).Remove(value);
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

private readonly List<T> _list = new List<T>();

Expand Down Expand Up @@ -205,7 +228,36 @@
=> _list.GetEnumerator();

private void RaiseVectorChanged(CollectionChange change, int index)
=> VectorChanged?.Invoke(this, new VectorChangedEventArgs(change, (uint)index));
{
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
lock (_vectorChangedHandlersLock)
{
if (_vectorChangedHandlers is { Count: > 0 })
{
var args = new VectorChangedEventArgs(change, (uint)index);

if (_vectorChangedHandlers.Count == 1)
{
_vectorChangedHandlers[0].Invoke(this, args);
}
else
{
// Clone the array to account for reentrancy.
var handlersClone = ArrayPool<VectorChangedEventHandler<T>>.Shared.Rent(_vectorChangedHandlers.Count);
_vectorChangedHandlers.CopyTo(handlersClone, 0);

// Use the original count, the rented array may be larger.
var count = _vectorChangedHandlers.Count;

for (int i = 0; i < count; i++)
{
handlersClone[i].Invoke(this, args);
}

ArrayPool<VectorChangedEventHandler<T>>.Shared.Return(handlersClone);
}
}
}
}

private protected virtual void OnAdded(T d)
{
Expand Down
Loading