From fa9e7fbcd8df29ea9edc57442172ec38154fb0e5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 10 Mar 2016 01:44:44 +0100 Subject: [PATCH] Don't use observables to detach styles. Rather than adding a TakeUntil(control.StyleDetach) to each applied style, keep track of the applied styles in a static list. This dramatically reduces memory usage. --- src/Perspex.Base/Collections/PerspexList.cs | 2 - src/Perspex.Base/Perspex.Base.csproj | 1 + src/Perspex.Base/Reactive/ObservableEx.cs | 41 +++++++++++++++++ src/Perspex.Controls/Control.cs | 6 +-- src/Perspex.Styling/Styling/ISetter.cs | 2 +- src/Perspex.Styling/Styling/IStyleable.cs | 5 +- src/Perspex.Styling/Styling/Setter.cs | 15 +++--- src/Perspex.Styling/Styling/Style.cs | 46 +++++++++++++++++-- .../Perspex.Base.UnitTests.csproj | 4 ++ .../PerspexPropertyConverterTest.cs | 2 +- .../ActivatedValueTests.cs | 19 ++++++++ .../SelectorTests_Child.cs | 2 +- .../SelectorTests_Descendent.cs | 2 +- .../TestControlBase.cs | 2 +- .../TestTemplatedControl.cs | 2 +- 15 files changed, 124 insertions(+), 27 deletions(-) create mode 100644 src/Perspex.Base/Reactive/ObservableEx.cs diff --git a/src/Perspex.Base/Collections/PerspexList.cs b/src/Perspex.Base/Collections/PerspexList.cs index 82f654a4e0..17f40f2056 100644 --- a/src/Perspex.Base/Collections/PerspexList.cs +++ b/src/Perspex.Base/Collections/PerspexList.cs @@ -175,8 +175,6 @@ namespace Perspex.Collections set { this[index] = (T)value; } } - public int SubscriberCount => _collectionChanged?.GetInvocationList().Length ?? 0; - /// /// Adds an item to the collection. /// diff --git a/src/Perspex.Base/Perspex.Base.csproj b/src/Perspex.Base/Perspex.Base.csproj index 7420fef3af..970a8f2b99 100644 --- a/src/Perspex.Base/Perspex.Base.csproj +++ b/src/Perspex.Base/Perspex.Base.csproj @@ -69,6 +69,7 @@ + diff --git a/src/Perspex.Base/Reactive/ObservableEx.cs b/src/Perspex.Base/Reactive/ObservableEx.cs new file mode 100644 index 0000000000..b808543904 --- /dev/null +++ b/src/Perspex.Base/Reactive/ObservableEx.cs @@ -0,0 +1,41 @@ +// Copyright (c) The Perspex Project. All rights reserved. +// Licensed under the MIT license. See licence.md file in the project root for full license information. + +using System; +using System.Reactive.Disposables; + +namespace Perspex.Reactive +{ + /// + /// Provides common observable methods not found in standard Rx framework. + /// + public static class ObservableEx + { + /// + /// Returns an observable that fires once with the specified value and never completes. + /// + /// The type of the value. + /// The value. + /// The observable. + public static IObservable SingleValue(T value) + { + return new SingleValueImpl(value); + } + + private class SingleValueImpl : IObservable + { + private T _value; + + public SingleValueImpl(T value) + { + _value = value; + } + + public IDisposable Subscribe(IObserver observer) + { + observer.OnNext(_value); + return Disposable.Empty; + } + } + } +} diff --git a/src/Perspex.Controls/Control.cs b/src/Perspex.Controls/Control.cs index 460b0a5c8a..66fee6f60b 100644 --- a/src/Perspex.Controls/Control.cs +++ b/src/Perspex.Controls/Control.cs @@ -96,7 +96,7 @@ namespace Perspex.Controls private INameScope _nameScope; private Styles _styles; private bool _styled; - private Subject _styleDetach = new Subject(); + private Subject _styleDetach = new Subject(); /// /// Initializes static members of the class. @@ -308,7 +308,7 @@ namespace Perspex.Controls Type IStyleable.StyleKey => GetType(); /// - IObservable IStyleable.StyleDetach => _styleDetach; + IObservable IStyleable.StyleDetach => _styleDetach; /// IStyleHost IStyleHost.StylingParent => (IStyleHost)InheritanceParent; @@ -534,7 +534,7 @@ namespace Perspex.Controls } _isAttachedToLogicalTree = false; - _styleDetach.OnNext(Unit.Default); + _styleDetach.OnNext(this); this.TemplatedParent = null; DetachedFromLogicalTree?.Invoke(this, e); diff --git a/src/Perspex.Styling/Styling/ISetter.cs b/src/Perspex.Styling/Styling/ISetter.cs index 105a8ec6ac..8e5db104de 100644 --- a/src/Perspex.Styling/Styling/ISetter.cs +++ b/src/Perspex.Styling/Styling/ISetter.cs @@ -16,6 +16,6 @@ namespace Perspex.Styling /// The style that is being applied. /// The control. /// An optional activator. - void Apply(IStyle style, IStyleable control, IObservable activator); + IDisposable Apply(IStyle style, IStyleable control, IObservable activator); } } \ No newline at end of file diff --git a/src/Perspex.Styling/Styling/IStyleable.cs b/src/Perspex.Styling/Styling/IStyleable.cs index 36b092ff1d..aa1e749dad 100644 --- a/src/Perspex.Styling/Styling/IStyleable.cs +++ b/src/Perspex.Styling/Styling/IStyleable.cs @@ -3,7 +3,6 @@ using System; using Perspex.Collections; -using System.Reactive; namespace Perspex.Styling { @@ -13,9 +12,9 @@ namespace Perspex.Styling public interface IStyleable : IPerspexObject, INamed { /// - /// Raised when the control's style should be removed. + /// Signalled when the control's style should be removed. /// - IObservable StyleDetach { get; } + IObservable StyleDetach { get; } /// /// Gets the list of classes for the control. diff --git a/src/Perspex.Styling/Styling/Setter.cs b/src/Perspex.Styling/Styling/Setter.cs index 3c4592fa81..23b5aa7d7e 100644 --- a/src/Perspex.Styling/Styling/Setter.cs +++ b/src/Perspex.Styling/Styling/Setter.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; +using System.Reactive.Disposables; using System.Reactive.Subjects; using Perspex.Data; using Perspex.Metadata; @@ -63,7 +64,7 @@ namespace Perspex.Styling /// The style that is being applied. /// The control. /// An optional activator. - public void Apply(IStyle style, IStyleable control, IObservable activator) + public IDisposable Apply(IStyle style, IStyleable control, IObservable activator) { Contract.Requires(control != null); @@ -80,16 +81,12 @@ namespace Perspex.Styling { if (activator == null) { - control.SetValue(Property, Value, BindingPriority.Style); + return control.Bind(Property, ObservableEx.SingleValue(Value), BindingPriority.Style); } else { var activated = new ActivatedValue(activator, Value, description); - var instanced = new InstancedBinding( - activated, - BindingMode.OneWay, - BindingPriority.StyleTrigger); - BindingOperations.Apply(control, Property, instanced, null); + return control.Bind(Property, activated, BindingPriority.StyleTrigger); } } else @@ -99,9 +96,11 @@ namespace Perspex.Styling if (source != null) { var cloned = Clone(source, style, activator); - BindingOperations.Apply(control, Property, cloned, null); + return BindingOperations.Apply(control, Property, cloned, null); } } + + return Disposable.Empty; } private InstancedBinding Clone(InstancedBinding sourceInstance, IStyle style, IObservable activator) diff --git a/src/Perspex.Styling/Styling/Style.cs b/src/Perspex.Styling/Styling/Style.cs index 7f0cb00e4b..73e5c19467 100644 --- a/src/Perspex.Styling/Styling/Style.cs +++ b/src/Perspex.Styling/Styling/Style.cs @@ -13,7 +13,9 @@ namespace Perspex.Styling /// public class Style : IStyle { - private static readonly IObservable True = Observable.Never().StartWith(true); + private static Dictionary> _applied = + new Dictionary>(); + private Dictionary _resources; /// @@ -85,20 +87,23 @@ namespace Perspex.Styling if (match.ImmediateResult != false) { - var activator = (match.ObservableResult ?? True) - .TakeUntil(control.StyleDetach); + var subs = GetSubscriptions(control); foreach (var setter in Setters) { - setter.Apply(this, control, activator); + var sub = setter.Apply(this, control, match.ObservableResult); + subs.Add(sub); } } } else if (control == container) { + var subs = GetSubscriptions(control); + foreach (var setter in Setters) { - setter.Apply(this, control, null); + var sub = setter.Apply(this, control, null); + subs.Add(sub); } } } @@ -139,5 +144,36 @@ namespace Perspex.Styling return "Style"; } } + + private static List GetSubscriptions(IStyleable control) + { + List subscriptions; + + if (!_applied.TryGetValue(control, out subscriptions)) + { + subscriptions = new List(2); + subscriptions.Add(control.StyleDetach.Subscribe(ControlDetach)); + _applied.Add(control, subscriptions); + } + + return subscriptions; + } + + /// + /// Called when a control's is signalled to remove + /// all applied styles. + /// + /// The control. + private static void ControlDetach(IStyleable control) + { + var subscriptions = _applied[control]; + + foreach (var subscription in subscriptions) + { + subscription.Dispose(); + } + + _applied.Remove(control); + } } } diff --git a/tests/Perspex.Base.UnitTests/Perspex.Base.UnitTests.csproj b/tests/Perspex.Base.UnitTests/Perspex.Base.UnitTests.csproj index e30e84d576..da3d2aa9cc 100644 --- a/tests/Perspex.Base.UnitTests/Perspex.Base.UnitTests.csproj +++ b/tests/Perspex.Base.UnitTests/Perspex.Base.UnitTests.csproj @@ -69,6 +69,10 @@ ..\..\packages\Rx-PlatformServices.2.2.5\lib\net45\System.Reactive.PlatformServices.dll True + + ..\..\packages\Rx-PlatformServices.2.2.5\lib\net45\System.Reactive.PlatformServices.dll + True + ..\..\packages\xunit.abstractions.2.0.0\lib\net35\xunit.abstractions.dll diff --git a/tests/Perspex.Markup.Xaml.UnitTests/Converters/PerspexPropertyConverterTest.cs b/tests/Perspex.Markup.Xaml.UnitTests/Converters/PerspexPropertyConverterTest.cs index b25542de0c..a6314f14e1 100644 --- a/tests/Perspex.Markup.Xaml.UnitTests/Converters/PerspexPropertyConverterTest.cs +++ b/tests/Perspex.Markup.Xaml.UnitTests/Converters/PerspexPropertyConverterTest.cs @@ -97,7 +97,7 @@ namespace Perspex.Markup.Xaml.UnitTests.Converters get { throw new NotImplementedException(); } } - IObservable IStyleable.StyleDetach { get; } + IObservable IStyleable.StyleDetach { get; } } private class AttachedOwner diff --git a/tests/Perspex.Styling.UnitTests/ActivatedValueTests.cs b/tests/Perspex.Styling.UnitTests/ActivatedValueTests.cs index 4921aeaa7b..ca31d1f86f 100644 --- a/tests/Perspex.Styling.UnitTests/ActivatedValueTests.cs +++ b/tests/Perspex.Styling.UnitTests/ActivatedValueTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Reactive.Linq; using System.Reactive.Subjects; +using Microsoft.Reactive.Testing; using Xunit; namespace Perspex.Styling.UnitTests @@ -38,5 +39,23 @@ namespace Perspex.Styling.UnitTests Assert.True(completed); } + + [Fact] + public void Should_Unsubscribe_From_Activator_When_All_Subscriptions_Disposed() + { + var scheduler = new TestScheduler(); + var activator1 = scheduler.CreateColdObservable(); + var activator2 = scheduler.CreateColdObservable(); + var activator = StyleActivator.And(new[] { activator1, activator2 }); + var target = new ActivatedValue(activator, 1, string.Empty); + + var subscription = target.Subscribe(_ => { }); + Assert.Equal(1, activator1.Subscriptions.Count); + Assert.Equal(Subscription.Infinite, activator1.Subscriptions[0].Unsubscribe); + + subscription.Dispose(); + Assert.Equal(1, activator1.Subscriptions.Count); + Assert.Equal(0, activator1.Subscriptions[0].Unsubscribe); + } } } diff --git a/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs b/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs index 55eff92311..465610b045 100644 --- a/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs +++ b/tests/Perspex.Styling.UnitTests/SelectorTests_Child.cs @@ -105,7 +105,7 @@ namespace Perspex.Styling.UnitTests public ITemplatedControl TemplatedParent { get; } - IObservable IStyleable.StyleDetach { get; } + IObservable IStyleable.StyleDetach { get; } IPerspexReadOnlyList IStyleable.Classes => Classes; diff --git a/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs b/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs index 07bb16ee14..286a5ec269 100644 --- a/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs +++ b/tests/Perspex.Styling.UnitTests/SelectorTests_Descendent.cs @@ -138,7 +138,7 @@ namespace Perspex.Styling.UnitTests IPerspexReadOnlyList IStyleable.Classes => Classes; - IObservable IStyleable.StyleDetach { get; } + IObservable IStyleable.StyleDetach { get; } public object GetValue(PerspexProperty property) { diff --git a/tests/Perspex.Styling.UnitTests/TestControlBase.cs b/tests/Perspex.Styling.UnitTests/TestControlBase.cs index 3a0f0e5434..d5bd85f1dd 100644 --- a/tests/Perspex.Styling.UnitTests/TestControlBase.cs +++ b/tests/Perspex.Styling.UnitTests/TestControlBase.cs @@ -35,7 +35,7 @@ namespace Perspex.Styling.UnitTests IPerspexReadOnlyList IStyleable.Classes => Classes; - IObservable IStyleable.StyleDetach { get; } + IObservable IStyleable.StyleDetach { get; } public object GetValue(PerspexProperty property) { diff --git a/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs b/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs index 847a124066..3e16e8a18c 100644 --- a/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs +++ b/tests/Perspex.Styling.UnitTests/TestTemplatedControl.cs @@ -35,7 +35,7 @@ namespace Perspex.Styling.UnitTests IPerspexReadOnlyList IStyleable.Classes => Classes; - IObservable IStyleable.StyleDetach { get; } + IObservable IStyleable.StyleDetach { get; } public object GetValue(PerspexProperty property) {