diff --git a/CalculatedProperties/CalculatedProperty.cs b/CalculatedProperties/CalculatedProperty.cs index 005088b..bf7004e 100644 --- a/CalculatedProperties/CalculatedProperty.cs +++ b/CalculatedProperties/CalculatedProperty.cs @@ -121,7 +121,7 @@ public DebugView(CalculatedProperty property) public HashSet Sources { get { return _property._sources; } } - public HashSet Targets { get { return _base.Targets; } } + public List Targets { get { return _base.Targets; } } public bool ListeningToCollectionChanged { get { return _property._collectionChangedHandler != null; } } } diff --git a/CalculatedProperties/Internal/SourcePropertyBase.cs b/CalculatedProperties/Internal/SourcePropertyBase.cs index 2bdf390..ce8973a 100644 --- a/CalculatedProperties/Internal/SourcePropertyBase.cs +++ b/CalculatedProperties/Internal/SourcePropertyBase.cs @@ -2,6 +2,8 @@ using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; +using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; namespace CalculatedProperties.Internal @@ -13,7 +15,8 @@ public abstract class SourcePropertyBase : ISourceProperty { private readonly int _threadId; private readonly Action _onPropertyChanged; - private readonly HashSet _targets; + private readonly ConditionalWeakTable _targetsLookup; + private readonly List _targetsList; private PropertyChangedEventArgs _args; private string _propertyName; @@ -25,7 +28,8 @@ protected SourcePropertyBase(Action onPropertyChanged) { _threadId = Thread.CurrentThread.ManagedThreadId; _onPropertyChanged = onPropertyChanged; - _targets = new HashSet(); + _targetsLookup = new ConditionalWeakTable(); + _targetsList = new List(); } /// @@ -73,12 +77,15 @@ public virtual void Invalidate() // Queue OnNotifyPropertyChanged. (PropertyChangedNotificationManager.Instance as IPropertyChangedNotificationManager).Register(this); - // Invalidate all targets. - foreach (var target in _targets) - target.Invalidate(); + InvalidateAllTargets(); } } + private void InvalidateAllTargets() + { + foreach (ITargetProperty target in GetAliveTargets()) + target.Invalidate(); + } /// /// Invalidates this property and the transitive closure of all its target properties. If notifications are not deferred, then this method will raise for all affected properties before returning. is not raised for this property. /// @@ -87,20 +94,22 @@ public virtual void InvalidateTargets() // Ensure notifications are deferred. using (PropertyChangedNotificationManager.Instance.DeferNotifications()) { - // Invalidate all targets. - foreach (var target in _targets) - target.Invalidate(); + InvalidateAllTargets(); } } void ISourceProperty.AddTarget(ITargetProperty targetProperty) { - _targets.Add(targetProperty); + if (!ContainsTarget(targetProperty)) + { + _targetsLookup.Add(targetProperty, targetProperty); + _targetsList.Add(new WeakReference(targetProperty)); + } } void ISourceProperty.RemoveTarget(ITargetProperty targetProperty) { - _targets.Remove(targetProperty); + _targetsLookup.Remove(targetProperty); } /// @@ -127,7 +136,19 @@ public DebugView(SourcePropertyBase property) /// /// Gets the target properties. /// - public HashSet Targets { get { return _property._targets; } } + public List Targets { get { return _property.GetAliveTargets(); } } + } + + private List GetAliveTargets() + { + _targetsList.RemoveAll(@ref => !ContainsTarget((ITargetProperty) @ref.Target)); + return _targetsList.Select(@ref => (ITargetProperty) @ref.Target).Where(t => t != null).ToList(); + } + + private bool ContainsTarget(ITargetProperty target) + { + ITargetProperty cachedTarget; + return target != null && _targetsLookup.TryGetValue(target, out cachedTarget); } } } diff --git a/CalculatedProperties/Properties/AssemblyInfo.cs b/CalculatedProperties/Properties/AssemblyInfo.cs index 57add62..6ba75dd 100644 --- a/CalculatedProperties/Properties/AssemblyInfo.cs +++ b/CalculatedProperties/Properties/AssemblyInfo.cs @@ -4,5 +4,5 @@ [assembly: AssemblyCompany("Stephen Cleary")] [assembly: AssemblyDescription("Easy-to-use calculated properties for MVVM apps (.NET 4, MonoTouch, MonoDroid, Windows 8, Windows Phone 8.1, Windows Phone Silverlight 8.0, and Silverlight 5).")] -[assembly: AssemblyVersion("1.0.0")] -[assembly: AssemblyInformationalVersion("1.0.0")] \ No newline at end of file +[assembly: AssemblyVersion("1.0.1")] +[assembly: AssemblyInformationalVersion("1.0.1")] \ No newline at end of file diff --git a/CalculatedProperties/TriggerProperty.cs b/CalculatedProperties/TriggerProperty.cs index f7d9bdf..5f6c4f3 100644 --- a/CalculatedProperties/TriggerProperty.cs +++ b/CalculatedProperties/TriggerProperty.cs @@ -102,7 +102,7 @@ public DebugView(TriggerProperty property) public T Value { get { return _property._value; } } - public HashSet Targets { get { return _base.Targets; } } + public List Targets { get { return _base.Targets; } } public bool ListeningToCollectionChanged { get { return _property._collectionChangedHandler != null; } } } diff --git a/Unit Tests/MemoryLeakUnitTests.cs b/Unit Tests/MemoryLeakUnitTests.cs new file mode 100644 index 0000000..6bd1c9c --- /dev/null +++ b/Unit Tests/MemoryLeakUnitTests.cs @@ -0,0 +1,87 @@ +using System; +using System.Collections.Generic; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Unit_Tests +{ + [TestClass] + public class MemoryLeakUnitTests + { + [TestMethod] + public void LongLivingPublisherAllowsToGarbageCollectShortLivingSubscribers() + { + FullGarbageCollection(); + + var longLivingVm = new LongLivingVm(); + + var notifications = new List(); + Action transientScopeAction = () => + { + var transientObject = new ShortLivingViewModel(longLivingVm); + transientObject.PropertyChanged += (sender, args) => notifications.Add(args.PropertyName); + // emulate read from UI + var tmp = transientObject.FullName; + Assert.IsTrue(notifications.Count == 0); + + longLivingVm.FirstName = "Mister"; + Assert.IsTrue(notifications.Count == 1 && notifications[0] == nameof(ShortLivingViewModel.FullName)); + + // we have one instance of short living object + Assert.IsTrue(ShortLivingViewModel.InstanceCount == 1); + // scope finished: eligible for GC + }; + transientScopeAction(); + + FullGarbageCollection(); + + notifications.Clear(); + longLivingVm.FirstName = "Twister"; + + // transient listener has been GCed and didn't receive any notifications + Assert.IsTrue(notifications.Count == 0); + Assert.IsTrue(ShortLivingViewModel.InstanceCount == 0); + } + + private static void FullGarbageCollection() + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + } + } + + class ShortLivingViewModel : ViewModelBase + { + public static int InstanceCount = 0; + + ~ShortLivingViewModel() + { + InstanceCount--; + } + + public ShortLivingViewModel(LongLivingVm longLivingVm) + { + InstanceCount++; + _longLivingVm = longLivingVm; + } + + private readonly LongLivingVm _longLivingVm; + + public string FullName => Properties.Calculated(() => $"{_longLivingVm.FirstName} {_longLivingVm.LastName}"); + } + + class LongLivingVm : ViewModelBase + { + public string FirstName + { + get { return Properties.Get((string)null); } + set { Properties.Set(value); } + } + + public string LastName + { + get { return Properties.Get((string)null); } + set { Properties.Set(value); } + } + } +} diff --git a/Unit Tests/PerformanceUnitTests.cs b/Unit Tests/PerformanceUnitTests.cs new file mode 100644 index 0000000..35c4550 --- /dev/null +++ b/Unit Tests/PerformanceUnitTests.cs @@ -0,0 +1,101 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Unit_Tests +{ + [TestClass] + public class PerformanceUnitTests + { + [TestMethod] + public void TriggerPropertyWithThousandsOfTargetsHasAcceptableRewiringTime() + { + SourceViewModel source = new SourceViewModel(); + + CreateAndRewireManyTargets(source); + + // GC can collect all targets which went out of scope + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + Assert.IsTrue(TargetViewModel.InstanceCount == 0); + } + + private static void CreateAndRewireManyTargets(SourceViewModel source) + { + const int size = 50000; + + // populate source.BaseValue collection of targets is populated + Stopwatch sw = new Stopwatch(); + sw.Start(); + + TargetViewModel[] manyTargets = new TargetViewModel[2 * size]; + for (int i = 0; i < size; i++) + { + TargetViewModel target = new TargetViewModel {Source = source}; + var tmp = target.DerivedValue; + manyTargets[i] = target; + } + sw.Stop(); + Console.WriteLine("Initial wiring time: " + sw.Elapsed); + + Assert.IsTrue(TargetViewModel.InstanceCount == size); + + // emulate massive rewiring: adding new targets and resetting old + sw.Restart(); + + for (int i = 0; i < size; i++) + { + // unplug a target + manyTargets[i].Source = null; + var tmp = manyTargets[i].DerivedValue; + + // plug a new target and keep it alive in array + TargetViewModel target = new TargetViewModel {Source = source}; + manyTargets[i + size] = target; + tmp = target.DerivedValue; + } + sw.Stop(); + Console.WriteLine("Rewiring time: " + sw.Elapsed); + + // rewiring should take under a second on fast dev machine but let's say two seconds + Assert.IsTrue(sw.Elapsed.TotalSeconds < 2); + } + + class SourceViewModel : ViewModelBase + { + public int BaseValue + { + get { return Properties.Get(0); } + set { Properties.Set(value); } + } + } + + class TargetViewModel : ViewModelBase + { + public static int InstanceCount = 0; + + ~TargetViewModel() + { + Interlocked.Decrement(ref InstanceCount); + } + + public TargetViewModel() + { + Interlocked.Increment(ref InstanceCount); + } + + public SourceViewModel Source + { + get { return Properties.Get((SourceViewModel)null); } + set { Properties.Set(value); } + } + + public int DerivedValue => Properties.Calculated(() => Source?.BaseValue ?? 0 + 5); + } + } +} diff --git a/Unit Tests/Unit Tests.csproj b/Unit Tests/Unit Tests.csproj index bd7a114..fc13572 100644 --- a/Unit Tests/Unit Tests.csproj +++ b/Unit Tests/Unit Tests.csproj @@ -53,6 +53,8 @@ + +