From d3a0507f35c6951daa1cc06c4fe1c9f4e7664af5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 01:07:14 +0100 Subject: [PATCH 01/19] Implement custom observable for classes. Use a custom `ClassObserver` for observing changes to a control's classes. This saves shaves off about 15% memory use in ControlCatalog after cyling through all tabs. --- .../Styling/TypeNameAndClassSelector.cs | 70 ++++++++++++++++--- 1 file changed, 62 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs index fb32913e7e..5d48496e8c 100644 --- a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs @@ -5,9 +5,11 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; using System.Reactive; +using System.Reactive.Disposables; using System.Reactive.Linq; using System.Reflection; using System.Text; +using Avalonia.Collections; namespace Avalonia.Styling { @@ -122,14 +124,7 @@ namespace Avalonia.Styling { if (subscribe) { - var observable = Observable.FromEventPattern< - NotifyCollectionChangedEventHandler, - NotifyCollectionChangedEventArgs>( - x => control.Classes.CollectionChanged += x, - x => control.Classes.CollectionChanged -= x) - .StartWith((EventPattern)null) - .Select(_ => Matches(control.Classes)) - .DistinctUntilChanged(); + var observable = new ClassObserver(control.Classes, _classes.Value); return new SelectorMatch(observable); } else @@ -204,5 +199,64 @@ namespace Avalonia.Styling return builder.ToString(); } + + private class ClassObserver : IObservable + { + readonly IList _match; + readonly List> _observers = new List>(); + IAvaloniaReadOnlyList _classes; + + public ClassObserver(IAvaloniaReadOnlyList classes, IList match) + { + _classes = classes; + _match = match; + } + + public IDisposable Subscribe(IObserver observer) + { + if (_observers.Count == 0) + { + _classes.CollectionChanged += ClassesChanged; + } + + _observers.Add(observer); + observer.OnNext(GetResult()); + + return Disposable.Create(() => + { + _observers.Remove(observer); + if (_observers.Count == 0) + { + _classes.CollectionChanged -= ClassesChanged; + } + }); + } + + private void ClassesChanged(object sender, NotifyCollectionChangedEventArgs e) + { + if (e.Action != NotifyCollectionChangedAction.Move) + { + foreach (var observer in _observers) + { + observer.OnNext(GetResult()); + } + } + } + + private bool GetResult() + { + int remaining = _match.Count; + + foreach (var c in _classes) + { + if (_match.Contains(c)) + { + --remaining; + } + } + + return remaining == 0; + } + } } } From 6d3381dd6e8992f7dd39b4c3f439f98f531775e5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 13:56:45 +0100 Subject: [PATCH 02/19] Make ActivatedObservable a custom observable. --- .../Styling/ActivatedObservable.cs | 123 +++++++++++++++--- 1 file changed, 102 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 2cd324fff4..1d9af6ed41 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -2,8 +2,8 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive; -using System.Reactive.Linq; +using System.Collections.Generic; +using System.Reactive.Disposables; namespace Avalonia.Styling { @@ -17,8 +17,17 @@ namespace Avalonia.Styling /// value. When the activator produces false it will produce /// . /// - internal class ActivatedObservable : ObservableBase, IDescription + internal class ActivatedObservable : IObservable, IDescription { + private static readonly object NotSent = new object(); + private readonly Listener _listener; + private List> _observers; + private IDisposable _activatorSubscription; + private IDisposable _sourceSubscription; + private bool? _active; + private object _value; + private object _last = NotSent; + /// /// Initializes a new instance of the class. /// @@ -36,6 +45,7 @@ namespace Avalonia.Styling Activator = activator; Description = description; Source = source; + _listener = new Listener(this); } /// @@ -53,25 +63,96 @@ namespace Avalonia.Styling /// public IObservable Source { get; } - /// - /// Notifies the provider that an observer is to receive notifications. - /// - /// The observer. - /// IDisposable object used to unsubscribe from the observable sequence. - protected override IDisposable SubscribeCore(IObserver observer) + public IDisposable Subscribe(IObserver observer) + { + var subscribe = _observers == null; + + _observers = _observers ?? new List>(); + _observers.Add(observer); + + if (subscribe) + { + _sourceSubscription = Source.Subscribe(_listener); + _activatorSubscription = Activator.Subscribe(_listener); + } + + return Disposable.Create(() => + { + _observers.Remove(observer); + + if (_observers.Count == 0) + { + _activatorSubscription.Dispose(); + _sourceSubscription.Dispose(); + _activatorSubscription = null; + _sourceSubscription = null; + } + }); + } + + private void NotifyCompleted() + { + foreach (var observer in _observers) + { + observer.OnCompleted(); + } + + _observers = null; + } + + private void NotifyError(Exception error) { - Contract.Requires(observer != null); - - var sourceCompleted = Source.LastOrDefaultAsync().Select(_ => Unit.Default); - var activatorCompleted = Activator.LastOrDefaultAsync().Select(_ => Unit.Default); - var completed = sourceCompleted.Merge(activatorCompleted); - - return Activator - .CombineLatest(Source, (x, y) => new { Active = x, Value = y }) - .Select(x => x.Active ? x.Value : AvaloniaProperty.UnsetValue) - .DistinctUntilChanged() - .TakeUntil(completed) - .Subscribe(observer); + foreach (var observer in _observers) + { + observer.OnError(error); + } + + _observers = null; + } + + private void Update() + { + if (_active.HasValue) + { + var v = _active.Value ? _value : AvaloniaProperty.UnsetValue; + + if (!Equals(v, _last)) + { + foreach (var observer in _observers) + { + observer.OnNext(v); + } + + _last = v; + } + } + } + + private class Listener : IObserver, IObserver + { + private readonly ActivatedObservable _parent; + + public Listener(ActivatedObservable parent) + { + _parent = parent; + } + + void IObserver.OnCompleted() => _parent.NotifyCompleted(); + void IObserver.OnCompleted() => _parent.NotifyCompleted(); + void IObserver.OnError(Exception error) => _parent.NotifyError(error); + void IObserver.OnError(Exception error) => _parent.NotifyError(error); + + void IObserver.OnNext(bool value) + { + _parent._active = value; + _parent.Update(); + } + + void IObserver.OnNext(object value) + { + _parent._value = value; + _parent.Update(); + } } } } From de35eb9fee049179e41076de2fc441454e415959 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 15:56:22 +0100 Subject: [PATCH 03/19] Updated ActivatedSubject. Can be expressed more cleanly given the new `ActivatedObject`. --- .../Styling/ActivatedObservable.cs | 46 +++++++++------ .../Styling/ActivatedSubject.cs | 59 ++++++++++--------- .../ActivatedSubjectTests.cs | 10 +++- 3 files changed, 66 insertions(+), 49 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 1d9af6ed41..3c58bb5f9d 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -24,8 +24,6 @@ namespace Avalonia.Styling private List> _observers; private IDisposable _activatorSubscription; private IDisposable _sourceSubscription; - private bool? _active; - private object _value; private object _last = NotSent; /// @@ -58,11 +56,21 @@ namespace Avalonia.Styling /// public string Description { get; } + /// + /// Gets a value indicating whether the observable is active. + /// + public bool? IsActive { get; private set; } + /// /// Gets an observable which produces the . /// public IObservable Source { get; } + /// + /// Gets the value that will be produced when is true. + /// + public object Value { get; private set; } + public IDisposable Subscribe(IObserver observer) { var subscribe = _observers == null; @@ -90,7 +98,7 @@ namespace Avalonia.Styling }); } - private void NotifyCompleted() + protected virtual void NotifyCompleted() { foreach (var observer in _observers) { @@ -100,7 +108,7 @@ namespace Avalonia.Styling _observers = null; } - private void NotifyError(Exception error) + protected virtual void NotifyError(Exception error) { foreach (var observer in _observers) { @@ -110,11 +118,23 @@ namespace Avalonia.Styling _observers = null; } + protected virtual void NotifyValue(object value) + { + Value = value; + Update(); + } + + protected virtual void NotifyActive(bool active) + { + IsActive = active; + Update(); + } + private void Update() { - if (_active.HasValue) + if (IsActive.HasValue) { - var v = _active.Value ? _value : AvaloniaProperty.UnsetValue; + var v = IsActive.Value ? Value : AvaloniaProperty.UnsetValue; if (!Equals(v, _last)) { @@ -141,18 +161,8 @@ namespace Avalonia.Styling void IObserver.OnCompleted() => _parent.NotifyCompleted(); void IObserver.OnError(Exception error) => _parent.NotifyError(error); void IObserver.OnError(Exception error) => _parent.NotifyError(error); - - void IObserver.OnNext(bool value) - { - _parent._active = value; - _parent.Update(); - } - - void IObserver.OnNext(object value) - { - _parent._value = value; - _parent.Update(); - } + void IObserver.OnNext(bool value) => _parent.NotifyActive(value); + void IObserver.OnNext(object value) => _parent.NotifyValue(value); } } } diff --git a/src/Avalonia.Styling/Styling/ActivatedSubject.cs b/src/Avalonia.Styling/Styling/ActivatedSubject.cs index 4c2e8dde85..bcbf30649a 100644 --- a/src/Avalonia.Styling/Styling/ActivatedSubject.cs +++ b/src/Avalonia.Styling/Styling/ActivatedSubject.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive.Linq; using System.Reactive.Subjects; namespace Avalonia.Styling @@ -19,9 +18,8 @@ namespace Avalonia.Styling /// internal class ActivatedSubject : ActivatedObservable, ISubject, IDescription { - private bool? _active; private bool _completed; - private object _value; + private object _pushValue; /// /// Initializes a new instance of the class. @@ -35,7 +33,6 @@ namespace Avalonia.Styling string description) : base(activator, source, description) { - Activator.Subscribe(ActivatorChanged, ActivatorError, ActivatorCompleted); } /// @@ -46,53 +43,57 @@ namespace Avalonia.Styling get { return (ISubject)base.Source; } } - /// - /// Notifies all subscribed observers about the end of the sequence. - /// public void OnCompleted() { - if (_active.Value && !_completed) + Source.OnCompleted(); + } + + public void OnError(Exception error) + { + Source.OnError(error); + } + + public void OnNext(object value) + { + _pushValue = value; + + if (IsActive == true && !_completed) { - Source.OnCompleted(); + Source.OnNext(_pushValue); } } - /// - /// Notifies all subscribed observers with the exception. - /// - /// The exception to send to all subscribed observers. - /// is null. - public void OnError(Exception error) + protected override void NotifyCompleted() { - if (_active.Value && !_completed) + base.NotifyCompleted(); + + if (!_completed) { - Source.OnError(error); + Source.OnCompleted(); + _completed = true; } } - /// - /// Notifies all subscribed observers with the value. - /// - /// The value to send to all subscribed observers. - public void OnNext(object value) + protected override void NotifyError(Exception error) { - _value = value; + base.NotifyError(error); - if (_active.Value && !_completed) + if (!_completed) { - Source.OnNext(value); + Source.OnError(error); + _completed = true; } } - private void ActivatorChanged(bool active) + protected override void NotifyActive(bool active) { - bool first = !_active.HasValue; + bool first = !IsActive.HasValue; - _active = active; + base.NotifyActive(active); if (!first) { - Source.OnNext(active ? _value : AvaloniaProperty.UnsetValue); + Source.OnNext(active ? _pushValue : AvaloniaProperty.UnsetValue); } } diff --git a/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs b/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs index cc42e3f840..03f91d97a1 100644 --- a/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs +++ b/tests/Avalonia.Styling.UnitTests/ActivatedSubjectTests.cs @@ -17,6 +17,7 @@ namespace Avalonia.Styling.UnitTests var source = new TestSubject(); var target = new ActivatedSubject(activator, source, string.Empty); + target.Subscribe(); target.OnNext("bar"); Assert.Equal(AvaloniaProperty.UnsetValue, source.Value); activator.OnNext(true); @@ -36,6 +37,7 @@ namespace Avalonia.Styling.UnitTests var source = new TestSubject(); var target = new ActivatedSubject(activator, source, string.Empty); + target.Subscribe(); activator.OnCompleted(); Assert.True(source.Completed); @@ -47,10 +49,14 @@ namespace Avalonia.Styling.UnitTests var activator = new BehaviorSubject(false); var source = new TestSubject(); var target = new ActivatedSubject(activator, source, string.Empty); + var targetError = default(Exception); + var error = new Exception(); - activator.OnError(new Exception()); + target.Subscribe(_ => { }, e => targetError = e); + activator.OnError(error); - Assert.NotNull(source.Error); + Assert.Same(error, source.Error); + Assert.Same(error, targetError); } private class TestSubject : ISubject From cdba9b856d93d597e8192e00236763acab4952ee Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 18:25:57 +0100 Subject: [PATCH 04/19] Make ActivatedValue a custom Observable. And make `ActivatedObservable` inherit from it. --- .../Styling/ActivatedObservable.cs | 125 ++------------- .../Styling/ActivatedValue.cs | 147 +++++++++++++++--- 2 files changed, 139 insertions(+), 133 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 3c58bb5f9d..4933daed31 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -2,8 +2,6 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections.Generic; -using System.Reactive.Disposables; namespace Avalonia.Styling { @@ -17,14 +15,9 @@ namespace Avalonia.Styling /// value. When the activator produces false it will produce /// . /// - internal class ActivatedObservable : IObservable, IDescription + internal class ActivatedObservable : ActivatedValue, IDescription { - private static readonly object NotSent = new object(); - private readonly Listener _listener; - private List> _observers; - private IDisposable _activatorSubscription; private IDisposable _sourceSubscription; - private object _last = NotSent; /// /// Initializes a new instance of the class. @@ -36,133 +29,49 @@ namespace Avalonia.Styling IObservable activator, IObservable source, string description) + : base(activator, AvaloniaProperty.UnsetValue, description) { - Contract.Requires(activator != null); Contract.Requires(source != null); - Activator = activator; - Description = description; Source = source; - _listener = new Listener(this); } - /// - /// Gets the activator observable. - /// - public IObservable Activator { get; } - - /// - /// Gets a description of the binding. - /// - public string Description { get; } - - /// - /// Gets a value indicating whether the observable is active. - /// - public bool? IsActive { get; private set; } - /// /// Gets an observable which produces the . /// public IObservable Source { get; } - /// - /// Gets the value that will be produced when is true. - /// - public object Value { get; private set; } - - public IDisposable Subscribe(IObserver observer) - { - var subscribe = _observers == null; - - _observers = _observers ?? new List>(); - _observers.Add(observer); - - if (subscribe) - { - _sourceSubscription = Source.Subscribe(_listener); - _activatorSubscription = Activator.Subscribe(_listener); - } - - return Disposable.Create(() => - { - _observers.Remove(observer); - - if (_observers.Count == 0) - { - _activatorSubscription.Dispose(); - _sourceSubscription.Dispose(); - _activatorSubscription = null; - _sourceSubscription = null; - } - }); - } + protected override ActivatorListener CreateListener() => new ValueListener(this); - protected virtual void NotifyCompleted() + protected override void Deinitialize() { - foreach (var observer in _observers) - { - observer.OnCompleted(); - } - - _observers = null; + base.Deinitialize(); + _sourceSubscription.Dispose(); + _sourceSubscription = null; } - protected virtual void NotifyError(Exception error) + protected override void Initialize() { - foreach (var observer in _observers) - { - observer.OnError(error); - } - - _observers = null; + base.Initialize(); + _sourceSubscription = Source.Subscribe((ValueListener)Listener); } protected virtual void NotifyValue(object value) { Value = value; - Update(); } - protected virtual void NotifyActive(bool active) + private class ValueListener : ActivatorListener, IObserver { - IsActive = active; - Update(); - } - - private void Update() - { - if (IsActive.HasValue) - { - var v = IsActive.Value ? Value : AvaloniaProperty.UnsetValue; - - if (!Equals(v, _last)) - { - foreach (var observer in _observers) - { - observer.OnNext(v); - } - - _last = v; - } - } - } - - private class Listener : IObserver, IObserver - { - private readonly ActivatedObservable _parent; - - public Listener(ActivatedObservable parent) + public ValueListener(ActivatedObservable parent) + : base(parent) { - _parent = parent; } + protected new ActivatedObservable Parent => (ActivatedObservable)base.Parent; - void IObserver.OnCompleted() => _parent.NotifyCompleted(); - void IObserver.OnCompleted() => _parent.NotifyCompleted(); - void IObserver.OnError(Exception error) => _parent.NotifyError(error); - void IObserver.OnError(Exception error) => _parent.NotifyError(error); - void IObserver.OnNext(bool value) => _parent.NotifyActive(value); - void IObserver.OnNext(object value) => _parent.NotifyValue(value); + void IObserver.OnCompleted() => Parent.NotifyCompleted(); + void IObserver.OnError(Exception error) => Parent.NotifyError(error); + void IObserver.OnNext(object value) => Parent.NotifyValue(value); } } } diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index 3b9324c059..683121628c 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -2,8 +2,8 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Reactive; -using System.Reactive.Linq; +using System.Collections.Generic; +using System.Reactive.Disposables; namespace Avalonia.Styling { @@ -16,12 +16,13 @@ namespace Avalonia.Styling /// will produce the current value. When the activator /// produces false it will produce . /// - internal class ActivatedValue : ObservableBase, IDescription + internal class ActivatedValue : IObservable, IDescription { - /// - /// The activator. - /// - private readonly IObservable _activator; + private static readonly object NotSent = new object(); + private List> _observers = new List>(); + private IDisposable _activatorSubscription; + private object _value; + private object _last = NotSent; /// /// Initializes a new instance of the class. @@ -34,39 +35,135 @@ namespace Avalonia.Styling object value, string description) { - _activator = activator; + Contract.Requires(activator != null); + + Activator = activator; Value = value; Description = description; + Listener = CreateListener(); } /// - /// Gets the activated value. + /// Gets the activator observable. /// - public object Value - { - get; - } + public IObservable Activator { get; } /// /// Gets a description of the binding. /// - public string Description - { - get; - } + public string Description { get; } + + /// + /// Gets a value indicating whether the activator is active. + /// + public bool? IsActive { get; private set; } /// - /// Notifies the provider that an observer is to receive notifications. + /// Gets the value that will be produced when is true. /// - /// The observer. - /// IDisposable object used to unsubscribe from the observable sequence. - protected override IDisposable SubscribeCore(IObserver observer) + public object Value + { + get => _value; + protected set + { + _value = value; + PublishValue(); + } + } + + protected ActivatorListener Listener { get; } + + public virtual IDisposable Subscribe(IObserver observer) + { + _observers.Add(observer); + + if (_observers.Count == 1) + { + Initialize(); + } + + return Disposable.Create(() => + { + _observers.Remove(observer); + + if (_observers.Count == 0) + { + Deinitialize(); + } + }); + } + + protected virtual ActivatorListener CreateListener() => new ActivatorListener(this); + + protected virtual void Deinitialize() { - Contract.Requires(observer != null); + _activatorSubscription.Dispose(); + _activatorSubscription = null; + } + + protected virtual void Initialize() + { + _activatorSubscription = Activator.Subscribe(Listener); + } + + protected virtual void NotifyCompleted() + { + foreach (var observer in _observers) + { + observer.OnCompleted(); + } + + Deinitialize(); + _observers = null; + } + + protected virtual void NotifyError(Exception error) + { + foreach (var observer in _observers) + { + observer.OnError(error); + } + + Deinitialize(); + _observers = null; + } + + protected virtual void NotifyActive(bool active) + { + IsActive = active; + PublishValue(); + } + + private void PublishValue() + { + if (IsActive.HasValue) + { + var v = IsActive.Value ? Value : AvaloniaProperty.UnsetValue; + + if (!Equals(v, _last)) + { + foreach (var observer in _observers) + { + observer.OnNext(v); + } + + _last = v; + } + } + } + + protected class ActivatorListener : IObserver + { + public ActivatorListener(ActivatedValue parent) + { + Parent = parent; + } + + protected ActivatedValue Parent { get; } - return _activator - .Select(active => active ? Value : AvaloniaProperty.UnsetValue) - .Subscribe(observer); + void IObserver.OnCompleted() => Parent.NotifyCompleted(); + void IObserver.OnError(Exception error) => Parent.NotifyError(error); + void IObserver.OnNext(bool value) => Parent.NotifyActive(value); } } } From fcbde80682939636c7aa49da99018252dfa1b1c2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 14 Dec 2017 19:35:30 +0100 Subject: [PATCH 05/19] Added a couple of tests. --- src/Avalonia.Styling/Styling/ActivatedValue.cs | 4 ++++ .../ActivatedObservableTests.cs | 7 ++++--- .../ActivatedValueTests.cs | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index 683121628c..a68ae1e435 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Reactive.Disposables; +using Avalonia.Threading; namespace Avalonia.Styling { @@ -75,6 +76,9 @@ namespace Avalonia.Styling public virtual IDisposable Subscribe(IObserver observer) { + Contract.Requires(observer != null); + Dispatcher.UIThread.VerifyAccess(); + _observers.Add(observer); if (_observers.Count == 1) diff --git a/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs b/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs index d5d1d3b01c..7773d4767a 100644 --- a/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs +++ b/tests/Avalonia.Styling.UnitTests/ActivatedObservableTests.cs @@ -54,15 +54,16 @@ namespace Avalonia.Styling.UnitTests } [Fact] - public void Should_Complete_When_Activator_Completes() + public void Should_Error_When_Source_Errors() { var activator = new BehaviorSubject(false); var source = new BehaviorSubject(1); var target = new ActivatedObservable(activator, source, string.Empty); + var error = new Exception(); var completed = false; - target.Subscribe(_ => { }, () => completed = true); - activator.OnCompleted(); + target.Subscribe(_ => { }, x => completed = true); + source.OnError(error); Assert.True(completed); } diff --git a/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs b/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs index e2cc470a1e..92a7c1bd1f 100644 --- a/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs +++ b/tests/Avalonia.Styling.UnitTests/ActivatedValueTests.cs @@ -40,6 +40,20 @@ namespace Avalonia.Styling.UnitTests Assert.True(completed); } + [Fact] + public void Should_Error_When_Activator_Errors() + { + var activator = new BehaviorSubject(false); + var target = new ActivatedValue(activator, 1, string.Empty); + var error = new Exception(); + var completed = false; + + target.Subscribe(_ => { }, x => completed = true); + activator.OnError(error); + + Assert.True(completed); + } + [Fact] public void Should_Unsubscribe_From_Activator_When_All_Subscriptions_Disposed() { From 29eafc6be226ab7932fd5abe616b9f43ba48c4ff Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 20 Dec 2017 22:15:35 +0000 Subject: [PATCH 06/19] Added LightweightObservableBase class. And use it for `ActivatedValue`. --- .../Reactive/LightweightObservableBase.cs | 115 ++++++++++++++++++ .../Styling/ActivatedObservable.cs | 6 +- .../Styling/ActivatedSubject.cs | 36 +++--- .../Styling/ActivatedValue.cs | 76 +++--------- 4 files changed, 153 insertions(+), 80 deletions(-) create mode 100644 src/Avalonia.Base/Reactive/LightweightObservableBase.cs diff --git a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs new file mode 100644 index 0000000000..3ae6e5916b --- /dev/null +++ b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs @@ -0,0 +1,115 @@ +using System; +using System.Collections.Generic; +using System.Reactive; +using System.Reactive.Disposables; +using Avalonia.Threading; + +namespace Avalonia.Reactive +{ + /// + /// Lightweight base class for observable implementations. + /// + /// The observable type. + /// + /// is rather heavyweight in terms of allocations and memory + /// usage. This class provides a more lightweight base for some internal observable types + /// in the Avalonia framework. + /// + public abstract class LightweightObservableBase : IObservable + { + private Exception _error; + private List> _observers = new List>(); + + public IDisposable Subscribe(IObserver observer) + { + Contract.Requires(observer != null); + Dispatcher.UIThread.VerifyAccess(); + + if (_observers == null) + { + if (_error != null) + { + observer.OnError(_error); + } + else + { + observer.OnCompleted(); + } + + return Disposable.Empty; + } + + lock (_observers) + { + _observers.Add(observer); + } + + if (_observers.Count == 1) + { + Initialize(); + } + + Subscribed(observer); + + return Disposable.Create(() => + { + _observers?.Remove(observer); + + if (_observers?.Count == 0) + { + Deinitialize(); + _observers.TrimExcess(); + } + }); + } + + protected abstract void Initialize(); + protected abstract void Deinitialize(); + + protected void PublishNext(T value) + { + lock (_observers) + { + foreach (var observer in _observers) + { + observer.OnNext(value); + } + } + } + + protected void PublishCompleted() + { + lock (_observers) + { + foreach (var observer in _observers) + { + observer.OnCompleted(); + } + + _observers = null; + } + + Deinitialize(); + } + + protected void PublishError(Exception error) + { + lock (_observers) + { + foreach (var observer in _observers) + { + observer.OnError(error); + } + + _observers = null; + } + + _error = error; + Deinitialize(); + } + + protected virtual void Subscribed(IObserver observer) + { + } + } +} diff --git a/src/Avalonia.Styling/Styling/ActivatedObservable.cs b/src/Avalonia.Styling/Styling/ActivatedObservable.cs index 4933daed31..5b2774943a 100644 --- a/src/Avalonia.Styling/Styling/ActivatedObservable.cs +++ b/src/Avalonia.Styling/Styling/ActivatedObservable.cs @@ -9,7 +9,7 @@ namespace Avalonia.Styling /// An observable which is switched on or off according to an activator observable. /// /// - /// An has two inputs: an activator observable a + /// An has two inputs: an activator observable and a /// observable which produces the activated value. When the activator /// produces true, the will produce the current activated /// value. When the activator produces false it will produce @@ -69,8 +69,8 @@ namespace Avalonia.Styling } protected new ActivatedObservable Parent => (ActivatedObservable)base.Parent; - void IObserver.OnCompleted() => Parent.NotifyCompleted(); - void IObserver.OnError(Exception error) => Parent.NotifyError(error); + void IObserver.OnCompleted() => Parent.CompletedReceived(); + void IObserver.OnError(Exception error) => Parent.ErrorReceived(error); void IObserver.OnNext(object value) => Parent.NotifyValue(value); } } diff --git a/src/Avalonia.Styling/Styling/ActivatedSubject.cs b/src/Avalonia.Styling/Styling/ActivatedSubject.cs index bcbf30649a..a8446c4bfb 100644 --- a/src/Avalonia.Styling/Styling/ActivatedSubject.cs +++ b/src/Avalonia.Styling/Styling/ActivatedSubject.cs @@ -10,11 +10,9 @@ namespace Avalonia.Styling /// A subject which is switched on or off according to an activator observable. /// /// - /// An has two inputs: an activator observable and either an - /// or a observable which produces the - /// activated value. When the activator produces true, the will - /// produce the current activated value. When the activator produces false it will produce - /// . + /// An extends to + /// be an . When the object is active then values + /// received via will be passed to the source subject. /// internal class ActivatedSubject : ActivatedObservable, ISubject, IDescription { @@ -63,37 +61,37 @@ namespace Avalonia.Styling } } - protected override void NotifyCompleted() + protected override void ActiveChanged(bool active) { - base.NotifyCompleted(); + bool first = !IsActive.HasValue; - if (!_completed) + base.ActiveChanged(active); + + if (!first) { - Source.OnCompleted(); - _completed = true; + Source.OnNext(active ? _pushValue : AvaloniaProperty.UnsetValue); } } - protected override void NotifyError(Exception error) + protected override void CompletedReceived() { - base.NotifyError(error); + base.CompletedReceived(); if (!_completed) { - Source.OnError(error); + Source.OnCompleted(); _completed = true; } } - protected override void NotifyActive(bool active) + protected override void ErrorReceived(Exception error) { - bool first = !IsActive.HasValue; + base.ErrorReceived(error); - base.NotifyActive(active); - - if (!first) + if (!_completed) { - Source.OnNext(active ? _pushValue : AvaloniaProperty.UnsetValue); + Source.OnError(error); + _completed = true; } } diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index a68ae1e435..5c69c2c9aa 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -2,9 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections.Generic; -using System.Reactive.Disposables; -using Avalonia.Threading; +using Avalonia.Reactive; namespace Avalonia.Styling { @@ -17,10 +15,9 @@ namespace Avalonia.Styling /// will produce the current value. When the activator /// produces false it will produce . /// - internal class ActivatedValue : IObservable, IDescription + internal class ActivatedValue : LightweightObservableBase, IDescription { private static readonly object NotSent = new object(); - private List> _observers = new List>(); private IDisposable _activatorSubscription; private object _value; private object _last = NotSent; @@ -74,68 +71,35 @@ namespace Avalonia.Styling protected ActivatorListener Listener { get; } - public virtual IDisposable Subscribe(IObserver observer) + protected virtual void ActiveChanged(bool active) { - Contract.Requires(observer != null); - Dispatcher.UIThread.VerifyAccess(); - - _observers.Add(observer); - - if (_observers.Count == 1) - { - Initialize(); - } - - return Disposable.Create(() => - { - _observers.Remove(observer); - - if (_observers.Count == 0) - { - Deinitialize(); - } - }); + IsActive = active; + PublishValue(); } + protected virtual void CompletedReceived() => PublishCompleted(); + protected virtual ActivatorListener CreateListener() => new ActivatorListener(this); - protected virtual void Deinitialize() + protected override void Deinitialize() { _activatorSubscription.Dispose(); _activatorSubscription = null; } - protected virtual void Initialize() - { - _activatorSubscription = Activator.Subscribe(Listener); - } + protected virtual void ErrorReceived(Exception error) => PublishError(error); - protected virtual void NotifyCompleted() + protected override void Initialize() { - foreach (var observer in _observers) - { - observer.OnCompleted(); - } - - Deinitialize(); - _observers = null; + _activatorSubscription = Activator.Subscribe(Listener); } - protected virtual void NotifyError(Exception error) + protected override void Subscribed(IObserver observer) { - foreach (var observer in _observers) + if (IsActive == true) { - observer.OnError(error); + observer.OnNext(Value); } - - Deinitialize(); - _observers = null; - } - - protected virtual void NotifyActive(bool active) - { - IsActive = active; - PublishValue(); } private void PublishValue() @@ -146,11 +110,7 @@ namespace Avalonia.Styling if (!Equals(v, _last)) { - foreach (var observer in _observers) - { - observer.OnNext(v); - } - + PublishNext(v); _last = v; } } @@ -165,9 +125,9 @@ namespace Avalonia.Styling protected ActivatedValue Parent { get; } - void IObserver.OnCompleted() => Parent.NotifyCompleted(); - void IObserver.OnError(Exception error) => Parent.NotifyError(error); - void IObserver.OnNext(bool value) => Parent.NotifyActive(value); + void IObserver.OnCompleted() => Parent.CompletedReceived(); + void IObserver.OnError(Exception error) => Parent.ErrorReceived(error); + void IObserver.OnNext(bool value) => Parent.ActiveChanged(value); } } } From dadc30d84ebfbb257c0a2625937efc781b525b49 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Dec 2017 00:26:36 +0000 Subject: [PATCH 07/19] Use lightweight observable for ExpressionObserver. --- .../Data/Core/ExpressionObserver.cs | 105 ++++++++---------- .../Reactive/LightweightObservableBase.cs | 69 ++++++++---- .../Styling/ActivatedValue.cs | 4 +- 3 files changed, 98 insertions(+), 80 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs index 14bc09f5b7..3a25407133 100644 --- a/src/Avalonia.Base/Data/Core/ExpressionObserver.cs +++ b/src/Avalonia.Base/Data/Core/ExpressionObserver.cs @@ -4,18 +4,19 @@ using System; using System.Collections.Generic; using System.Reactive; -using System.Reactive.Disposables; using System.Reactive.Linq; -using System.Reactive.Subjects; using Avalonia.Data; using Avalonia.Data.Core.Plugins; +using Avalonia.Reactive; namespace Avalonia.Data.Core { /// /// Observes and sets the value of an expression on an object. /// - public class ExpressionObserver : ObservableBase, IDescription + public class ExpressionObserver : LightweightObservableBase, + IDescription, + IObserver { /// /// An ordered collection of property accessor plugins that can be used to customize @@ -54,9 +55,10 @@ namespace Avalonia.Data.Core private static readonly object UninitializedValue = new object(); private readonly ExpressionNode _node; - private readonly Subject _finished; - private readonly object _root; - private IObservable _result; + private IDisposable _nodeSubscription; + private object _root; + private IDisposable _rootSubscription; + private WeakReference _value; /// /// Initializes a new instance of the class. @@ -107,7 +109,6 @@ namespace Avalonia.Data.Core Expression = expression; Description = description ?? expression; _node = Parse(expression, enableDataValidation); - _finished = new Subject(); _root = rootObservable; } @@ -135,8 +136,6 @@ namespace Avalonia.Data.Core Expression = expression; Description = description ?? expression; _node = Parse(expression, enableDataValidation); - _finished = new Subject(); - _node.Target = new WeakReference(rootGetter()); _root = update.Select(x => rootGetter()); } @@ -203,27 +202,42 @@ namespace Avalonia.Data.Core } } - /// - protected override IDisposable SubscribeCore(IObserver observer) + void IObserver.OnNext(object value) { - if (_result == null) - { - var source = (IObservable)_node; + var broken = BindingNotification.ExtractError(value) as MarkupBindingChainException; + broken?.Commit(Description); + _value = new WeakReference(value); + PublishNext(value); + } - if (_finished != null) - { - source = source.TakeUntil(_finished); - } + void IObserver.OnCompleted() + { + } - _result = Observable.Using(StartRoot, _ => source) - .Select(ToWeakReference) - .Publish(UninitializedValue) - .RefCount() - .Where(x => x != UninitializedValue) - .Select(Translate); - } + void IObserver.OnError(Exception error) + { + } + + protected override void Initialize() + { + _value = null; + _nodeSubscription = _node.Subscribe(this); + StartRoot(); + } + + protected override void Deinitialize() + { + _rootSubscription?.Dispose(); + _nodeSubscription?.Dispose(); + _rootSubscription = _nodeSubscription = null; + } - return _result.Subscribe(observer); + protected override void Subscribed(IObserver observer, bool first) + { + if (!first && _value != null && _value.TryGetTarget(out var value)) + { + observer.OnNext(value); + } } private static ExpressionNode Parse(string expression, bool enableDataValidation) @@ -238,42 +252,19 @@ namespace Avalonia.Data.Core } } - private static object ToWeakReference(object o) + private void StartRoot() { - return o is BindingNotification ? o : new WeakReference(o); - } - - private object Translate(object o) - { - if (o is WeakReference weak) + if (_root is IObservable observable) { - return weak.Target; + _rootSubscription = observable.Subscribe( + x => _node.Target = new WeakReference(x != AvaloniaProperty.UnsetValue ? x : null), + x => PublishCompleted(), + () => PublishCompleted()); } - else if (BindingNotification.ExtractError(o) is MarkupBindingChainException broken) - { - broken.Commit(Description); - } - - return o; - } - - private IDisposable StartRoot() - { - switch (_root) + else { - case IObservable observable: - return observable.Subscribe( - x => _node.Target = new WeakReference(x != AvaloniaProperty.UnsetValue ? x : null), - _ => _finished.OnNext(Unit.Default), - () => _finished.OnNext(Unit.Default)); - case WeakReference weak: - _node.Target = weak; - break; - default: - throw new AvaloniaInternalException("The ExpressionObserver._root member should only be either an observable or WeakReference."); + _node.Target = (WeakReference)_root; } - - return Disposable.Empty; } } } diff --git a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs index 3ae6e5916b..ced2c03b01 100644 --- a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs +++ b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs @@ -39,26 +39,34 @@ namespace Avalonia.Reactive return Disposable.Empty; } + var first = _observers.Count == 0; + lock (_observers) { _observers.Add(observer); } - if (_observers.Count == 1) + if (first) { Initialize(); } - Subscribed(observer); + Subscribed(observer, first); return Disposable.Create(() => { - _observers?.Remove(observer); - - if (_observers?.Count == 0) + if (_observers != null) { - Deinitialize(); - _observers.TrimExcess(); + lock (_observers) + { + _observers?.Remove(observer); + + if (_observers?.Count == 0) + { + Deinitialize(); + _observers.TrimExcess(); + } + } } }); } @@ -68,9 +76,16 @@ namespace Avalonia.Reactive protected void PublishNext(T value) { - lock (_observers) + if (_observers != null) { - foreach (var observer in _observers) + IObserver[] observers; + + lock (_observers) + { + observers = _observers.ToArray(); + } + + foreach (var observer in observers) { observer.OnNext(value); } @@ -79,36 +94,48 @@ namespace Avalonia.Reactive protected void PublishCompleted() { - lock (_observers) + if (_observers != null) { - foreach (var observer in _observers) + IObserver[] observers; + + lock (_observers) + { + observers = _observers.ToArray(); + _observers = null; + } + + foreach (var observer in observers) { observer.OnCompleted(); } - _observers = null; + Deinitialize(); } - - Deinitialize(); } protected void PublishError(Exception error) { - lock (_observers) + if (_observers != null) { - foreach (var observer in _observers) + IObserver[] observers; + + lock (_observers) + { + observers = _observers.ToArray(); + _observers = null; + } + + foreach (var observer in observers) { observer.OnError(error); } - _observers = null; + _error = error; + Deinitialize(); } - - _error = error; - Deinitialize(); } - protected virtual void Subscribed(IObserver observer) + protected virtual void Subscribed(IObserver observer, bool first) { } } diff --git a/src/Avalonia.Styling/Styling/ActivatedValue.cs b/src/Avalonia.Styling/Styling/ActivatedValue.cs index 5c69c2c9aa..908d89b751 100644 --- a/src/Avalonia.Styling/Styling/ActivatedValue.cs +++ b/src/Avalonia.Styling/Styling/ActivatedValue.cs @@ -94,9 +94,9 @@ namespace Avalonia.Styling _activatorSubscription = Activator.Subscribe(Listener); } - protected override void Subscribed(IObserver observer) + protected override void Subscribed(IObserver observer, bool first) { - if (IsActive == true) + if (IsActive == true && !first) { observer.OnNext(Value); } From 0f1664a84672f49a85f26a7e65718c631d8c81c4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 24 Dec 2017 21:05:17 +0000 Subject: [PATCH 08/19] Use custom rx for observing properties. --- src/Avalonia.Base/AvaloniaObjectExtensions.cs | 46 ++----------------- .../Reactive/AvaloniaObservable.cs | 42 ----------------- .../Reactive/AvaloniaPropertyObservable.cs | 46 +++++++++++++++++++ 3 files changed, 51 insertions(+), 83 deletions(-) delete mode 100644 src/Avalonia.Base/Reactive/AvaloniaObservable.cs create mode 100644 src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs diff --git a/src/Avalonia.Base/AvaloniaObjectExtensions.cs b/src/Avalonia.Base/AvaloniaObjectExtensions.cs index 1da2ecb942..71e6b9474f 100644 --- a/src/Avalonia.Base/AvaloniaObjectExtensions.cs +++ b/src/Avalonia.Base/AvaloniaObjectExtensions.cs @@ -41,27 +41,7 @@ namespace Avalonia Contract.Requires(o != null); Contract.Requires(property != null); - return new AvaloniaObservable( - observer => - { - EventHandler handler = (s, e) => - { - if (e.Property == property) - { - observer.OnNext(e.NewValue); - } - }; - - observer.OnNext(o.GetValue(property)); - - o.PropertyChanged += handler; - - return Disposable.Create(() => - { - o.PropertyChanged -= handler; - }); - }, - GetDescription(o, property)); + return new AvaloniaPropertyObservable(o, property); } /// @@ -79,7 +59,7 @@ namespace Avalonia Contract.Requires(o != null); Contract.Requires(property != null); - return o.GetObservable((AvaloniaProperty)property).Cast(); + return new AvaloniaPropertyObservable(o, property); } /// @@ -100,25 +80,9 @@ namespace Avalonia Contract.Requires(o != null); Contract.Requires(property != null); - return new AvaloniaObservable>( - observer => - { - EventHandler handler = (s, e) => - { - if (e.Property == property) - { - observer.OnNext(Tuple.Create((T)e.OldValue, (T)e.NewValue)); - } - }; - - o.PropertyChanged += handler; - - return Disposable.Create(() => - { - o.PropertyChanged -= handler; - }); - }, - GetDescription(o, property)); + return new AvaloniaPropertyObservable(o, property) + .Buffer(2, 1) + .Select(x => Tuple.Create(x[0], x[1])); } /// diff --git a/src/Avalonia.Base/Reactive/AvaloniaObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaObservable.cs deleted file mode 100644 index b59d8e5226..0000000000 --- a/src/Avalonia.Base/Reactive/AvaloniaObservable.cs +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (c) The Avalonia 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; -using System.Reactive.Disposables; - -namespace Avalonia.Reactive -{ - /// - /// An with an additional description. - /// - /// The type of the elements in the sequence. - public class AvaloniaObservable : ObservableBase, IDescription - { - private readonly Func, IDisposable> _subscribe; - - /// - /// Initializes a new instance of the class. - /// - /// The subscribe function. - /// The description of the observable. - public AvaloniaObservable(Func, IDisposable> subscribe, string description) - { - Contract.Requires(subscribe != null); - - _subscribe = subscribe; - Description = description; - } - - /// - /// Gets the description of the observable. - /// - public string Description { get; } - - /// - protected override IDisposable SubscribeCore(IObserver observer) - { - return _subscribe(observer) ?? Disposable.Empty; - } - } -} \ No newline at end of file diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs new file mode 100644 index 0000000000..c19423b623 --- /dev/null +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs @@ -0,0 +1,46 @@ +using System; + +namespace Avalonia.Reactive +{ + public class AvaloniaPropertyObservable : LightweightObservableBase, IDescription + { + private readonly IAvaloniaObject _target; + private readonly AvaloniaProperty _property; + private T _value; + + public AvaloniaPropertyObservable( + IAvaloniaObject target, + AvaloniaProperty property) + { + _target = target; + _property = property; + } + + public string Description => $"{_target.GetType().Name}.{_property.Name}"; + + protected override void Initialize() + { + _value = (T)_target.GetValue(_property); + _target.PropertyChanged += PropertyChanged; + } + + protected override void Deinitialize() + { + _target.PropertyChanged -= PropertyChanged; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(_value); + } + + private void PropertyChanged(object sender, AvaloniaPropertyChangedEventArgs e) + { + if (e.Property == _property) + { + _value = (T)e.NewValue; + PublishNext(_value); + } + } + } +} From 34474af8350f0312aa85f287ddace77db8a05e16 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Dec 2017 11:54:42 +0000 Subject: [PATCH 09/19] Make AvaloniaPropertyObservable use weak refs. And remove `WeakPropertyChangedObservable`/`GetWeakObservable`. --- src/Avalonia.Base/AvaloniaObjectExtensions.cs | 23 ++--- .../Plugins/AvaloniaPropertyAccessorPlugin.cs | 2 +- .../Reactive/AvaloniaPropertyObservable.cs | 17 ++-- .../Reactive/WeakPropertyChangedObservable.cs | 85 ------------------- 4 files changed, 19 insertions(+), 108 deletions(-) delete mode 100644 src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs diff --git a/src/Avalonia.Base/AvaloniaObjectExtensions.cs b/src/Avalonia.Base/AvaloniaObjectExtensions.cs index 71e6b9474f..75e56c7186 100644 --- a/src/Avalonia.Base/AvaloniaObjectExtensions.cs +++ b/src/Avalonia.Base/AvaloniaObjectExtensions.cs @@ -36,6 +36,9 @@ namespace Avalonia /// An observable which fires immediately with the current value of the property on the /// object and subsequently each time the property value changes. /// + /// + /// The subscription to is created using a weak reference. + /// public static IObservable GetObservable(this IAvaloniaObject o, AvaloniaProperty property) { Contract.Requires(o != null); @@ -54,6 +57,9 @@ namespace Avalonia /// An observable which fires immediately with the current value of the property on the /// object and subsequently each time the property value changes. /// + /// + /// The subscription to is created using a weak reference. + /// public static IObservable GetObservable(this IAvaloniaObject o, AvaloniaProperty property) { Contract.Requires(o != null); @@ -130,23 +136,6 @@ namespace Avalonia o.GetObservable(property)); } - /// - /// Gets a weak observable for a . - /// - /// The object. - /// The property. - /// An observable. - public static IObservable GetWeakObservable(this IAvaloniaObject o, AvaloniaProperty property) - { - Contract.Requires(o != null); - Contract.Requires(property != null); - - return new WeakPropertyChangedObservable( - new WeakReference(o), - property, - GetDescription(o, property)); - } - /// /// Binds a property on an to an . /// diff --git a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs index 8cbcaa8233..48edb218dc 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs @@ -153,7 +153,7 @@ namespace Avalonia.Data.Core.Plugins protected override void SubscribeCore(IObserver observer) { - _subscription = Instance?.GetWeakObservable(_property).Subscribe(observer); + _subscription = Instance?.GetObservable(_property).Subscribe(observer); } } } diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs index c19423b623..0e07065d6c 100644 --- a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs @@ -1,10 +1,11 @@ using System; +using Avalonia.Utilities; namespace Avalonia.Reactive { public class AvaloniaPropertyObservable : LightweightObservableBase, IDescription { - private readonly IAvaloniaObject _target; + private readonly WeakReference _target; private readonly AvaloniaProperty _property; private T _value; @@ -12,7 +13,7 @@ namespace Avalonia.Reactive IAvaloniaObject target, AvaloniaProperty property) { - _target = target; + _target = new WeakReference(target); _property = property; } @@ -20,13 +21,19 @@ namespace Avalonia.Reactive protected override void Initialize() { - _value = (T)_target.GetValue(_property); - _target.PropertyChanged += PropertyChanged; + if (_target.TryGetTarget(out var target)) + { + _value = (T)target.GetValue(_property); + target.PropertyChanged += PropertyChanged; + } } protected override void Deinitialize() { - _target.PropertyChanged -= PropertyChanged; + if (_target.TryGetTarget(out var target)) + { + target.PropertyChanged -= PropertyChanged; + } } protected override void Subscribed(IObserver observer, bool first) diff --git a/src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs b/src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs deleted file mode 100644 index aa5f553865..0000000000 --- a/src/Avalonia.Base/Reactive/WeakPropertyChangedObservable.cs +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) The Avalonia 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; -using System.Reactive.Disposables; -using System.Reactive.Linq; -using System.Reactive.Subjects; -using Avalonia.Utilities; - -namespace Avalonia.Reactive -{ - internal class WeakPropertyChangedObservable : ObservableBase, - IWeakSubscriber, IDescription - { - private WeakReference _sourceReference; - private readonly AvaloniaProperty _property; - private readonly Subject _changed = new Subject(); - - private int _count; - - public WeakPropertyChangedObservable( - WeakReference source, - AvaloniaProperty property, - string description) - { - _sourceReference = source; - _property = property; - Description = description; - } - - public string Description { get; } - - public void OnEvent(object sender, AvaloniaPropertyChangedEventArgs e) - { - if (e.Property == _property) - { - _changed.OnNext(e.NewValue); - } - } - - protected override IDisposable SubscribeCore(IObserver observer) - { - IAvaloniaObject instance; - - if (_sourceReference.TryGetTarget(out instance)) - { - if (_count++ == 0) - { - WeakSubscriptionManager.Subscribe( - instance, - nameof(instance.PropertyChanged), - this); - } - - observer.OnNext(instance.GetValue(_property)); - - return Observable.Using(() => Disposable.Create(DecrementCount), _ => _changed) - .Subscribe(observer); - } - else - { - _changed.OnCompleted(); - observer.OnCompleted(); - return Disposable.Empty; - } - } - - private void DecrementCount() - { - if (--_count == 0) - { - IAvaloniaObject instance; - - if (_sourceReference.TryGetTarget(out instance)) - { - WeakSubscriptionManager.Unsubscribe( - instance, - nameof(instance.PropertyChanged), - this); - } - } - } - } -} From 3150d3f10aa9b22f06b0f9ecee16dc8f60f680f1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 Dec 2017 12:29:01 +0000 Subject: [PATCH 10/19] Added GetPropertyChangedObservable. Replaced `GetObservableWithHistory` with `GetPropertyChangedObservable`. --- src/Avalonia.Base/AvaloniaObjectExtensions.cs | 18 ++++---- .../AvaloniaPropertyChangedObservable.cs | 46 +++++++++++++++++++ .../Reactive/AvaloniaPropertyObservable.cs | 3 +- .../Mixins/ContentControlMixin.cs | 40 ++++++---------- 4 files changed, 69 insertions(+), 38 deletions(-) create mode 100644 src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs diff --git a/src/Avalonia.Base/AvaloniaObjectExtensions.cs b/src/Avalonia.Base/AvaloniaObjectExtensions.cs index 75e56c7186..393482cccf 100644 --- a/src/Avalonia.Base/AvaloniaObjectExtensions.cs +++ b/src/Avalonia.Base/AvaloniaObjectExtensions.cs @@ -69,26 +69,24 @@ namespace Avalonia } /// - /// Gets an observable for a . + /// Gets an observable that listens for property changed events for an + /// . /// /// The object. - /// The type of the property. /// The property. /// - /// An observable which when subscribed pushes the old and new values of the property each - /// time it is changed. Note that the observable returned from this method does not fire - /// with the current value of the property immediately. + /// An observable which when subscribed pushes the property changed event args + /// each time a event is raised + /// for the specified property. /// - public static IObservable> GetObservableWithHistory( + public static IObservable GetPropertyChangedObservable( this IAvaloniaObject o, - AvaloniaProperty property) + AvaloniaProperty property) { Contract.Requires(o != null); Contract.Requires(property != null); - return new AvaloniaPropertyObservable(o, property) - .Buffer(2, 1) - .Select(x => Tuple.Create(x[0], x[1])); + return new AvaloniaPropertyChangedObservable(o, property); } /// diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs new file mode 100644 index 0000000000..5ef0d25133 --- /dev/null +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyChangedObservable.cs @@ -0,0 +1,46 @@ +using System; + +namespace Avalonia.Reactive +{ + internal class AvaloniaPropertyChangedObservable : + LightweightObservableBase, + IDescription + { + private readonly WeakReference _target; + private readonly AvaloniaProperty _property; + + public AvaloniaPropertyChangedObservable( + IAvaloniaObject target, + AvaloniaProperty property) + { + _target = new WeakReference(target); + _property = property; + } + + public string Description => $"{_target.GetType().Name}.{_property.Name}"; + + protected override void Initialize() + { + if (_target.TryGetTarget(out var target)) + { + target.PropertyChanged += PropertyChanged; + } + } + + protected override void Deinitialize() + { + if (_target.TryGetTarget(out var target)) + { + target.PropertyChanged -= PropertyChanged; + } + } + + private void PropertyChanged(object sender, AvaloniaPropertyChangedEventArgs e) + { + if (e.Property == _property) + { + PublishNext(e); + } + } + } +} diff --git a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs index 0e07065d6c..4385ab13ef 100644 --- a/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs +++ b/src/Avalonia.Base/Reactive/AvaloniaPropertyObservable.cs @@ -1,9 +1,8 @@ using System; -using Avalonia.Utilities; namespace Avalonia.Reactive { - public class AvaloniaPropertyObservable : LightweightObservableBase, IDescription + internal class AvaloniaPropertyObservable : LightweightObservableBase, IDescription { private readonly WeakReference _target; private readonly AvaloniaProperty _property; diff --git a/src/Avalonia.Controls/Mixins/ContentControlMixin.cs b/src/Avalonia.Controls/Mixins/ContentControlMixin.cs index daaf3fd5cf..95193c0432 100644 --- a/src/Avalonia.Controls/Mixins/ContentControlMixin.cs +++ b/src/Avalonia.Controls/Mixins/ContentControlMixin.cs @@ -49,11 +49,9 @@ namespace Avalonia.Controls.Mixins Contract.Requires(content != null); Contract.Requires(logicalChildrenSelector != null); - EventHandler templateApplied = (s, ev) => + void TemplateApplied(object s, RoutedEventArgs ev) { - var sender = s as TControl; - - if (sender != null) + if (s is TControl sender) { var e = (TemplateAppliedEventArgs)ev; var presenter = (IControl)e.NameScope.Find(presenterName); @@ -64,12 +62,12 @@ namespace Avalonia.Controls.Mixins var logicalChildren = logicalChildrenSelector(sender); var subscription = presenter - .GetObservableWithHistory(ContentPresenter.ChildProperty) - .Subscribe(child => UpdateLogicalChild( + .GetPropertyChangedObservable(ContentPresenter.ChildProperty) + .Subscribe(c => UpdateLogicalChild( sender, - logicalChildren, - child.Item1, - child.Item2)); + logicalChildren, + c.OldValue, + c.NewValue)); UpdateLogicalChild( sender, @@ -80,18 +78,16 @@ namespace Avalonia.Controls.Mixins subscriptions.Value.Add(sender, subscription); } } - }; + } TemplatedControl.TemplateAppliedEvent.AddClassHandler( typeof(TControl), - templateApplied, + TemplateApplied, RoutingStrategies.Direct); content.Changed.Subscribe(e => { - var sender = e.Sender as TControl; - - if (sender != null) + if (e.Sender is TControl sender) { var logicalChildren = logicalChildrenSelector(sender); UpdateLogicalChild(sender, logicalChildren, e.OldValue, e.NewValue); @@ -100,9 +96,7 @@ namespace Avalonia.Controls.Mixins Control.TemplatedParentProperty.Changed.Subscribe(e => { - var sender = e.Sender as TControl; - - if (sender != null) + if (e.Sender is TControl sender) { var logicalChild = logicalChildrenSelector(sender).FirstOrDefault() as IControl; logicalChild?.SetValue(Control.TemplatedParentProperty, sender.TemplatedParent); @@ -111,13 +105,9 @@ namespace Avalonia.Controls.Mixins TemplatedControl.TemplateProperty.Changed.Subscribe(e => { - var sender = e.Sender as TControl; - - if (sender != null) + if (e.Sender is TControl sender) { - IDisposable subscription; - - if (subscriptions.Value.TryGetValue(sender, out subscription)) + if (subscriptions.Value.TryGetValue(sender, out IDisposable subscription)) { subscription.Dispose(); subscriptions.Value.Remove(sender); @@ -134,9 +124,7 @@ namespace Avalonia.Controls.Mixins { if (oldValue != newValue) { - var child = oldValue as IControl; - - if (child != null) + if (oldValue is IControl child) { logicalChildren.Remove(child); } From 360a2603c3285482f7d981c417d4b66e6cfafab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Karnok?= Date: Sat, 26 May 2018 00:36:39 +0200 Subject: [PATCH 11/19] Fix LightweightObservableBase's concurrency, save 1 allocation. --- .../Reactive/LightweightObservableBase.cs | 122 +++++++++++++----- 1 file changed, 91 insertions(+), 31 deletions(-) diff --git a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs index ced2c03b01..a2786d63da 100644 --- a/src/Avalonia.Base/Reactive/LightweightObservableBase.cs +++ b/src/Avalonia.Base/Reactive/LightweightObservableBase.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Reactive; using System.Reactive.Disposables; +using System.Threading; using Avalonia.Threading; namespace Avalonia.Reactive @@ -25,25 +26,35 @@ namespace Avalonia.Reactive Contract.Requires(observer != null); Dispatcher.UIThread.VerifyAccess(); - if (_observers == null) + var first = false; + + for (; ; ) { - if (_error != null) - { - observer.OnError(_error); - } - else + if (Volatile.Read(ref _observers) == null) { - observer.OnCompleted(); - } + if (_error != null) + { + observer.OnError(_error); + } + else + { + observer.OnCompleted(); + } - return Disposable.Empty; - } + return Disposable.Empty; + } - var first = _observers.Count == 0; + lock (this) + { + if (_observers == null) + { + continue; + } - lock (_observers) - { - _observers.Add(observer); + first = _observers.Count == 0; + _observers.Add(observer); + break; + } } if (first) @@ -53,22 +64,57 @@ namespace Avalonia.Reactive Subscribed(observer, first); - return Disposable.Create(() => + return new RemoveObserver(this, observer); + } + + void Remove(IObserver observer) + { + if (Volatile.Read(ref _observers) != null) { - if (_observers != null) + lock (this) { - lock (_observers) + var observers = _observers; + + if (observers != null) { - _observers?.Remove(observer); + observers.Remove(observer); - if (_observers?.Count == 0) + if (observers.Count == 0) + { + observers.TrimExcess(); + } + else { - Deinitialize(); - _observers.TrimExcess(); + return; } + } else + { + return; } } - }); + + Deinitialize(); + } + } + + sealed class RemoveObserver : IDisposable + { + LightweightObservableBase _parent; + + IObserver _observer; + + public RemoveObserver(LightweightObservableBase parent, IObserver observer) + { + _parent = parent; + Volatile.Write(ref _observer, observer); + } + + public void Dispose() + { + var observer = _observer; + Interlocked.Exchange(ref _parent, null)?.Remove(observer); + _observer = null; + } } protected abstract void Initialize(); @@ -76,12 +122,16 @@ namespace Avalonia.Reactive protected void PublishNext(T value) { - if (_observers != null) + if (Volatile.Read(ref _observers) != null) { IObserver[] observers; - lock (_observers) + lock (this) { + if (_observers == null) + { + return; + } observers = _observers.ToArray(); } @@ -94,14 +144,18 @@ namespace Avalonia.Reactive protected void PublishCompleted() { - if (_observers != null) + if (Volatile.Read(ref _observers) != null) { IObserver[] observers; - lock (_observers) + lock (this) { + if (_observers == null) + { + return; + } observers = _observers.ToArray(); - _observers = null; + Volatile.Write(ref _observers, null); } foreach (var observer in observers) @@ -115,14 +169,21 @@ namespace Avalonia.Reactive protected void PublishError(Exception error) { - if (_observers != null) + if (Volatile.Read(ref _observers) != null) { + IObserver[] observers; - lock (_observers) + lock (this) { + if (_observers == null) + { + return; + } + + _error = error; observers = _observers.ToArray(); - _observers = null; + Volatile.Write(ref _observers, null); } foreach (var observer in observers) @@ -130,7 +191,6 @@ namespace Avalonia.Reactive observer.OnError(error); } - _error = error; Deinitialize(); } } From 9b0aff8cc1c0a1ed18333355459394aee75b8268 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 01:14:49 +0200 Subject: [PATCH 12/19] Use LightweightObservableBase in TypeNameAndClassSelector --- .../Styling/TypeNameAndClassSelector.cs | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs index 5d48496e8c..93d4e14f27 100644 --- a/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Styling/Styling/TypeNameAndClassSelector.cs @@ -4,12 +4,10 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; -using System.Reactive; -using System.Reactive.Disposables; -using System.Reactive.Linq; using System.Reflection; using System.Text; using Avalonia.Collections; +using Avalonia.Reactive; namespace Avalonia.Styling { @@ -200,10 +198,9 @@ namespace Avalonia.Styling return builder.ToString(); } - private class ClassObserver : IObservable + private class ClassObserver : LightweightObservableBase { readonly IList _match; - readonly List> _observers = new List>(); IAvaloniaReadOnlyList _classes; public ClassObserver(IAvaloniaReadOnlyList classes, IList match) @@ -212,34 +209,19 @@ namespace Avalonia.Styling _match = match; } - public IDisposable Subscribe(IObserver observer) - { - if (_observers.Count == 0) - { - _classes.CollectionChanged += ClassesChanged; - } + protected override void Deinitialize() => _classes.CollectionChanged -= ClassesChanged; + protected override void Initialize() => _classes.CollectionChanged += ClassesChanged; - _observers.Add(observer); + protected override void Subscribed(IObserver observer, bool first) + { observer.OnNext(GetResult()); - - return Disposable.Create(() => - { - _observers.Remove(observer); - if (_observers.Count == 0) - { - _classes.CollectionChanged -= ClassesChanged; - } - }); } private void ClassesChanged(object sender, NotifyCollectionChangedEventArgs e) { if (e.Action != NotifyCollectionChangedAction.Move) { - foreach (var observer in _observers) - { - observer.OnNext(GetResult()); - } + PublishNext(GetResult()); } } From de325ec501f2104ea2870e92dd5ea2ef792fec5d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 01:44:59 +0200 Subject: [PATCH 13/19] Use LightweightObservableBase for WeakCollectionChangedObservable --- .../NotifyCollectionChangedExtensions.cs | 59 ++++++------------- 1 file changed, 17 insertions(+), 42 deletions(-) diff --git a/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs b/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs index d295cb91ce..3a355bcb48 100644 --- a/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs +++ b/src/Avalonia.Base/Collections/NotifyCollectionChangedExtensions.cs @@ -2,13 +2,9 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections; -using System.Collections.Generic; using System.Collections.Specialized; -using System.Reactive; -using System.Reactive.Disposables; using System.Reactive.Linq; -using System.Reactive.Subjects; +using Avalonia.Reactive; using Avalonia.Utilities; namespace Avalonia.Collections @@ -43,9 +39,8 @@ namespace Avalonia.Collections Contract.Requires(collection != null); Contract.Requires(handler != null); - return - collection.GetWeakCollectionChangedObservable() - .Subscribe(e => handler.Invoke(collection, e)); + return collection.GetWeakCollectionChangedObservable() + .Subscribe(e => handler(collection, e)); } /// @@ -63,18 +58,13 @@ namespace Avalonia.Collections Contract.Requires(collection != null); Contract.Requires(handler != null); - return - collection.GetWeakCollectionChangedObservable() - .Subscribe(handler); + return collection.GetWeakCollectionChangedObservable().Subscribe(handler); } - private class WeakCollectionChangedObservable : ObservableBase, + private class WeakCollectionChangedObservable : LightweightObservableBase, IWeakSubscriber { private WeakReference _sourceReference; - private readonly Subject _changed = new Subject(); - - private int _count; public WeakCollectionChangedObservable(WeakReference source) { @@ -83,43 +73,28 @@ namespace Avalonia.Collections public void OnEvent(object sender, NotifyCollectionChangedEventArgs e) { - _changed.OnNext(e); + PublishNext(e); } - protected override IDisposable SubscribeCore(IObserver observer) + protected override void Initialize() { if (_sourceReference.TryGetTarget(out INotifyCollectionChanged instance)) { - if (_count++ == 0) - { - WeakSubscriptionManager.Subscribe( - instance, - nameof(instance.CollectionChanged), - this); - } - - return Observable.Using(() => Disposable.Create(DecrementCount), _ => _changed) - .Subscribe(observer); - } - else - { - _changed.OnCompleted(); - observer.OnCompleted(); - return Disposable.Empty; + WeakSubscriptionManager.Subscribe( + instance, + nameof(instance.CollectionChanged), + this); } } - private void DecrementCount() + protected override void Deinitialize() { - if (--_count == 0) + if (_sourceReference.TryGetTarget(out INotifyCollectionChanged instance)) { - if (_sourceReference.TryGetTarget(out INotifyCollectionChanged instance)) - { - WeakSubscriptionManager.Unsubscribe( - instance, - nameof(instance.CollectionChanged), - this); - } + WeakSubscriptionManager.Unsubscribe( + instance, + nameof(instance.CollectionChanged), + this); } } } From a3dea23560c04c1d603f91af1d6fe65e0e949254 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 02:28:12 +0200 Subject: [PATCH 14/19] Use LightweightObservableBase for BindingExpression. --- .../Data/Core/BindingExpression.cs | 43 ++++++++++++++++--- .../Data/Core/BindingExpressionTests.cs | 15 +++++++ 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/BindingExpression.cs b/src/Avalonia.Base/Data/Core/BindingExpression.cs index 4b41d1568c..911c621fb7 100644 --- a/src/Avalonia.Base/Data/Core/BindingExpression.cs +++ b/src/Avalonia.Base/Data/Core/BindingExpression.cs @@ -7,6 +7,7 @@ using System.Reactive.Linq; using System.Reactive.Subjects; using Avalonia.Data.Converters; using Avalonia.Logging; +using Avalonia.Reactive; using Avalonia.Utilities; namespace Avalonia.Data.Core @@ -15,13 +16,14 @@ namespace Avalonia.Data.Core /// Binds to an expression on an object using a type value converter to convert the values /// that are send and received. /// - public class BindingExpression : ISubject, IDescription + public class BindingExpression : LightweightObservableBase, ISubject, IDescription { private readonly ExpressionObserver _inner; private readonly Type _targetType; private readonly object _fallbackValue; private readonly BindingPriority _priority; - private readonly Subject _errors = new Subject(); + InnerListener _innerListener; + WeakReference _value; /// /// Initializes a new instance of the class. @@ -139,7 +141,7 @@ namespace Avalonia.Data.Core "IValueConverter should not return non-errored BindingNotification."); } - _errors.OnNext(notification); + PublishNext(notification); if (_fallbackValue != AvaloniaProperty.UnsetValue) { @@ -170,12 +172,18 @@ namespace Avalonia.Data.Core } } - /// - public IDisposable Subscribe(IObserver observer) + protected override void Initialize() => _innerListener = new InnerListener(this); + protected override void Deinitialize() => _innerListener.Dispose(); + + protected override void Subscribed(IObserver observer, bool first) { - return _inner.Select(ConvertValue).Merge(_errors).Subscribe(observer); + if (!first && _value != null && _value.TryGetTarget(out var val) == true) + { + observer.OnNext(val); + } } + /// private object ConvertValue(object value) { var notification = value as BindingNotification; @@ -301,5 +309,28 @@ namespace Avalonia.Data.Core return a; } + + public class InnerListener : IObserver, IDisposable + { + private readonly BindingExpression _owner; + private readonly IDisposable _dispose; + + public InnerListener(BindingExpression owner) + { + _owner = owner; + _dispose = owner._inner.Subscribe(this); + } + + public void Dispose() => _dispose.Dispose(); + public void OnCompleted() => _owner.PublishCompleted(); + public void OnError(Exception error) => _owner.PublishError(error); + + public void OnNext(object value) + { + var converted = _owner.ConvertValue(value); + _owner._value = new WeakReference(converted); + _owner.PublishNext(converted); + } + } } } diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs index 6b71d28e22..81acd6a087 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.cs @@ -337,6 +337,21 @@ namespace Avalonia.Base.UnitTests.Data.Core GC.KeepAlive(data); } + [Fact] + public void Second_Subscription_Should_Fire_Immediately() + { + var data = new Class1 { StringValue = "foo" }; + var target = new BindingExpression(new ExpressionObserver(data, "StringValue"), typeof(string)); + object result = null; + + target.Subscribe(); + target.Subscribe(x => result = x); + + Assert.Equal("foo", result); + + GC.KeepAlive(data); + } + private class Class1 : NotifyingBase { private string _stringValue; From 365aa42b4ab1396561f94f148fe024f494a9c4e3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 13:10:25 +0200 Subject: [PATCH 15/19] Use custom rx for binding update signal. --- .../Data/Core/BindingExpression.cs | 2 +- .../SingleSubscriberObservableBase.cs | 79 +++++++++++++++++++ .../Styling/StyleActivator.cs | 4 +- src/Markup/Avalonia.Markup/Data/Binding.cs | 44 ++++++++--- 4 files changed, 115 insertions(+), 14 deletions(-) create mode 100644 src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs diff --git a/src/Avalonia.Base/Data/Core/BindingExpression.cs b/src/Avalonia.Base/Data/Core/BindingExpression.cs index 911c621fb7..c4ffa839e0 100644 --- a/src/Avalonia.Base/Data/Core/BindingExpression.cs +++ b/src/Avalonia.Base/Data/Core/BindingExpression.cs @@ -14,7 +14,7 @@ namespace Avalonia.Data.Core { /// /// Binds to an expression on an object using a type value converter to convert the values - /// that are send and received. + /// that are sent and received. /// public class BindingExpression : LightweightObservableBase, ISubject, IDescription { diff --git a/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs new file mode 100644 index 0000000000..af48df177e --- /dev/null +++ b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs @@ -0,0 +1,79 @@ +using System; +using Avalonia.Threading; + +namespace Avalonia.Reactive +{ + public abstract class SingleSubscriberObservableBase : IObservable, IDisposable + { + private Exception _error; + private IObserver _observer; + private bool _completed; + + public IDisposable Subscribe(IObserver observer) + { + Contract.Requires(observer != null); + Dispatcher.UIThread.VerifyAccess(); + + if (_observer != null) + { + throw new InvalidOperationException("The observable can only be subscribed once."); + } + + if (_error != null) + { + observer.OnError(_error); + } + else if (_completed) + { + observer.OnCompleted(); + } + else + { + _observer = observer; + Subscribed(); + } + + return this; + } + + void IDisposable.Dispose() + { + Unsubscribed(); + _observer = null; + } + + protected abstract void Unsubscribed(); + + protected void PublishNext(T value) + { + if (_observer != null) + { + _observer?.OnNext(value); + } + } + + protected void PublishCompleted() + { + if (_observer != null) + { + _observer?.OnCompleted(); + _completed = true; + Unsubscribed(); + _observer = null; + } + } + + protected void PublishError(Exception error) + { + if (_observer != null) + { + _observer?.OnError(error); + _error = error; + Unsubscribed(); + _observer = null; + } + } + + protected abstract void Subscribed(); + } +} diff --git a/src/Avalonia.Styling/Styling/StyleActivator.cs b/src/Avalonia.Styling/Styling/StyleActivator.cs index 5bdea9f42e..63945037d8 100644 --- a/src/Avalonia.Styling/Styling/StyleActivator.cs +++ b/src/Avalonia.Styling/Styling/StyleActivator.cs @@ -48,8 +48,8 @@ namespace Avalonia.Styling else { return inputs.CombineLatest() - .Select(values => values.Any(x => x)) - .DistinctUntilChanged(); + .Select(values => values.Any(x => x)) + .DistinctUntilChanged(); } } } diff --git a/src/Markup/Avalonia.Markup/Data/Binding.cs b/src/Markup/Avalonia.Markup/Data/Binding.cs index 96fc2986e8..ef5ebb9844 100644 --- a/src/Markup/Avalonia.Markup/Data/Binding.cs +++ b/src/Markup/Avalonia.Markup/Data/Binding.cs @@ -5,11 +5,10 @@ using System; using System.Linq; using System.Reactive; using System.Reactive.Linq; -using System.Reflection; -using Avalonia.Controls; using Avalonia.Data.Converters; using Avalonia.Data.Core; using Avalonia.LogicalTree; +using Avalonia.Reactive; using Avalonia.VisualTree; namespace Avalonia.Data @@ -190,13 +189,10 @@ namespace Avalonia.Data if (!targetIsDataContext) { - var update = target.GetObservable(StyledElement.DataContextProperty) - .Skip(1) - .Select(_ => Unit.Default); var result = new ExpressionObserver( () => target.GetValue(StyledElement.DataContextProperty), path, - update, + new UpdateSignal(target, StyledElement.DataContextProperty), enableDataValidation); return result; @@ -278,14 +274,10 @@ namespace Avalonia.Data { Contract.Requires(target != null); - var update = target.GetObservable(StyledElement.TemplatedParentProperty) - .Skip(1) - .Select(_ => Unit.Default); - var result = new ExpressionObserver( () => target.GetValue(StyledElement.TemplatedParentProperty), path, - update, + new UpdateSignal(target, StyledElement.TemplatedParentProperty), enableDataValidation); return result; @@ -306,5 +298,35 @@ namespace Avalonia.Data Observable.Return((object)null); }).Switch(); } + + private class UpdateSignal : SingleSubscriberObservableBase + { + private readonly IAvaloniaObject _target; + private readonly AvaloniaProperty _property; + + public UpdateSignal(IAvaloniaObject target, AvaloniaProperty property) + { + _target = target; + _property = property; + } + + protected override void Subscribed() + { + _target.PropertyChanged += PropertyChanged; + } + + protected override void Unsubscribed() + { + _target.PropertyChanged -= PropertyChanged; + } + + private void PropertyChanged(object sender, AvaloniaPropertyChangedEventArgs e) + { + if (e.Property == _property) + { + PublishNext(Unit.Default); + } + } + } } } From b488ca179a0b72aa074c24d7948fe78f380af69a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 13:32:05 +0200 Subject: [PATCH 16/19] Use custom class for direct binding subscriptions. --- src/Avalonia.Base/AvaloniaObject.cs | 53 ++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 48e72db126..baca1494be 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -10,6 +10,7 @@ using System.Reactive.Linq; using Avalonia.Data; using Avalonia.Diagnostics; using Avalonia.Logging; +using Avalonia.Reactive; using Avalonia.Threading; using Avalonia.Utilities; @@ -38,7 +39,7 @@ namespace Avalonia /// Maintains a list of direct property binding subscriptions so that the binding source /// doesn't get collected. /// - private List _directBindings; + private List _directBindings; /// /// Event handler for implementation. @@ -359,25 +360,12 @@ namespace Avalonia property, description); - IDisposable subscription = null; - if (_directBindings == null) { - _directBindings = new List(); + _directBindings = new List(); } - subscription = source - .Select(x => CastOrDefault(x, property.PropertyType)) - .Do(_ => { }, () => _directBindings.Remove(subscription)) - .Subscribe(x => SetDirectValue(property, x)); - - _directBindings.Add(subscription); - - return Disposable.Create(() => - { - subscription.Dispose(); - _directBindings.Remove(subscription); - }); + return new DirectBindingSubscription(this, property, source); } else { @@ -908,5 +896,38 @@ namespace Avalonia value, priority); } + + private class DirectBindingSubscription : IObserver, IDisposable + { + readonly AvaloniaObject _owner; + readonly AvaloniaProperty _property; + IDisposable _subscription; + + public DirectBindingSubscription( + AvaloniaObject owner, + AvaloniaProperty property, + IObservable source) + { + _owner = owner; + _property = property; + _owner._directBindings.Add(this); + _subscription = source.Subscribe(this); + } + + public void Dispose() + { + _subscription.Dispose(); + _owner._directBindings.Remove(this); + } + + public void OnCompleted() => Dispose(); + public void OnError(Exception error) => Dispose(); + + public void OnNext(object value) + { + var castValue = CastOrDefault(value, _property.PropertyType); + _owner.SetDirectValue(_property, castValue); + } + } } } From a8d5dc1da4e49d57d241bc21beac4df7412d4b70 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 14:13:58 +0200 Subject: [PATCH 17/19] Use custom rx for ResourceObservable. --- .../Controls/ResourceProviderExtensions.cs | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs b/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs index 1f25fa132d..1586c61185 100644 --- a/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs +++ b/src/Avalonia.Styling/Controls/ResourceProviderExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Reactive; using System.Reactive.Linq; +using Avalonia.Reactive; namespace Avalonia.Controls { @@ -55,11 +56,39 @@ namespace Avalonia.Controls public static IObservable GetResourceObservable(this IResourceNode target, string key) { - return Observable.FromEventPattern( - x => target.ResourcesChanged += x, - x => target.ResourcesChanged -= x) - .StartWith((EventPattern)null) - .Select(x => target.FindResource(key)); + return new ResourceObservable(target, key); + } + + private class ResourceObservable : LightweightObservableBase + { + private readonly IResourceNode _target; + private readonly string _key; + + public ResourceObservable(IResourceNode target, string key) + { + _target = target; + _key = key; + } + + protected override void Initialize() + { + _target.ResourcesChanged += ResourcesChanged; + } + + protected override void Deinitialize() + { + _target.ResourcesChanged -= ResourcesChanged; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(_target.FindResource(_key)); + } + + private void ResourcesChanged(object sender, ResourcesChangedEventArgs e) + { + PublishNext(_target.FindResource(_key)); + } } } } From 26b3efd8d402b4a182e20152f9d7f5f6d84cc1a9 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 15:27:53 +0200 Subject: [PATCH 18/19] Use custom rx for control locators. --- .../LogicalTree/ControlLocator.cs | 162 ++++++++++++------ .../VisualTree/VisualLocator.cs | 67 +++++--- 2 files changed, 147 insertions(+), 82 deletions(-) diff --git a/src/Avalonia.Styling/LogicalTree/ControlLocator.cs b/src/Avalonia.Styling/LogicalTree/ControlLocator.cs index 2858d11d9d..7186143bf9 100644 --- a/src/Avalonia.Styling/LogicalTree/ControlLocator.cs +++ b/src/Avalonia.Styling/LogicalTree/ControlLocator.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Reactive.Linq; using System.Reflection; using Avalonia.Controls; +using Avalonia.Reactive; namespace Avalonia.LogicalTree { @@ -23,75 +24,122 @@ namespace Avalonia.LogicalTree /// The name of the control to find. public static IObservable Track(ILogical relativeTo, string name) { - var attached = Observable.FromEventPattern( - x => relativeTo.AttachedToLogicalTree += x, - x => relativeTo.AttachedToLogicalTree -= x) - .Select(x => ((ILogical)x.Sender).FindNameScope()) - .StartWith(relativeTo.FindNameScope()); - - var detached = Observable.FromEventPattern( - x => relativeTo.DetachedFromLogicalTree += x, - x => relativeTo.DetachedFromLogicalTree -= x) - .Select(x => (INameScope)null); - - return attached.Merge(detached).Select(nameScope => + return new ControlTracker(relativeTo, name); + } + + public static IObservable Track(ILogical relativeTo, int ancestorLevel, Type ancestorType = null) + { + return new ControlTracker(relativeTo, ancestorLevel, ancestorType); + } + + private class ControlTracker : LightweightObservableBase + { + private readonly ILogical _relativeTo; + private readonly string _name; + private readonly int _ancestorLevel; + private readonly Type _ancestorType; + INameScope _nameScope; + ILogical _value; + + public ControlTracker(ILogical relativeTo, string name) + { + _relativeTo = relativeTo; + _name = name; + } + + public ControlTracker(ILogical relativeTo, int ancestorLevel, Type ancestorType) { - if (nameScope != null) + _relativeTo = relativeTo; + _ancestorLevel = ancestorLevel; + _ancestorType = ancestorType; + } + + protected override void Initialize() + { + Update(); + _relativeTo.AttachedToLogicalTree += Attached; + _relativeTo.DetachedFromLogicalTree += Detached; + } + + protected override void Deinitialize() + { + _relativeTo.AttachedToLogicalTree -= Attached; + _relativeTo.DetachedFromLogicalTree -= Detached; + + if (_nameScope != null) { - var registered = Observable.FromEventPattern( - x => nameScope.Registered += x, - x => nameScope.Registered -= x) - .Where(x => x.EventArgs.Name == name) - .Select(x => x.EventArgs.Element) - .OfType(); - var unregistered = Observable.FromEventPattern( - x => nameScope.Unregistered += x, - x => nameScope.Unregistered -= x) - .Where(x => x.EventArgs.Name == name) - .Select(_ => (ILogical)null); - return registered - .StartWith(nameScope.Find(name)) - .Merge(unregistered); + _nameScope.Registered -= Registered; + _nameScope.Unregistered -= Unregistered; } - else + + _value = null; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(_value); + } + + private void Attached(object sender, LogicalTreeAttachmentEventArgs e) + { + Update(); + PublishNext(_value); + } + + private void Detached(object sender, LogicalTreeAttachmentEventArgs e) + { + if (_nameScope != null) { - return Observable.Return(null); + _nameScope.Registered -= Registered; + _nameScope.Unregistered -= Unregistered; } - }).Switch(); - } - public static IObservable Track(ILogical relativeTo, int ancestorLevel, Type ancestorType = null) - { - return TrackAttachmentToTree(relativeTo).Select(isAttachedToTree => + _value = null; + PublishNext(null); + } + + private void Registered(object sender, NameScopeEventArgs e) { - if (isAttachedToTree) + if (e.Name == _name && e.Element is ILogical logical) { - return relativeTo.GetLogicalAncestors() - .Where(x => ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) - .ElementAtOrDefault(ancestorLevel); + _value = logical; + PublishNext(logical); } - else + } + + private void Unregistered(object sender, NameScopeEventArgs e) + { + if (e.Name == _name) { - return null; + _value = null; + PublishNext(null); } - }); - } + } - private static IObservable TrackAttachmentToTree(ILogical relativeTo) - { - var attached = Observable.FromEventPattern( - x => relativeTo.AttachedToLogicalTree += x, - x => relativeTo.AttachedToLogicalTree -= x) - .Select(x => true) - .StartWith(relativeTo.IsAttachedToLogicalTree); - - var detached = Observable.FromEventPattern( - x => relativeTo.DetachedFromLogicalTree += x, - x => relativeTo.DetachedFromLogicalTree -= x) - .Select(x => false); - - var attachmentStatus = attached.Merge(detached); - return attachmentStatus; + private void Update() + { + if (_name != null) + { + _nameScope = _relativeTo.FindNameScope(); + + if (_nameScope != null) + { + _nameScope.Registered += Registered; + _nameScope.Unregistered += Unregistered; + _value = _nameScope.Find(_name); + } + else + { + _value = null; + } + } + else + { + _value = _relativeTo.GetLogicalAncestors() + .Where(x => _ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) + .ElementAtOrDefault(_ancestorLevel); + } + } } } } diff --git a/src/Avalonia.Visuals/VisualTree/VisualLocator.cs b/src/Avalonia.Visuals/VisualTree/VisualLocator.cs index 6fcb6c0db1..eaca9ed9d7 100644 --- a/src/Avalonia.Visuals/VisualTree/VisualLocator.cs +++ b/src/Avalonia.Visuals/VisualTree/VisualLocator.cs @@ -1,9 +1,8 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Reactive.Linq; using System.Reflection; -using System.Text; +using Avalonia.Reactive; namespace Avalonia.VisualTree { @@ -11,36 +10,54 @@ namespace Avalonia.VisualTree { public static IObservable Track(IVisual relativeTo, int ancestorLevel, Type ancestorType = null) { - return TrackAttachmentToTree(relativeTo).Select(isAttachedToTree => + return new VisualTracker(relativeTo, ancestorLevel, ancestorType); + } + + private class VisualTracker : LightweightObservableBase + { + private readonly IVisual _relativeTo; + private readonly int _ancestorLevel; + private readonly Type _ancestorType; + + public VisualTracker(IVisual relativeTo, int ancestorLevel, Type ancestorType) + { + _relativeTo = relativeTo; + _ancestorLevel = ancestorLevel; + _ancestorType = ancestorType; + } + + protected override void Initialize() + { + _relativeTo.AttachedToVisualTree += AttachedDetached; + _relativeTo.DetachedFromVisualTree += AttachedDetached; + } + + protected override void Deinitialize() { - if (isAttachedToTree) + _relativeTo.AttachedToVisualTree -= AttachedDetached; + _relativeTo.DetachedFromVisualTree -= AttachedDetached; + } + + protected override void Subscribed(IObserver observer, bool first) + { + observer.OnNext(GetResult()); + } + + private void AttachedDetached(object sender, VisualTreeAttachmentEventArgs e) => PublishNext(GetResult()); + + private IVisual GetResult() + { + if (_relativeTo.IsAttachedToVisualTree) { - return relativeTo.GetVisualAncestors() - .Where(x => ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) - .ElementAtOrDefault(ancestorLevel); + return _relativeTo.GetVisualAncestors() + .Where(x => _ancestorType?.GetTypeInfo().IsAssignableFrom(x.GetType().GetTypeInfo()) ?? true) + .ElementAtOrDefault(_ancestorLevel); } else { return null; } - }); - } - - private static IObservable TrackAttachmentToTree(IVisual relativeTo) - { - var attached = Observable.FromEventPattern( - x => relativeTo.AttachedToVisualTree += x, - x => relativeTo.AttachedToVisualTree -= x) - .Select(x => true) - .StartWith(relativeTo.IsAttachedToVisualTree); - - var detached = Observable.FromEventPattern( - x => relativeTo.DetachedFromVisualTree += x, - x => relativeTo.DetachedFromVisualTree -= x) - .Select(x => false); - - var attachmentStatus = attached.Merge(detached); - return attachmentStatus; + } } } } From ef5be70c4b6451a1f142b33dfd6b154b7c3d0ddd Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 23 Jun 2018 21:13:48 +0200 Subject: [PATCH 19/19] Don't double-check nulls. --- .../Reactive/SingleSubscriberObservableBase.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs index af48df177e..cd8ce2cd80 100644 --- a/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs +++ b/src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs @@ -46,17 +46,14 @@ namespace Avalonia.Reactive protected void PublishNext(T value) { - if (_observer != null) - { - _observer?.OnNext(value); - } + _observer?.OnNext(value); } protected void PublishCompleted() { if (_observer != null) { - _observer?.OnCompleted(); + _observer.OnCompleted(); _completed = true; Unsubscribed(); _observer = null; @@ -67,7 +64,7 @@ namespace Avalonia.Reactive { if (_observer != null) { - _observer?.OnError(error); + _observer.OnError(error); _error = error; Unsubscribed(); _observer = null;