Skip to content

Commit

Permalink
chore: Adjust remove order, don't lock on execution, clear pooled arr…
Browse files Browse the repository at this point in the history
…ay memory
  • Loading branch information
jeromelaban committed Nov 26, 2024
1 parent ef926a2 commit 263a278
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 19 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"]));
}
}
72 changes: 53 additions & 19 deletions src/Uno.UI/UI/Xaml/DependencyObjectCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@ public event VectorChangedEventHandler<T> VectorChanged
{
lock (_vectorChangedHandlersLock)
{
(_vectorChangedHandlers ??= new()).Remove(value);
var list = _vectorChangedHandlers ??= new();

var lastIndex = list.LastIndexOf(value);

if (lastIndex != -1)
{
list.RemoveAt(lastIndex);
}
}
}
}
Expand Down Expand Up @@ -229,32 +236,59 @@ internal List<T>.Enumerator GetEnumeratorFast()

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

if (_vectorChangedHandlers.Count == 1)
if (_vectorChangedHandlers is { Count: > 0 })
{
_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);
if (_vectorChangedHandlers.Count == 1)
{
single = _vectorChangedHandlers[0];
array = null;
return 1;
}
else
{
single = null;

// Use the original count, the rented array may be larger.
var count = _vectorChangedHandlers.Count;
array = ArrayPool<VectorChangedEventHandler<T>>.Shared.Rent(_vectorChangedHandlers.Count);
_vectorChangedHandlers.CopyTo(array, 0);

for (int i = 0; i < count; i++)
{
handlersClone[i].Invoke(this, args);
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);

ArrayPool<VectorChangedEventHandler<T>>.Shared.Return(handlersClone);
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);
}
}
}
Expand Down

0 comments on commit 263a278

Please sign in to comment.