Skip to content

Commit 346e3bb

Browse files
jl0pdglennawatson
jl0pd
and
glennawatson
authored
Reduce allocations within ReactiveObject and ReactiveRecord (#4195)
<!-- Please be sure to read the [Contribute](https://github.com/reactiveui/reactiveui#contribute) section of the README --> **What kind of change does this PR introduce?** <!-- Bug fix, feature, docs update, ... --> Performance improvement. **What is the current behavior?** <!-- You can also link to an open issue here. --> Currently `ReactiveObject` and `ReactiveRecord` use `Lazy<T>` to lazily initialize `PropertyChanging`, `PropertyChanged`, `Changing`, `Changed`, `ThrownExceptions`. This requires allocation of `Lazy<T>` itself and allocation of closure. This produces unnecessary overhead of allocating 11 objects during construction instead of 1. This was main problem when I was writing log reader, where each entry initially inherited from `ReactiveObject` and I had to reimplement `INotifyProertyChanged` manually. **What is the new behavior?** <!-- If this is a feature change --> `Lazy<T>` was replaced with `Interlocked.CompareExchange`, events initialization was performed with a simple `if` statement. It's a simplified version of what [`System.Threading.LazyInitializer`](https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Threading/LazyInitializer.cs) does. **What might this PR break?** Concurrent first access to `PropertyChanging` or `PropertyChanged` events or first access to `Changing`, `Changed`, `ThrownExceptions` properties. **Please check if the PR fulfills these requirements** - [ ] Tests for the changes have been added (for bug fixes / features) - [ ] Docs have been added / updated (for bug fixes / features) **Other information**: --------- Co-authored-by: Glenn <[email protected]>
1 parent d6b0ced commit 346e3bb

File tree

2 files changed

+37
-54
lines changed

2 files changed

+37
-54
lines changed

‎src/ReactiveUI/ReactiveObject/ReactiveObject.cs‎

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,42 +13,27 @@ namespace ReactiveUI;
1313
[DataContract]
1414
public class ReactiveObject : IReactiveNotifyPropertyChanged<IReactiveObject>, IHandleObservableErrors, IReactiveObject
1515
{
16-
private readonly Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>> _changing;
17-
private readonly Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>> _changed;
18-
private readonly Lazy<Unit> _propertyChangingEventsSubscribed;
19-
private readonly Lazy<Unit> _propertyChangedEventsSubscribed;
20-
private readonly Lazy<IObservable<Exception>> _thrownExceptions;
16+
private bool _propertyChangingEventsSubscribed;
17+
private bool _propertyChangedEventsSubscribed;
2118

2219
/// <summary>
2320
/// Initializes a new instance of the <see cref="ReactiveObject"/> class.
2421
/// </summary>
2522
public ReactiveObject()
2623
{
27-
_changing = new Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>>(() => ((IReactiveObject)this).GetChangingObservable(), LazyThreadSafetyMode.PublicationOnly);
28-
_changed = new Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>>(() => ((IReactiveObject)this).GetChangedObservable(), LazyThreadSafetyMode.PublicationOnly);
29-
_propertyChangingEventsSubscribed = new Lazy<Unit>(
30-
() =>
31-
{
32-
this.SubscribePropertyChangingEvents();
33-
return Unit.Default;
34-
},
35-
LazyThreadSafetyMode.PublicationOnly);
36-
_propertyChangedEventsSubscribed = new Lazy<Unit>(
37-
() =>
38-
{
39-
this.SubscribePropertyChangedEvents();
40-
return Unit.Default;
41-
},
42-
LazyThreadSafetyMode.PublicationOnly);
43-
_thrownExceptions = new Lazy<IObservable<Exception>>(this.GetThrownExceptionsObservable, LazyThreadSafetyMode.PublicationOnly);
4424
}
4525

4626
/// <inheritdoc/>
4727
public event PropertyChangingEventHandler? PropertyChanging
4828
{
4929
add
5030
{
51-
_ = _propertyChangingEventsSubscribed.Value;
31+
if (!_propertyChangingEventsSubscribed)
32+
{
33+
this.SubscribePropertyChangingEvents();
34+
_propertyChangingEventsSubscribed = true;
35+
}
36+
5237
PropertyChangingHandler += value;
5338
}
5439
remove => PropertyChangingHandler -= value;
@@ -59,7 +44,12 @@ public event PropertyChangedEventHandler? PropertyChanged
5944
{
6045
add
6146
{
62-
_ = _propertyChangedEventsSubscribed.Value;
47+
if (!_propertyChangedEventsSubscribed)
48+
{
49+
this.SubscribePropertyChangedEvents();
50+
_propertyChangedEventsSubscribed = true;
51+
}
52+
6353
PropertyChangedHandler += value;
6454
}
6555
remove => PropertyChangedHandler -= value;
@@ -78,7 +68,8 @@ public event PropertyChangedEventHandler? PropertyChanged
7868
[Browsable(false)]
7969
[Display(Order = -1, AutoGenerateField = false, AutoGenerateFilter = false)]
8070
#endif
81-
public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changing => _changing.Value;
71+
public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changing =>
72+
Volatile.Read(ref field) ?? Interlocked.CompareExchange(ref field, ((IReactiveObject)this).GetChangingObservable(), null) ?? field;
8273

8374
/// <inheritdoc />
8475
[IgnoreDataMember]
@@ -87,7 +78,8 @@ public event PropertyChangedEventHandler? PropertyChanged
8778
[Browsable(false)]
8879
[Display(Order = -1, AutoGenerateField = false, AutoGenerateFilter = false)]
8980
#endif
90-
public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changed => _changed.Value;
81+
public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changed =>
82+
Volatile.Read(ref field) ?? Interlocked.CompareExchange(ref field, ((IReactiveObject)this).GetChangedObservable(), null) ?? field;
9183

9284
/// <inheritdoc/>
9385
[IgnoreDataMember]
@@ -96,7 +88,8 @@ public event PropertyChangedEventHandler? PropertyChanged
9688
[Browsable(false)]
9789
[Display(Order = -1, AutoGenerateField = false, AutoGenerateFilter = false)]
9890
#endif
99-
public IObservable<Exception> ThrownExceptions => _thrownExceptions.Value;
91+
public IObservable<Exception> ThrownExceptions =>
92+
Volatile.Read(ref field) ?? Interlocked.CompareExchange(ref field, this.GetThrownExceptionsObservable(), null) ?? field;
10093

10194
/// <inheritdoc/>
10295
void IReactiveObject.RaisePropertyChanging(PropertyChangingEventArgs args) =>

‎src/ReactiveUI/ReactiveObject/ReactiveRecord.cs‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,8 @@ namespace ReactiveUI;
1313
[DataContract]
1414
public record ReactiveRecord : IReactiveNotifyPropertyChanged<IReactiveObject>, IHandleObservableErrors, IReactiveObject
1515
{
16-
private readonly Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>> _changing;
17-
private readonly Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>> _changed;
18-
private readonly Lazy<Unit> _propertyChangingEventsSubscribed;
19-
private readonly Lazy<Unit> _propertyChangedEventsSubscribed;
20-
private readonly Lazy<IObservable<Exception>> _thrownExceptions;
16+
private bool _propertyChangingEventsSubscribed;
17+
private bool _propertyChangedEventsSubscribed;
2118

2219
/// <summary>
2320
/// Initializes a new instance of the <see cref="ReactiveRecord"/> class.
@@ -28,31 +25,19 @@ public record ReactiveRecord : IReactiveNotifyPropertyChanged<IReactiveObject>,
2825
#endif
2926
public ReactiveRecord()
3027
{
31-
_changing = new Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>>(() => ((IReactiveObject)this).GetChangingObservable(), LazyThreadSafetyMode.PublicationOnly);
32-
_changed = new Lazy<IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>>>(() => ((IReactiveObject)this).GetChangedObservable(), LazyThreadSafetyMode.PublicationOnly);
33-
_propertyChangingEventsSubscribed = new Lazy<Unit>(
34-
() =>
35-
{
36-
this.SubscribePropertyChangingEvents();
37-
return Unit.Default;
38-
},
39-
LazyThreadSafetyMode.PublicationOnly);
40-
_propertyChangedEventsSubscribed = new Lazy<Unit>(
41-
() =>
42-
{
43-
this.SubscribePropertyChangedEvents();
44-
return Unit.Default;
45-
},
46-
LazyThreadSafetyMode.PublicationOnly);
47-
_thrownExceptions = new Lazy<IObservable<Exception>>(this.GetThrownExceptionsObservable, LazyThreadSafetyMode.PublicationOnly);
4828
}
4929

5030
/// <inheritdoc/>
5131
public event PropertyChangingEventHandler? PropertyChanging
5232
{
5333
add
5434
{
55-
_ = _propertyChangingEventsSubscribed.Value;
35+
if (!_propertyChangingEventsSubscribed)
36+
{
37+
this.SubscribePropertyChangingEvents();
38+
_propertyChangingEventsSubscribed = true;
39+
}
40+
5641
PropertyChangingHandler += value;
5742
}
5843
remove => PropertyChangingHandler -= value;
@@ -63,7 +48,12 @@ public event PropertyChangedEventHandler? PropertyChanged
6348
{
6449
add
6550
{
66-
_ = _propertyChangedEventsSubscribed.Value;
51+
if (!_propertyChangedEventsSubscribed)
52+
{
53+
this.SubscribePropertyChangedEvents();
54+
_propertyChangedEventsSubscribed = true;
55+
}
56+
6757
PropertyChangedHandler += value;
6858
}
6959
remove => PropertyChangedHandler -= value;
@@ -83,7 +73,7 @@ public event PropertyChangedEventHandler? PropertyChanged
8373
[Display(Order = -1, AutoGenerateField = false, AutoGenerateFilter = false)]
8474
#endif
8575
public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changing => // TODO: Create Test
86-
_changing.Value;
76+
Volatile.Read(ref field) ?? Interlocked.CompareExchange(ref field, ((IReactiveObject)this).GetChangingObservable(), null) ?? field;
8777

8878
/// <inheritdoc />
8979
[IgnoreDataMember]
@@ -93,7 +83,7 @@ public event PropertyChangedEventHandler? PropertyChanged
9383
[Display(Order = -1, AutoGenerateField = false, AutoGenerateFilter = false)]
9484
#endif
9585
public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changed => // TODO: Create Test
96-
_changed.Value;
86+
Volatile.Read(ref field) ?? Interlocked.CompareExchange(ref field, ((IReactiveObject)this).GetChangedObservable(), null) ?? field;
9787

9888
/// <inheritdoc/>
9989
[IgnoreDataMember]
@@ -102,7 +92,7 @@ public event PropertyChangedEventHandler? PropertyChanged
10292
[Browsable(false)]
10393
[Display(Order = -1, AutoGenerateField = false, AutoGenerateFilter = false)]
10494
#endif
105-
public IObservable<Exception> ThrownExceptions => _thrownExceptions.Value;
95+
public IObservable<Exception> ThrownExceptions => Volatile.Read(ref field) ?? Interlocked.CompareExchange(ref field, this.GetThrownExceptionsObservable(), null) ?? field;
10696

10797
/// <inheritdoc/>
10898
void IReactiveObject.RaisePropertyChanging(PropertyChangingEventArgs args) => PropertyChangingHandler?.Invoke(this, args);

0 commit comments

Comments
 (0)