Skip to content

Commit

Permalink
Merge pull request #18910 from unoplatform/dev/jela/overflow-perf
Browse files Browse the repository at this point in the history
perf: Don't use multicast delegates for DependencyObjectCollection.VectorChanged
  • Loading branch information
jeromelaban authored Nov 27, 2024
2 parents 11c4968 + 7c41e28 commit 5b90f03
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using System.Collections.Generic;
using System.Linq;
using System.Runtime.InteropServices.ObjectiveC;
using Microsoft.UI.Xaml;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml;

[TestClass]
public class Given_DependencyObjectCollection
{
[TestMethod]
public void When_Add_Multiple_And_Invoke()
{
DependencyObjectCollection c = new();

List<string> list = [];

void One(object sender, object args) => list.Add("One");
void Two(object sender, object args) => list.Add("Two");

c.VectorChanged += One;
c.VectorChanged += Two;
c.VectorChanged += One;
c.VectorChanged -= One;

c.Add(c);

Assert.IsTrue(list.SequenceEqual(["One", "Two"]));
}
}
91 changes: 89 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,37 @@ public partial class DependencyObjectCollectionBase : DependencyObject
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();

// Explicit handlers list to avoid the cost of generic multicast
// delegates handling on mono's AOT.
private List<VectorChangedEventHandler<T>> _vectorChangedHandlers;

public event VectorChangedEventHandler<T> VectorChanged
{
add
{
lock (_vectorChangedHandlersLock)
{
(_vectorChangedHandlers ??= new()).Add(value);
}
}

remove
{
lock (_vectorChangedHandlersLock)
{
var list = _vectorChangedHandlers ??= new();

var lastIndex = list.LastIndexOf(value);

if (lastIndex != -1)
{
list.RemoveAt(lastIndex);
}
}
}
}

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

Expand Down Expand Up @@ -205,7 +236,63 @@ internal List<T>.Enumerator GetEnumeratorFast()
=> _list.GetEnumerator();

private void RaiseVectorChanged(CollectionChange change, int index)
=> VectorChanged?.Invoke(this, new VectorChangedEventArgs(change, (uint)index));
{
// Gets an executable list that does not need to be locked
int GetInvocationList(out VectorChangedEventHandler<T> single, out VectorChangedEventHandler<T>[] array)
{
lock (_vectorChangedHandlersLock)
{
if (_vectorChangedHandlers is { Count: > 0 })
{
if (_vectorChangedHandlers.Count == 1)
{
single = _vectorChangedHandlers[0];
array = null;
return 1;
}
else
{
single = null;

array = ArrayPool<VectorChangedEventHandler<T>>.Shared.Rent(_vectorChangedHandlers.Count);
_vectorChangedHandlers.CopyTo(array, 0);

return _vectorChangedHandlers.Count;
}
}
}

single = null;
array = null;
return 0;
}

var count = GetInvocationList(out var single, out var array);

if (count > 0)
{
var args = new VectorChangedEventArgs(change, (uint)index);

if (count == 1)
{
single.Invoke(this, args);
}
else
{
for (int i = 0; i < count; i++)
{
ref var handler = ref array[i];
handler.Invoke(this, args);

// Clear the handle immediately, so we don't
// call ArrayPool.Return with clear.
handler = null;
}

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

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

0 comments on commit 5b90f03

Please sign in to comment.